On Wed, Nov 16, 2022 at 4:51 AM Himanshu Upadhyaya <upadhyaya.himanshu@gmail.com> wrote: > yes, got it, have tried to test and it is giving false corruption in case of subtransaction. > I think a better way to have this check is, we need to check that if pred_xmin is > aborted then current_xmin should be aborted only. So there is no way that we > validate corruption with in_progress txid.
Please note that you can't use TransactionIdDidAbort here, because that will return false for transactions aborted by a crash. You have to check that it's not in progress and then afterwards check that it's not committed. Also note that if you check whether it's committed first and then check whether it's in progress afterwards, there's a race condition: it might commit just after you verify that it isn't committed yet, and then it won't be in progress any more and will look aborted.
I disagree with the idea that we can't check in progress. I think the checks could look something like this:
pred_in_progress = TransactionIdIsInProgress(pred_xmin); current_in_progress = TransactionIdIsInProgress(current_xmin); if (pred_in_progress) { if (current_in_progress) return ok; // recheck to avoid race condition if (TransactionIdIsInProgress(pred_xmin)) { if (TransactionIdDidCommit(current_xmin)) return corruption: predecessor xmin in progress, but current xmin committed; else return corruption: predecessor xmin in progress, but current xmin aborted; }
I think we can have a situation where pred_xmin is in progress but curr_xmin is aborted, consider below example:
‘postgres[14723]=#’BEGIN;
BEGIN ‘postgres[14723]=#*’insert into test2 values (1,1); INSERT 0 1 ‘postgres[14723]=#*’savepoint s1; SAVEPOINT ‘postgres[14723]=#*’update test2 set a =2; UPDATE 1 ‘postgres[14723]=#*’rollback to savepoint s1; ROLLBACK
Now pred_xmin is in progress but curr_xmin is aborted, am I missing anything here?
I think if pred_xmin is aborted and curr_xmin is in progress we should consider it as a corruption case but vice versa is not true.
