Re: [BUG] [PATCH] pg_basebackup produces wrong incremental files after relation truncation in segmented tables

From: Oleg Tkachenko <oatkachenko(at)gmail(dot)com>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: Amul Sul <sulamul(at)gmail(dot)com>, pgsql-hackers(at)lists(dot)postgresql(dot)org, Stanislav Bashkyrtsev <stanislav(dot)bashkyrtsev(at)elsci(dot)io>
Subject: Re: [BUG] [PATCH] pg_basebackup produces wrong incremental files after relation truncation in segmented tables
Date: 2025-12-15 18:46:23
Message-ID: 22474984-C4CD-4300-A84E-C1CA58F61CDA@gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hello Robert,

Thank you for the explanation.

At first, I also thought about clamping truncation_block_length to RELSEG_SIZE. But I hesitated because I thought the reconstructed relation file couldn’t be larger than relative_limit.

After reading the reconstruction code and the comments on top of the discussed block of code (many times), I finally understood that truncation_block_length is the minimum length of the reconstructed file, not just a safety limit. It determines which blocks must be fetched from older backups. So a simple clamp could change how reconstruction works if some blocks are included in incremental backups.

I’ve tested the version with the limit enforced to RELSEG_SIZE, and it works correctly.

Also, I’ve attached a patch based on your guidance. The changes are effectively the same as your suggested approach, but I would be happy to be listed as a contributor.

Regards,
Oleg Tkachenko


> On Dec 15, 2025, at 17:35, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
>
> [ sorry for not noticing this thread sooner; thanks to Andres for
> pointing me to it ]
>
> On Mon, Dec 15, 2025 at 9:01 AM Amul Sul <sulamul(at)gmail(dot)com> wrote:
>> Thanks for the reproducer; I can see the reported issue, but I am not
>> quite sure the proposed fix is correct and might break other cases (I
>> haven't tried constructed that case yet) but there is a comment
>> detailing that case just before the point where you are planning to do
>> the changes:
>>
>> /*
>> * The truncation block length is the minimum length of the reconstructed
>> * file. Any block numbers below this threshold that are not present in
>> * the backup need to be fetched from the prior backup. At or above this
>> * threshold, blocks should only be included in the result if they are
>> * present in the backup. (This may require inserting zero blocks if the
>> * blocks included in the backup are non-consecutive.)
>> */
>>
>> IIUC, we might need the original assignment logic as it is. But we
>> need to ensure that truncation_block_length is not set to a value that
>> exceeds RELSEG_SIZE.
>
> I think you're right. By way of example, let's say that the current
> length of the file is 200 blocks, but the limit block is 100 blocks
> into the current segment. That means that the only blocks that we can
> get from any previous backup are blocks 0-99. Blocks 100-199 of the
> current segment are either mentioned in the WAL summaries we're using
> for this backup, or they're all zeroes. We can't set the
> truncation_block_length to a value greater than 100, or we'll go
> looking for the contents of any zero-filled blocks in previous
> backups, will will either fail or produce the wrong answer. But Oleg
> is correct that we also shouldn't set it to a value greater than
> RELSEG_SIZE. So my guess is that the correct fix might be something
> like the attached (untested, for discussion).
>
> --
> Robert Haas
> EDB: http://www.enterprisedb.com

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Joao Foltran 2025-12-15 18:54:23 [BUG] [PATCH] Allow physical replication slots to recover from archive after invalidation
Previous Message Noah Misch 2025-12-15 18:30:18 Re: pg_dump crash due to incomplete ordering of DO_SUBSCRIPTION_REL objects