Forum OpenACS Development: Deadlock / ShareLock fix!

Collapse
Posted by Rocael Hernández Rizzardini on
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.

Collapse
Posted by Gustaf Neumann on
how is the performance? if i see corretly, we are loosing MVCC in these cases - maybe still the best choice. The same lock will be needed for other inserts/updates as well with FK to oacs_objects. anyhow, still bettern than the deadlock. i'll redo my changes an do some tests as well.

-gustaf

Collapse
Posted by Gustaf Neumann on
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?

Collapse
Posted by Gustaf Neumann on
rocael, can you check wether for your test cases the lock in content_item new is sufficient as well?
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.
Collapse
Posted by Gustaf Neumann on
today is familiy day. this evening i'll provide a patch/update.
Collapse
Posted by Gustaf Neumann on
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.

Collapse
Posted by Derick Leony on
The patch worked great for one of our development instances with PG8.0.

I tried to modify the SQL script in order to apply the patch on PG 7.4.7 but since the "new" object of the rule can't be sent as a parameter the functions have to be generated with a parameter per column. This led to another problem because there are some revision views (e.g. as_sessionsi) with more than 32 attributes and 32 is the parameter limit for PG functions.

So, the chosen approach was to apply this change to every view with 32 or less attributes. Any advice will be really appreciated, though I think upgrading to PG8.1 would be the best to do :)

Collapse
Posted by Malte Sussdorff on
Can you provide a patch or commit your change to the repository? This would help a lot.