Re: HOT chain validation in verify_heapam()

On Wed, Nov 16, 2022 at 11:23 PM Robert Haas <robertmhaas@gmail.com> wrote:

> 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.


-- 
Regards,
Himanshu Upadhyaya
EnterpriseDB: http://www.enterprisedb.com