When I (rhaas) wrote the WAL summarizer code, I incorrectly believed
that XLOG_SMGR_TRUNCATE truncates all forks to the same length. In
fact, what other parts of the code do is compute the truncation length
for the FSM and VM forks from the truncation length used for the main
fork. But, because I was confused, I coded the WAL summarizer to set the
limit block for the VM fork to the same value as for the main fork.
(Incremental backup always copies FSM forks in full, so there is no
similar issue in that case.)
Doing that doesn't directly cause any data corruption, as far as I can
see. However, it does create a serious risk of consuming a large amount
of extra disk space, because pg_combinebackup's reconstruct.c believes
that the reconstructed file should always be at least as long as the
limit block value. We might want to be smarter about that at some point
in the future, because it's always safe to omit all-zeroes blocks at the
end of the last segment of a relation, and doing so could save disk
space, but the current algorithm will rarely waste enough disk space to
worry about unless we believe that a relation has been truncated to a
length much longer than its actual length on disk, which is exactly what
happens as a result of the problem mentioned in the previous paragraph.
To fix, create a new visibilitymap helper function and use it to include
the right limit block in the summary files. Incremental backups taken
with existing summary files will still have this issue, but this should
improve the situation going forward.
Diagnosed-by: Oleg Tkachenko <oatkachenko@gmail.com>
Diagnosed-by: Amul Sul <sulamul@gmail.com>
Discussion: http://postgr.es/m/CAAJ_b97PqG89hvPNJ8cGwmk94gJ9KOf_pLsowUyQGZgJY32o9g@mail.gmail.com
Discussion: http://postgr.es/m/
6897DAF7-B699-41BF-A6FB-
B818FCFFD585%40gmail.com
Backpatch-through: 17
/* Mapping from heap block number to the right bit in the visibility map */
#define HEAPBLK_TO_MAPBLOCK(x) ((x) / HEAPBLOCKS_PER_PAGE)
+#define HEAPBLK_TO_MAPBLOCK_LIMIT(x) \
+ (((x) + HEAPBLOCKS_PER_PAGE - 1) / HEAPBLOCKS_PER_PAGE)
#define HEAPBLK_TO_MAPBYTE(x) (((x) % HEAPBLOCKS_PER_PAGE) / HEAPBLOCKS_PER_BYTE)
#define HEAPBLK_TO_OFFSET(x) (((x) % HEAPBLOCKS_PER_BYTE) * BITS_PER_HEAPBLOCK)
return newnblocks;
}
+/*
+ * visibilitymap_truncation_length -
+ * compute truncation length for visibility map
+ *
+ * Given a proposed truncation length for the main fork, compute the
+ * correct truncation length for the visibility map. Should return the
+ * same answer as visibilitymap_prepare_truncate(), but without modifying
+ * anything.
+ */
+BlockNumber
+visibilitymap_truncation_length(BlockNumber nheapblocks)
+{
+ return HEAPBLK_TO_MAPBLOCK_LIMIT(nheapblocks);
+}
+
/*
* Read a visibility map page.
*
#include "postgres.h"
#include "access/timeline.h"
+#include "access/visibilitymap.h"
#include "access/xlog.h"
#include "access/xlog_internal.h"
#include "access/xlogrecovery.h"
MAIN_FORKNUM, xlrec->blkno);
if ((xlrec->flags & SMGR_TRUNCATE_VM) != 0)
BlockRefTableSetLimitBlock(brtab, &xlrec->rlocator,
- VISIBILITYMAP_FORKNUM, xlrec->blkno);
+ VISIBILITYMAP_FORKNUM,
+ visibilitymap_truncation_length(xlrec->blkno));
}
}
# Copyright (c) 2025-2026, PostgreSQL Global Development Group
#
-# This test aims to validate that the calculated truncation block never exceeds
-# the segment size.
+# This test aims to validate two things: (1) that the calculated truncation
+# block never exceeds the segment size and (2) that the correct limit block
+# length is calculated for the VM fork.
use strict;
use warnings FATAL => 'all';
CREATE TABLE t (
id int,
data text STORAGE PLAIN
- );
+ ) WITH (autovacuum_enabled = false);
});
# The tuple size should be enough to prevent two tuples from being on the same
$primary->backup('incr',
backup_options => [ '--incremental', "$full_backup/backup_manifest" ]);
+# We used to have a bug where the wrong limit block was calculated for the
+# VM fork, so verify that the WAL summary records the correct VM fork
+# truncation limit. We can't just check whether the restored VM fork is
+# the right size on disk, because it's so small that the incremental backup
+# code will send the entire file.
+my $relfilenode = $primary->safe_psql('postgres',
+ "SELECT pg_relation_filenode('t');");
+my $vm_limits = $primary->safe_psql('postgres',
+ "SELECT string_agg(relblocknumber::text, ',')
+ FROM pg_available_wal_summaries() s,
+ pg_wal_summary_contents(s.tli, s.start_lsn, s.end_lsn) c
+ WHERE c.relfilenode = $relfilenode
+ AND c.relforknumber = 2
+ AND c.is_limit_block;");
+is($vm_limits, '1',
+ 'WAL summary has correct VM fork truncation limit');
+
# Combine full and incremental backups. Before the fix, this failed because
# the INCREMENTAL file header contained an incorrect truncation_block value.
my $restored = PostgreSQL::Test::Cluster->new('node2');
extern void visibilitymap_count(Relation rel, BlockNumber *all_visible, BlockNumber *all_frozen);
extern BlockNumber visibilitymap_prepare_truncate(Relation rel,
BlockNumber nheapblocks);
+extern BlockNumber visibilitymap_truncation_length(BlockNumber nheapblocks);
#endif /* VISIBILITYMAP_H */
projects / thirdparty / postgresql.git / commitdiff
