Horton

Re: HOT chain validation in verify_heapam()

Himanshu Upadhyaya <upadhyaya.himanshu@gmail.com>

View thread View on postgr.es
From: Himanshu Upadhyaya <upadhyaya.himanshu@gmail.com>
To: Aleksander Alekseev <aleksander@timescale.com>
Cc: PostgreSQL Hackers <pgsql-hackers@postgresql.org>, Robert Haas <robertmhaas@gmail.com>
Date: 2022-09-19T16:20:40Z
Lists: pgsql-hackers

Attachments

On Mon, Sep 19, 2022 at 8:27 PM Aleksander Alekseev <
aleksander@timescale.com> wrote:

> Hi Himanshu,
>
> > Done, updated in the v3 patch.
>
> Thanks for the updated patch.
>
> Here is v4 with fixed compiler warnings and some minor tweaks from me.
>
> I didn't put too much thought into the algorithm but I already see
> something strange. At verify_heapam.c:553 you declared curr_xmax and
> next_xmin. However the variables are not used/initialized until you
> do:
>
> ```
>             if (lp_valid[nextoffnum] && lp_valid[ctx.offnum] &&
>                 TransactionIdIsValid(curr_xmax) &&
>                 TransactionIdEquals(curr_xmax, next_xmin)) {
> /* ... */
> ```
>
> In v4 I elected to initialize both curr_xmax and next_xmin with
> InvalidTransactionId for safety and in order to silence the compiler
> but still there is no way this condition can succeed.
>
> Please make sure there is no logic missing.
>
>
Hi Aleksander,

Thanks for sharing the feedback,
It's my mistake, sorry about that, I was trying to merge two if conditions
and forgot to move the initialization part for xmin and xmax. Now I think
that it will be good to have nested if, and have an inner if condition to
test xmax and xmin matching. This way we can retrieve and populate xmin and
xmax when it is actually required for the inner if condition.
I have changed this in the attached patch.

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

Commits

  1. amcheck: Generalize one of the recently-added update chain checks.

  2. amcheck: Tighten up validation of redirect line pointers.

  3. amcheck: Fix verify_heapam for tuples where xmin or xmax is 0.

  4. amcheck: Fix a few bugs in new update chain validation.

  5. Fix new test case to work on (some?) big-endian architectures.

  6. Don't test HEAP_XMAX_INVALID when freezing xmax.