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

Summary: Referral plugin prone to race conditions
Product: OJS Reporter: Alec Smecher <alec>
Component: Plug-insAssignee: PKP Support <pkp-support>
Status: RESOLVED FIXED    
Severity: normal CC: michael.thessel
Priority: P3    
Version: 2.4.3   
Hardware: All   
OS: All   
Version Reported In: Also Affects:

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
https://github.com/pkp/ojs/commit/936b78b11ba0bf89e2022a2830d022994e687163
Comment 7 Michael Thessel 2013-03-25 14:55:02 PDT
Whitespace cleanup
https://github.com/pkp/ojs/commit/1607437f27b4689d994a07d08d06fe300bb79857
Comment 8 Alec Smecher 2013-03-25 15:10:02 PDT
Race condition in referral plugin insert
https://github.com/pkp/ojs/commit/185b208fe6dc7fedb683b00adfaa077c4ec5a882