|
PKP Bugzilla – Full Text Bug Listing |
| Summary: | Referral plugin prone to race conditions | ||
|---|---|---|---|
| Product: | OJS | Reporter: | Alec Smecher <alec> |
| Component: | Plug-ins | Assignee: | 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 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 | | +-------------+--------------+------+-----+---------+----------------+ 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. 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. 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. I added the requested function name changes to the pull request. Renamed function to reflect new functionality https://github.com/pkp/ojs/commit/936b78b11ba0bf89e2022a2830d022994e687163 Whitespace cleanup https://github.com/pkp/ojs/commit/1607437f27b4689d994a07d08d06fe300bb79857 Race condition in referral plugin insert https://github.com/pkp/ojs/commit/185b208fe6dc7fedb683b00adfaa077c4ec5a882 |