Re: [BUG] [PATCH] pg_basebackup produces wrong incremental files after relation truncation in segmented tables
Oleg Tkachenko <oatkachenko@gmail.com>
From: Oleg Tkachenko <oatkachenko@gmail.com>
To: Robert Haas <robertmhaas@gmail.com>
Cc: Amul Sul <sulamul@gmail.com>,
pgsql-hackers@lists.postgresql.org,
Stanislav Bashkyrtsev <stanislav.bashkyrtsev@elsci.io>
Date: 2025-12-15T18:46:23Z
Lists: pgsql-hackers
Commits
Same data as JSON:
GET /api/v1/messages/:b64id/commits
the thread's linked commits as JSON, with link sources.
API reference →
-
Don't set the truncation block length greater than RELSEG_SIZE.
- ad569b54a106 17.8 landed
- c80b0c9d63b2 18.2 landed
- ecd275718be0 19 (unreleased) landed
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@gmail.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@gmail.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