There is a problem in cache_set function in includes/bootstrap.inc.

I am using postgres 7.4.6.

Sometime (not very often) I received errors something like duplicated key cache_pkey in watchdog.

I debugged the errors and examined the code in cache_set and think the code may need to be rewritten. I think we cannot always just run an update query and rely on the db_affected_rows to determine whether we should insert a record if the update query failed.

It was annoying to see such db errors presented in the log.

The fix that I have done is to check cid first then apply either an update query if cid existing or an insert query if cid not existing.

I am hoping this bug can be fixed in Drupal 4.7 using a more efficient method.

Comments

killes@www.drop.org’s picture

killes@www.drop.org commented
Status: Active » Fixed

this code has actually been rewritten for 4.7

Anonymous’s picture

(not verified) commented
Status: Fixed » Closed (fixed)
thinker’s picture

thinker commented
Version: » 4.7.0-beta3
Status: Closed (fixed) » Active

Forgive me for re-opening this issue.

Personally speaking, I don't think the re-written code can fix the bug.

The new code still runs update query regardless the existence of cid, then rely on the number of affected rows to decide to run an insert query.

Also I don't think the newly added table locking will prevent the "duplicate key violate" problem.

I'd rather think I have not had a good understanding of the newly rewritten code for cache_set function.
Please add sufficient comments to explain why the new code can fix the problem I reported before simply closing it.

dopry’s picture

dopry commented

bump any postgre / cache experts with an answer?

dries’s picture

dries commented

cache_set() grabs a table lock so I don't see how you could get duplicate cache entries.

1. Does table locking work properly on your setup?
2. Do you use any contributed modules that write to the cache table directly (not using chache_set/cache_get)?

killes@www.drop.org’s picture

killes@www.drop.org commented
Version: 4.7.0-beta3 » x.y.z

This seems to be pgsql related since we didn't get any reports on it from mysql users.

dries’s picture

dries commented 
/**
 * Lock a table.
 * This function automatically starts a transaction.
 */
function db_lock_table($table) {
  db_query('BEGIN; LOCK TABLE {%s} IN EXCLUSIVE MODE', $table);
}

/**
 * Unlock all locked tables.
 * This function automatically commits a transation.
 */
function db_unlock_tables() {
  db_query('COMMIT');
}

I don't see why the above code (taken from database.pgsql.inc) would fail ...

dries’s picture

dries commented

Did 'thinker' actually tried using the 4.7.0/CVS version of Drupal? Looks like his bug report might originally have been posted against 4.6.x. Comment #3 seems to suggest that he didn't actually try. Maybe he's just that ... a 'thinker'? (Just kidding) ;-)

Cvbge’s picture

Cvbge commented

Personally speaking, I don't think the re-written code can fix the bug.

In 4.6 if two clients called cache_set() at the same time, it was possibile that both clients executed UPDATE at the same time, which reported "no rows updated" for both. Then both clients executed INSERT which in turn produced errors for one of them.

In 4.7 we use table locking, which allows only 1 client at time to access the table, so if first client does INSERT, the second one will do UPDATE and there will be no error.

I hope that answers your question.

moshe weitzman’s picture

moshe weitzman commented
Priority: Critical » Normal

everyone thinks this is bogus or related to 4.6 so downgrading priority

killes@www.drop.org’s picture

killes@www.drop.org commented
Status: Active » Closed (won't fix)

no further feedback