Forum OpenACS Development: Duplicate keys on assessment - possible CR bug

While working on assessment performance I developed some stress test cases for the submission of an assessment answer. This test creates 10 concurrent sessions for a given assessment and when running it many deadlocks are risen but there are also many unique constraints violations:
ERROR: duplicate key violates unique constraint "acs_objects_pk"
CONTEXT: SQL statement "insert into acs_objects (object_id, object_type, context_id, creation_date, creation_user, creation_ip) values ( $1 , $2 , $3 , $4 , $5 , $6 )"
PL/pgSQL function "acs_object__new" line 24 at SQL statement
PL/pgSQL function "content_revision__new" line 22 at assignment
PL/pgSQL function "content_revision__new" line 14 at return

I'm running OpenACS 5.1 on PG 8.0.7.

Guided by Dave's post about cr_dummy http://www.openacs.org/forums/message-view?message_id=400513, I modified the content_type__refresh_view function so it doesn't use the cr_dummy table and the call to content_revision__new is done into the insert to the content type specific table.

For example, the as_sessionsi rule used to be:
as_sessions_r AS
ON INSERT TO as_sessionsi DO INSTEAD ( UPDATE cr_dummy SET val = ( SELECT content_revision__new(new.title, new.description::character varying, now(), new.mime_type, new.nls_language,
CASE
WHEN new.text IS NULL THEN new.data
ELSE new.text
END, content_symlink__resolve(new.item_id), new.revision_id, now(), new.creation_user, new.creation_ip) AS content_revision__new);
INSERT INTO as_sessions (session_id, assessment_id, subject_id, staff_id, target_datetime, creation_datetime, first_mod_datetime, last_mod_datetime, completed_datetime, session_status, assessment_status, percent_score)
VALUES (cr_dummy.val, new.assessment_id, new.subject_id, new.staff_id, new.target_datetime, new.creation_datetime, new.first_mod_datetime, new.last_mod_datetime, new.completed_datetime, new.session_status, new.assessment_status, new.percent_score);
)

after the change, it is:
as_sessions_r AS
ON INSERT TO as_sessionsi DO INSTEAD INSERT INTO as_sessions (session_id, assessment_id, subject_id, staff_id, target_datetime, creation_datetime, first_mod_datetime, last_mod_datetime, completed_datetime, session_status, assessment_status, percent_score)
VALUES (content_revision__new(new.title, new.description::character varying, now(), new.mime_type, new.nls_language,
CASE
WHEN new.text IS NULL THEN new.data
ELSE new.text
END, content_symlink__resolve(new.item_id), new.revision_id, now(), new.creation_user, new.creation_ip), new.assessment_id, new.subject_id, new.staff_id, new.target_datetime, new.creation_datetime, new.first_mod_datetime, new.last_mod_datetime, new.completed_datetime, new.session_status, new.assessment_status, new.percent_score)

I made this change willing to reduce the number of deadlocks, but it happened to stop rising unique constraint violations though deadlocks still happen.

I searched in postgresql forums wether rules are transactional or not, and found many different and contradictory answers, so I'm not sure about it but, because of the results I got, rules don't seem to be transactional.

Does cr_dummy have any other purpose than acting as a variable inside each view rule?
Does anyone know for sure if PG rules are transaction safe (specifically the rules of CR views)?
Would the inline function call be a better approach for this case?

Thanks in advance,

Derick

cr_dummy is just a dummy table used because the design of ACS required each object type to supply a values table.

That's a requirement we've been talking about relaxing for ages.

Rules should be transactional - every top-level query in PG runs in its own transaction if no global (begin/end) transaction is specified.

Now with PG 8's nested transaction facility things might've changed, I've not dug into that stuff yet...

What kind of lock is causing the deadlock? Some code in assessment may need to be wrapped in an explicit transaction block and protected with a select for update or other form of lock.

Thanks for the answers :)

The lock causing the deadlock is a ShareLock:

ERROR: deadlock detected
DETAIL: Process 27056 waits for ShareLock on transaction 21228135; blocked by process 27061.
Process 27061 waits for ShareLock on transaction 21228133; blocked by process 27056.
CONTEXT: SQL statement "SELECT 1 FROM ONLY "public"."users" x WHERE "user_id" = $1 FOR UPDATE OF x"
SQL statement "insert into acs_objects (object_id, object_type, context_id, creation_date, creation_user, creation_ip) values ( $1 , $2 , $3 , $4 , $5 , $6 )"
PL/pgSQL function "acs_object__new" line 24 at SQL statement
PL/pgSQL function "content_item__new" line 84 at assignment

SQL:

select content_item__new('0-27860472-59F6B62A-D884-4D03-AC1C-2D74A2B6E45E','27859840',NULL,NULL,NULL,'0',NULL,'0.0.0.0','content_item','as_sessions',NULL,NULL,NULL,NULL,NULL,NULL,NULL,NULL,'file')

For duplicate key issue, I agree, a generated function will be the best, and drop the use of cr_dummy.

For the CR deadlock there is no easy solution, though your suggestions are interesting, specially since we don't need permission granularity at the revision level for most of the cases... As a short term, and due assessment is showing multiple deadlocks, we are trying to produce an assessment version which uses CR less.

I think we found a good solution for the deadlock problem.
We LOCK TABLE acs_objects IN SHARE ROW EXCLUSIVE MODE.
In our tests (50 concurrent threads creating revisions) in a good oacs DB (7M acs_objets, 2M cr_revisions), we have NO deadlocks!!!

So you do in your plsql call:
Begin
LOCK TABLE acs_objects IN SHARE ROW EXCLUSIVE MODE;
your code (usually the insert to the view in the CR case);
Return 0;
End;

You lock only the row that you care for (this locks the primary key of the table), and share the rest. Enjoy!

As a note, just adding Begin end to the transaction reduced the number of deadlocks, but still some happen, and in the way it is now, around ~75% are deadlocks. So a change in this direction must happen.

just to follow up my own note. My tests came to the following results:

- the particular lock does not help on duplicate key issue (would not have expected it). i was using my version of the rule with the stored procedure in it for my tests.

- the lock on acs_objects avoids the deadlock for my test cases, no matter whether the lock was in content_item__new or in content revision new (triggered via the rule).

- for my tests, the performance was 10% faster when the lock was used in in the content_item new transaction

- comparing storage_method "file" with "text": storage method text appears only slightly faster than storage_method file (tested with short contents)

concerning the duplicate key issue: by altering the rule into something like
CREATE or REPLACE RULE xowiki_page_r AS ON INSERT TO xowiki_pagei DO INSTEAD SELECT xowiki_pagei_f(new);
we have the effect, that an INSERT INTO xowiki_pagei (a dml statement) gets rewritten into a select statement. oacs complains, if one tries to run a select in a db_dml. The best trick to allow people (e.g. content::revision::new) still use the db_dml for the insert on VIEWi is to put an "begin; ... end;" around the sql-query in the db_dml implementation (the trick is due to Neophytos). Other suggestions?

content_item__new is sufficient in our cases as well, did you add in specific place?
We plan to provide a patch for HEAD (deadlocks).
Will be good if other OCT members comment on what we have discussed, then we can apply the patch to HEAD.
Also, we need a patch for dropping the use of cr_dummy in favor of the function call.

Here is a patch + upgrade script.

The approach with the BEGIN .. END in dml statements (mentioned earlier) lead to other problems, so i changed for the time being the few instances, where sql statements inserts directly into the TABLENAMEi view, the command from db_dml to db_0or1row. This is necessary, since the rule processor of postgres rewrites now the insert rule into a select query calling the stored procedure. This happens only in three cases (acs-content-repository, dynamic-types, xowiki). There is as well an update to the TABLENAMEi view in form-procs of dynamic types, it might be necessary to check if this is ok.

The upgrade script contains the altered rule generator which generates now a rule + a stored procedure, which uses a variable to hold the temporary result. This should be faster than the old version based on cr_dummy, which essentially allows only one insert at a time.

The code was only tested with postgresql 8.0.

This patch is not intended for the general public,
but for developers for testing purposes. so, the
patch is against CVS head.

How to use this:
- make a dump of your database
- run the .sql file with
psql -U ... < cr-deadlock-duplicate-upgrade.sql
- cd packages; patch -p0 < cr-deadlock-duplicate.patch-HEAD

http://media.wu-wien.ac.at/download/cr-deadlock-duplicate-upgrade.sql
http://media.wu-wien.ac.at/download/cr-deadlock-duplicate.patch-HEAD

If we want to proceed with the patch, it will be necessary to provide a patch for content-types.sql as well.

enjoy.

Can you provide a patch or commit your change to the repository? This would help a lot.

This sounds related to the duplicate primary key problems Malte was posting about in June and July.

Rocael,

Should we add this to content::revision::new ?

Are the deadlock and duplicate revision_id problems discussed in this thread 100% specific to PostgreSQL? And OpenACS on Oracle is not subject to these problems at all? It sounds as if that's the case, but I haven't yet heard anyone say so for sure...

Oracle has triggers on views, so it uses per default a stored procedure, id does not need the trick with cr_dummy in the generated postgres rule; oracle cannot have this issue with duplicates. The proposed version is closer to the oracle version.

I am not sure, how Oracle deals with updates on foreign keys, but i would expect, it does not have this problem as well. It is likely, that in times before MVCC, postgres did not have these issues either.

Both problems are postgres related. Oracle might have problems on its own, but not these. So far, with this patch, i have not seem both of the problems any more.

Gustaf, after today's OCT meeting we decided that you can go ahead and commit to HEAD this patch.

Hi!

In the University of Valencia we have the same problem with assessment (ERROR: duplicate key violates unique constraint "acs_objects_pk")
It is possible to apply the patch to branch oacs_5_2, somebody has tried it?

We have the assessment version 0.22d2.

Thanks.

Hi!

We copied these files from head to 5_2 to solve the problem on our production instance:

openacs-4/packages/acs-content-repository/tcl/content-folder-procs.tcl
openacs-4/packages/acs-content-repository/tcl/content-folder-procs.xql
openacs-4/packages/acs-content-repository/sql/postgresql/upgrade/upgrade-5.3.0d4-5.3.0d5.sql
openacs-4/packages/acs-content-repository/sql/postgresql/upgrade/upgrade-5.3.0d6-5.3.0d7.sql

At the moment it seems that it worked well.

Thanks for the help

This bug in PG is no longer present at PG 8.2.x version, the 8.2.0 release actually fixed that. We might return to the previous state that CR was. It is recommened to upgrade to 8.2.x since you might experiment that bug in other tables, we did in acs_objects as well until the upgrade was performed.

is related to the deadlock, so no longer needed the explicit lock in 8.2.x series.

I would do the following:

a) Require PG 8.2 or higher for OpenACS 5.4
b) For 5.3 keep the lock table statements in, or, alternatively, provide a parameter in acs-content-repository "OldPGP" which is turned on if upon installation (or upgrade) you find that the system is running on an Old DB version. Or store it in an nsv_array and check upon starting the AOLserver.

Interestingly I got problems with "lock table acs-objects in share row exclusive mode" on a farily busy site running against PG 8.2.3.

oacs 5.4 can rollback the explicit lock, although the only ones that were experimenting the deadlock are here, and will be probably aware that pg 8.2.x upgrade is recommended, but still works oacs in 8.x versions.

We did test and everything is fine on this regard, now we have in production that.

I´m sure oracle did not have this silly deadlock.

Now that it has been established that the Postgres bug that caused this issue has been fixed in PG 8.2.x, there is one thing that is not clear to me from the posts above:

Does the currently committed new code include the explicit locking proposed by Rocael (LOCK TABLE acs_objects IN SHARE ROW EXCLUSIVE MODE;)?

If so, someone mentioned that this may undermine MVCC in some cases. Is this correct and if so are we sure that the new code is as good as it can be for PG versions 8.2.x and later?

Regards
RIchard

You are right, i have removed the LOCK statements in CVS head.