mirror of
https://github.com/postgres/postgres.git
synced 2026-03-27 04:44:15 -04:00
Prevent restore of incremental backup from bloating VM fork.
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
This commit is contained in:
parent
06d8302262
commit
ffc226ab64
4 changed files with 42 additions and 4 deletions
|
|
@ -116,6 +116,8 @@
|
|||
|
||||
/* 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)
|
||||
|
||||
|
|
@ -600,6 +602,21 @@ visibilitymap_prepare_truncate(Relation rel, BlockNumber nheapblocks)
|
|||
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.
|
||||
*
|
||||
|
|
|
|||
|
|
@ -23,6 +23,7 @@
|
|||
#include "postgres.h"
|
||||
|
||||
#include "access/timeline.h"
|
||||
#include "access/visibilitymap.h"
|
||||
#include "access/xlog.h"
|
||||
#include "access/xlog_internal.h"
|
||||
#include "access/xlogrecovery.h"
|
||||
|
|
@ -1351,7 +1352,8 @@ SummarizeSmgrRecord(XLogReaderState *xlogreader, BlockRefTable *brtab)
|
|||
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));
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -1,7 +1,8 @@
|
|||
# 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';
|
||||
|
|
@ -39,7 +40,7 @@ $primary->safe_psql(
|
|||
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
|
||||
|
|
@ -83,6 +84,23 @@ cmp_ok($t_blocks, '>', $target_blocks,
|
|||
$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');
|
||||
|
|
|
|||
|
|
@ -45,5 +45,6 @@ extern uint8 visibilitymap_get_status(Relation rel, BlockNumber heapBlk, Buffer
|
|||
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 */
|
||||
|
|
|
|||
Loading…
Reference in a new issue