Every so often on my site, I get an error message something like "user error: Duplicate entry 'locale:en' for key 1" inserting into the {cache} table.

It happens when the server is under heavy load. I searched for {cache} in my drupal directory and found that the only place where a SQL insert was being called on the table was in bootstrap.inc.

Looking at the code (which calls an update first, then if no rows were affected, calling insert), it seems to me that two concurrent calls with the same cid (which would happen under heavy load) would obviously fail if no entry for that cid were in the database. Both updates would not affect any rows simultaneously, and then both inserts would start with identical cids, which would cause the error above.

I've commented out the body of cache_set on my site for now, which seems like it works as a workaround.

CommentFileSizeAuthor
#2 patch_13.txt584 bytesWesley Tanaka

Comments

Wesley Tanaka’s picture

Wesley Tanaka commented

This is happening way too much to be the race condition above. Also, the locale keys should never disappear under normal site operation from what I can tell. I think the problem is this instead:

When using UPDATE, MySQL will not update columns where the new value is the same as the old value. This creates the possibility that mysql_affected_rows() may not actually equal the number of rows matched, only the number of rows that were literally affected by the query.

I'm surprised this hasn't come up before.

Wesley Tanaka’s picture

Wesley Tanaka commented
StatusFileSize
new patch_13.txt584 bytes

According to a comment on php.net, this can be dealt with by checking against equality with -1 instead of checking if TRUE/FALSE

Wesley Tanaka’s picture

Wesley Tanaka commented
Status: Active » Needs review

That patch (previously attached) seems to help a lot. The problem was coming up about 1/2 the time on certain pages on my site, and now it seems to have gone away.

chx’s picture

chx commented
Status: Needs review » Reviewed & tested by the community

Though 4.7 contains a different --and IMO better solution-- this will also do and does not affect version requirements like the 4.7 patch therefore it's better for the 4.6 tree.

Steven’s picture

Steven commented
Status: Reviewed & tested by the community » Needs work

First, the special -1 value is true for MySQL, but there is no such similiar comment on php.net for PgSQL. We should use the proper error functions to detect errors, rather than hacks like this.

Furthermore, I'm not sure about this 'fix'. If I understand the code right, it only executes the INSERT if the UPDATE query failed (affected rows is -1). But as far as I know, an UPDATE query which matches no rows is still considered succesful and affected_rows will return 0 in that case. So, this patch would stop any data from being inserted at all?

Cvbge’s picture

Cvbge commented

What about using first SELECT count(*) WHERE cid = ... and then if (found) { UPDATE } else { INSERT }
That should help if db_affected_rows() does not work as it should (don't know, hasn't investigated).

chaos21in’s picture

chaos21in commented
Version: 4.6.3 » 4.6.5
Status: Needs work » Active

i installed 4.6.5 version on my server and it was working perfectly. The problem started when i put smtp.module in module directory and make it active from settings. when i remove this module everything again works fine, but the error comes back whenever i put smtp.module and try to login/logout etc...

can somebody please make a small patch for it.

regards
--rafi

simbot’s picture

simbot commented

My solution to this was to use the MySQL statement INSERT ... ON DUPLICATE KEY UPDATE... as shown below. This eliminates the race condition, without the need for explicit table locking. Alternatively you could use the REPLACE INTO syntax, used in the id generation. Either way, these are DB specific commands, so the cache_set function should probably be moved into the database.{mysql|pgsql}.inc files.

function cache_set($cid, $data, $expire = CACHE_PERMANENT, $headers = NULL) {
$data = db_encode_blob($data);

@db_query("INSERT INTO {cache} (cid, data, created, expire, headers) VALUES ('%s', '%s', %d, %d, '%s') ON DUPLICATE KEY UPDATE data = '%s', created = %d, expire = %d, headers = '%s'", $cid, $data, time(), $expire, $headers, $data, time(), $expire, $headers);
}

I checked with the latest release of Drupal (4.6.5) and this patch still holds.

chaos21in’s picture

chaos21in commented

Hi ..i am using 4.6.5 and i patched bootstrap.inc with your code and so far it seems its working...
Thank you.

chaos21in’s picture

chaos21in commented
Status: Active » Closed (fixed)

unless somebody else has to add something..I think i should close this topic now...

simbot’s picture

simbot commented

Does someone want to make a real patch out of this, which can be merged into the main tree?

chx’s picture

chx commented

No. This won't get in main tree because it's mysql specific. The real mysql specific solution is in 4.7 but it makes PHP 4.3 a strict requirement and we won't change requirements in a minor release that would cause havoc. (It's still only recommended to have PHP 4.3.3 for 4.6, without search module you can get away with less)

Wesley Tanaka’s picture

Wesley Tanaka commented
Status: Closed (fixed) » Active

not sure why this was closed

chx’s picture

chx commented
Status: Active » Closed (won't fix)
knubbe’s picture

knubbe commented

The solution in followup #8 didn't work for me. However, the patch in followup #2 worked fine.

In cache_set() in bootstrap.inc i replaced:
"if (!db_affected_rows()) {"

..with:
"if (db_affected_rows() == -1) {"

And now it works like a charm.