Project:Memcache API and Integration
Version:6.x-1.x-dev
Component:Code
Category:bug report
Priority:major
Assigned:Unassigned
Status:closed (fixed)

Issue Summary

In regards of Issue http://drupal.org/node/426712 which is about D5 I made a patch for D6.

I have the same errors as on a high traffic site there seem to be some INSERTs running between the DELETE and the new INSERT and I get a lot of "Duplicate entry ... for key 1 query: INSERT INTO cache_filter ... in ...memcache.db.inc in Zeile 149." if I flush the cache.

From Line 147

<?php
    
// Save to the database
   
db_query("DELETE FROM {%s} WHERE cid = '%s'", $table, $cid);
   
db_query("INSERT INTO {%s} (cid, data, expire, created, headers, serialized) VALUES ('%s', %b, %d, %d, '%s', '%s')", $table, $cid, $data, $expire, $created, $headers, $serialized);
?>

This patch uses the same syntax as the core function does
http://api.drupal.org/api/function/cache_set/6

<?php
   
// Save to the database
   
db_query("UPDATE {". $table ."} SET data = %b, created = %d, expire = %d, headers = '%s', serialized = %d WHERE cid = '%s'", $data, $created, $expire, $headers, $serialized, $cid);
    if (!
db_affected_rows()) {
      @
db_query("INSERT INTO {". $table ."} (cid, data, created, expire, headers, serialized) VALUES ('%s', %b, %d, %d, '%s', %d)", $cid, $data, $created, $expire, $headers, $serialized);
    }
?>

And from my experience with simpletest {%s} does not always work as the table alias is not rewritten and you have to use {". $table ."}.

AttachmentSize
memcache.db_.inc_.patch1.1 KB

Comments

#1

http://drupal.org/node/566832 was marked as a dub. impact can be high for cached pages. please review this patch and apply

#2

It doesn't seem to work. I get errors that show there is no variables at a moment. For example:

"Warning: Division by zero in /home/sites/.../htdocs/sites/all/modules/boost/boost.module on line 103"

"Fatal error: Cannot use string offset as an array in /home/sites/.../htdocs/sites/all/modules/ubercart/payment/uc_credit/uc_credit.module on line 88"

Somehow at modules initial loading there are no variables.
Strangely I get these errors into fckeditor fields when editing a node. There are several of fckeditor fields and they are requesting data from Drupal at the same time.. So here we have a race.

I added table locking and it works fine.

AttachmentSize
memcache.db_.inc_.patch 811 bytes

#3

Status:needs review» needs work

Hi,

my patch above uses the same syntax as the core function does and all locks where removed from core for a reason. The locks are unnecessary and lessen performance. I don't think changing the patch back to the old syntax and use locking is a good idea.

Your errors could be some side effects from using boost and memcache. It just doesn't make sense that this saved data is missing during the same page call elsewhere. Feels more like they are not available in the currently active caching layer that did never receive the data because of some other caching layer.

Please make sure that your errors don't have another reason.

#4

I get this quite frequently too. The only caches I have are Drupal's standard page and block cache, using memcache.db.inc (and apc as an op-code cache).

#5

Status:needs work» needs review

Original patch applied and working (http://drupal.org/files/issues/memcache.db_.inc__0_0.patch).

#6

May be INSERT ... ON DUPLICATE KEY UPDATE is better option in this case? (no table locking)

#7

@Taras_ Sure but this is MySQL only. :-/

#8

Version:6.x-1.3» 6.x-1.x-dev
Status:needs review» reviewed & tested by the community

Original patch from Kars-T is the correct solution and should be committed.

#9

I have also tested the patch from Kars-T and can confirm that it does fix the issue.

#10

Re-rolled the original patch against the latest dev version. So far I've not had any duplicate key errors, *fingers crossed* ;-)

AttachmentSize
memcache_538042.patch 920 bytes

#11

Issue confirmed when using memcache.db.inc, patch in #10 works and should be committed.

#12

I've had to deploy this patch to several production sites. It has worked perfectly; eliminating the duplicate key errors.

Please get this into a release ASAP!

#13

Priority:normal» major

+1 on this. I set this to major because the warnings cause FUD and are completely unnecessary.

Dear Maintainers:

This a rather small patch in #10 that is backed up by some core source. It gets rid of some stupid and unnecessary warnings for high traffic sites. And this module is all about high traffic sites!

Pretty please with sugar pops add this to the module and make a bit better than it already is. Make your great work shine a bit brighter :)

#14

Just wondering if there's anything holding this one up in particular that I can help with. I've been using this patched version for quite a while now and haven't noticed any issues with it.

#15

Status:reviewed & tested by the community» fixed

#538042 by dukem, hadsie, Kars-T: Fixed cache_set() duplicated keys error.

I've committed #10 to latest dev branch (to become 1.8). Please note that in my testing, the memcache.db.inc file is *completely* broken and never hits memcache (with or without this patch), and that the file has been marked deprecated. It is suggested to move to using memcache.inc.

#16

Status:fixed» closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.