#398 Concurrent updates/deletes can cause data inconsistency

[Amit Khandekar]

In the UPDATE implemention in XC, once we get the source data alongwith ctids, we start updating the records, but we don't issue a row lock, so we can end up updating a totally different row if it is already updated by some other update that happened in between fetching the rows and updating them. And this results in data inconsistency. Below is an example:

-------
postgres=# \d student
Table "public.student"
Column | Type | Modifiers
--------+--------------+-----------
id | integer |
status | character(1) |
marks | integer |

Status is 'f' for marks < 35 , 'p' for marks >= 35 , and 'u' for unknown.

postgres=# select * from student ;
id | status | marks
----+--------+-------
1 | u | 80

App1 wants to correct the status to 'p', so it invokes :
postgres=# update student set status = 'p' where marks > 35;

Just when the above command has fetched the ctid of the source data and is about to update the row belonging to that ctid, App2 wants to update the marks of the student, so it invokes :
postgres=# begin;
BEGIN
postgres=# update student set status = 'f', marks = 30 where id = 1;
UPDATE 1
postgres=# select * from student ;
id | status | marks
----+--------+-------
1 | f | 30

And now the command of App1 continues with the status update using "SET status = 'p' ". But it updates the new version in the ctid chain, and not the one that it has fetched. So the resultant data is:
id | status | marks
----+--------+-------
1 | p | 30
So the marks are < 35, but status is 'p' . This is not expected. PG works well. PG correctly prevents the App1 update from happening because it again applies the "WHERE marks > 35" filter on the new version that was updated by App2, and so it does not update that row at all.

The same issue must be there for DELETE: a DELETE will delete a wrong record if the original record that it intended to delete is concurrently updated by someone else.

[Amit Khandekar]

Here is one way we can fix :
The scan subplan for UPDATE should use FOR UDPATE if there are no coordinator quals.
If there are coordinator quals, there would be two nodes:
The upper node would do :
SELECT * from tab1 where ctid in (.....) FOR UPDATE
The lower node would perhaps be a SubPlan node which would do:
select ctid from tab1
Coordinator filter .....
The lower node would supply the ctids for the IN clause of upper node.

The above will ensure that when it comes to updating the rows, those rows will already be locked because of FOR UPDATE.

[Koichi Suzuki]

Would like to determine if we can fix this in 1.2 or later in the F2F meeting, Sept, 2013.

[Amit Khandekar]

There is a precise mention of this scenario in PostgreSQL documentation:

"UPDATE, DELETE, SELECT FOR UPDATE, and SELECT FOR SHARE commands behave the same as SELECT in terms of searching for target rows: they will only find target rows that were committed as of the command start time. However, such a target row might have already been updated (or deleted or locked) by another concurrent transaction by the time it is found. In this case, the would-be updater will wait for the first updating transaction to commit or roll back (if it is still in progress). If the first updater rolls back, then its effects are negated and the second updater can proceed with updating the originally found row. If the first updater commits, the second updater will ignore the row if the first updater deleted it, otherwise it will attempt to apply its operation to the updated version of the row. The search condition of the command (the WHERE clause) is re-evaluated to see if the updated version of the row still matches the search condition. If so, the second updater proceeds with its operation using the updated version of the row. In the case of SELECT FOR UPDATE and SELECT FOR SHARE, this means it is the updated version of the row that is locked and returned to the client."

In Postgres-XC, if the first updater commits, the second updater does not re-apply the WHERE clause , and then does its operation (either delete, update or trigger func) using the OLD values. Not re-applying the WHERE clause is the main reason why this issue occurs.

[Koichi Suzuki]

Mason submitted a patch to use primary key if available. The patch has not been tested yet.

[Koichi Suzuki]

Use of ctid has been removed in usual GUC settings.