Обсуждение: Fix possible 'unexpected data beyond EOF' on replica restart
Hi,
On restart, a replica can fail with an 'unexpected data beyond EOF in block x of relation T/D/R' error. This happened on a PG17.7 and I've been able to reproduce it on PG 18. This can happen under the following circumstances:
- A relation has a size of 400 blocks.
- Blocks 201 to 400 are empty.
- Block 200 has two rows.
- Blocks 101 to 199 are empty.
- A restartpoint is done
- Vacuum truncates the relation to 200 blocks
- A FPW deletes a row in block 200
- A checkpoint is done
- A FPW deletes the last row in block 200
- Vacuum truncates the relation to 100 blocks
- The replica restarts
When the replica restarts:
- The relation on disk is reduced to 100 blocks due to having applied the truncate before restart.
- The first truncate to 200 blocks is replayed. It silently fails in mdtruncate since 'nblocks > curnblk', but the caller isn't aware of that and will still update the cached size to 200 blocks
- The first FPW on block 200 is applied, XLogReadBufferForRead will rely on the cached size and incorrectly assume the page exists in file, and thus won't extend the relation.
- The Checkpoint Online is replayed, calling smgrdestroyall which will discard the cached size.
- The second FPW on block 200 is applied. This time, the detected size is 100 blocks, an extend is attempted. However, the block 200 is already present in the buffer table due to the first FPW. This triggers the 'unexpected data beyond EOF' since the page isn't new.
On restart, a replica can fail with an 'unexpected data beyond EOF in block x of relation T/D/R' error. This happened on a PG17.7 and I've been able to reproduce it on PG 18. This can happen under the following circumstances:
- A relation has a size of 400 blocks.
- Blocks 201 to 400 are empty.
- Block 200 has two rows.
- Blocks 101 to 199 are empty.
- A restartpoint is done
- Vacuum truncates the relation to 200 blocks
- A FPW deletes a row in block 200
- A checkpoint is done
- A FPW deletes the last row in block 200
- Vacuum truncates the relation to 100 blocks
- The replica restarts
When the replica restarts:
- The relation on disk is reduced to 100 blocks due to having applied the truncate before restart.
- The first truncate to 200 blocks is replayed. It silently fails in mdtruncate since 'nblocks > curnblk', but the caller isn't aware of that and will still update the cached size to 200 blocks
- The first FPW on block 200 is applied, XLogReadBufferForRead will rely on the cached size and incorrectly assume the page exists in file, and thus won't extend the relation.
- The Checkpoint Online is replayed, calling smgrdestroyall which will discard the cached size.
- The second FPW on block 200 is applied. This time, the detected size is 100 blocks, an extend is attempted. However, the block 200 is already present in the buffer table due to the first FPW. This triggers the 'unexpected data beyond EOF' since the page isn't new.
The issue can be reproduced with the following script:
"""
pgbench -i
# Prepare the relation
psql -c "DELETE FROM pgbench_accounts WHERE aid > 80000 AND aid != ALL('{90000, 90001}');"
psql -c "VACUUM (VERBOSE, INDEX_CLEANUP ON, TRUNCATE OFF) pgbench_accounts;"
psql -c "VACUUM (VERBOSE, INDEX_CLEANUP ON, TRUNCATE OFF) pgbench_accounts;"
# Restartpoint here
psql -c "CHECKPOINT;"
psql -p 5433 -c "CHECKPOINT;"
# First truncate
psql -c "VACUUM (VERBOSE, INDEX_CLEANUP ON, TRUNCATE ON) pgbench_accounts;"
# First FPW deletion
psql -c "DELETE FROM pgbench_accounts WHERE aid = 90001;"
# Second FPW deletion
psql -c "CHECKPOINT;"
psql -c "DELETE FROM pgbench_accounts WHERE aid = 90000;"
# Second truncate
psql -c "VACUUM (VERBOSE, INDEX_CLEANUP ON, TRUNCATE ON) pgbench_accounts;"
# Let some time for replica to replay the truncate
psql -c "SELECT pg_sleep(1);"
# Stop without advancing the restartpoint
kill -9 $(pgrep -f "pg_data_replica")
# Restart should fail with the EOF error
pg_ctl -D pg_data_replica restart
psql -c "DELETE FROM pgbench_accounts WHERE aid = 90000;"
# Second truncate
psql -c "VACUUM (VERBOSE, INDEX_CLEANUP ON, TRUNCATE ON) pgbench_accounts;"
# Let some time for replica to replay the truncate
psql -c "SELECT pg_sleep(1);"
# Stop without advancing the restartpoint
kill -9 $(pgrep -f "pg_data_replica")
# Restart should fail with the EOF error
pg_ctl -D pg_data_replica restart
"""
This assumes the replica is running on port 5433 and no hot_standby_feedback (otherwise, tuples will be seen as 'not yet removable'). I've used kill -9 to avoid advancing the restart point, but I've seen the issue happening with a clean shutdown.
The patch fixes the issue by moving smgr_cached_nblocks updates in mdtruncate and only updating the cached value if truncate was successful.
Regards,
Anthonin Bonnefoy
Вложения
On Tue, Dec 16, 2025 at 6:38 PM Anthonin Bonnefoy
<anthonin.bonnefoy@datadoghq.com> wrote:
>
> [...]
> The patch fixes the issue by moving smgr_cached_nblocks updates in mdtruncate and only updating the cached value if
truncatewas successful.
>
Thanks for detailed reproducible steps, I can see the reported issue
and proposed patch fixes the same. Patch looks good to me except
following changes in smgrtruncate():
- /* Make the cached size is invalid if we encounter an error. */
- reln->smgr_cached_nblocks[forknum[i]] = InvalidBlockNumber;
-
smgrsw[reln->smgr_which].smgr_truncate(reln, forknum[i],
old_nblocks[i], nblocks[i]);
The deleted code you moved to mdtruncate() should be kept where it
was. We cannot ensure that every extension using this interface will
adhere to that requirement what the comment describes. Furthermore,
an extension's routine might miss updating smgr_cached_nblocks.
To ensure that updates smgr_cached_nblocks properly, we should also
add an assertion after the call, like this:
/*
* Ensure that the local smgr_cached_nblocks value is updated.
*/
Assert(reln->smgr_cached_nblocks[forknum[i]] != InvalidBlockNumber);
Regards,
Amul
On Wed, Dec 17, 2025 at 8:26 AM Amul Sul <sulamul@gmail.com> wrote:
Thanks for detailed reproducible steps, I can see the reported issue
and proposed patch fixes the same.
Thanks for the review!
The deleted code you moved to mdtruncate() should be kept where it
was. We cannot ensure that every extension using this interface will
adhere to that requirement what the comment describes.
Yeah, I've overlooked the case of extensions. I've moved back the InvalidBlockNumber assignment.
Furthermore, an extension's routine might miss updating
smgr_cached_nblocks. To ensure that updates smgr_cached_nblocks
properly, we should also add an assertion after the call, like this:
/*
* Ensure that the local smgr_cached_nblocks value is updated.
*/
Assert(reln->smgr_cached_nblocks[forknum[i]] != InvalidBlockNumber);
Good point. I've added the assertion.
I wonder how critical it is to have an up to date value of smgr_cached_nblocks after smgr_truncate. Leaving InvalidBlockNumber was also an option as the next user will ask the kernel for the real size. There are some functions like DropRelationBuffers which rely only on the cached value, so it's probably safer to keep the same behaviour.
Regards,
Anthonin Bonnefoy