Hi hackers,
> Please correct me if I'm wrong, but don't we have a race condition here:
>
> ```
> + if ((TransactionIdDidAbort(pred_xmin) ||
> TransactionIdIsInProgress(pred_xmin))
> + && !TransactionIdEquals(pred_xmin, curr_xmin))
> {
> ```
>
> The scenario that concerns me is the following:
>
> 1. TransactionIdDidAbort(pred_xmin) returns false
> 2. The transaction aborts
> 3. TransactionIdIsInProgress(pred_xmin) returns false
> 4. (false || false) gives us false. An error is reported, although
> actually the condition should have been true.
It looks like I had a slight brain fade here.
In order to report a false error either TransactionIdDidAbort() or
TransactionIdIsInProgress() should return true and
TransactionIdEquals() should be false. So actually under rare
conditions the error will NOT be reported while it should. Other than
that we seem to be safe from the concurrency perspective, unless I'm
missing something again.
Personally I don't have a strong opinion on whether we should bother
about this scenario. Probably an explicit comment will not hurt.
Also I suggest checking TransactionIdEquals() first though since it's cheaper.
--
Best regards,
Aleksander Alekseev