We are moving to Git Issues for bug tracking in future releases. During transition, content will be in both tools. If you'd like to file a new bug, please create an issue.

Bug 8129 - Referral plugin prone to race conditions
Referral plugin prone to race conditions
Product: OJS
Classification: Unclassified
Component: Plug-ins
All All
: P3 normal
Assigned To: PKP Support
Depends on:
  Show dependency treegraph
Reported: 2013-02-22 10:23 PST by Alec Smecher
Modified: 2013-03-25 15:10 PDT (History)
1 user (show)

See Also:
Version Reported In:
Also Affects:


Note You need to log in before you can comment on or make changes to this bug.
Description Alec Smecher 2013-02-22 10:23:00 PST
Currently the referral plugin checks to see if a referral exists, then if not, inserts it. This is prone to race conditions, e.g. two simultaneous-ish requests for the same article from the same URL will lead one to respond with a duplicate entry on index error from MySQL.
Comment 1 Michael Thessel 2013-03-19 10:27:59 PDT
Currently the primary key for the referrals table is referral_id. article_id and url have a UNIQUE constraint. We could change the primary key to url and article_id and use REPLACE INTO when inserting a new referral. This way we should be able to avoid race conditions. Based on how the referral system works I would still keep the referral_id (auto_increment, unique). 

To do so we would need to change the schema of that table. I'm not sure how you handle schema changes during upgrades. Could you please point me in the right direction?

mysql> explain referrals;
| Field       | Type         | Null | Key | Default | Extra          |
| referral_id | bigint(20)   | NO   | PRI | NULL    | auto_increment |
| article_id  | bigint(20)   | NO   | MUL | NULL    |                |
| status      | smallint(6)  | NO   |     | NULL    |                |
| url         | varchar(255) | NO   |     | NULL    |                |
| date_added  | datetime     | NO   |     | NULL    |                |
| link_count  | bigint(20)   | NO   |     | NULL    |                |
Comment 2 Alec Smecher 2013-03-19 14:05:12 PDT
Michael, I'd suggest avoiding schema changes for the moment; the change of indexes wouldn't be material for the race condition fix (if I understand you correctly). You shouldn't use REPLACE INTO directly in your SQL because PostgreSQL doesn't support it; you might be able to use ADODB's Replace() function instead, but in that case, be careful you don't clobber e.g. date_added and link_count if you do.
Comment 3 Michael Thessel 2013-03-21 10:41:18 PDT
I was planning on changing the primary key to be able to use MySQL's REPLACE INTO. This fix would work on MySQL. You are right though, REPLACE INTO is native to MySQL and won't work with PostgreSQL. I will look for a different solution.
Comment 4 Michael Thessel 2013-03-21 11:34:36 PDT
I added a pull request with a proposed fix to GITHub. It utilizes the ADODB replace() function for inserts. The referral plugin checks if the URL already exists before calling insertReferral(). This will prevent issues like resetting the date or the link count.
Comment 5 Michael Thessel 2013-03-22 09:09:00 PDT
I added the requested function name changes to the pull request.
Comment 6 Michael Thessel 2013-03-25 14:55:02 PDT
Renamed function to reflect new functionality
Comment 7 Michael Thessel 2013-03-25 14:55:02 PDT
Whitespace cleanup
Comment 8 Alec Smecher 2013-03-25 15:10:02 PDT
Race condition in referral plugin insert