pg.ddx.io pgsql-hackers@postgresql.org mailing list archive help / color / mirror / Atom feed From: Nathan Bossart <nathandbossart@gmail.com>
To: Robert Haas <robertmhaas@gmail.com>
Cc: Matthias van de Meent <boekewurm+postgres@gmail.com>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: cleanup patches for incremental backup
Date: Thu, 14 Mar 2024 20:52:55 -0500
Message-ID: <20240315015255.GA3248600@nathanxps13> (raw )
In-Reply-To: <20240314210010.GA3056455@nathanxps13 >
References: <CAEze2WizrxyWg+xeVMuyVqfZc-8U1myy1V4H3zwV-7xXztVf2w@mail.gmail.com >
<CA+TgmoadPUdt8ZJ_Zem-T2_qZ5OTXVdxH6wGtnaV-TunN+LOHQ@mail.gmail.com >
<CAEze2Wjbf==0=v+Ck8B2vcCeqD_3_SfDV3HtpEQiP-r3e2fsSA@mail.gmail.com >
<CA+TgmobTfzrnBesboXutZmrABdP79mO2HhXupY5YOdZ9VLWFog@mail.gmail.com >
<CAEze2WjhdVCqEe_qqEok3NA6DwUdOGSBjAxzmYdAqiaaH1uRcg@mail.gmail.com >
<CA+TgmoaQHtGNFczJzSO5sMobUpuikSHphtFPZpdLmjzwdmM1DQ@mail.gmail.com >
<20240124170846.GA2643050@nathanxps13 >
<CA+TgmoZusu5g3rM1k=UB29Sf53c1OKjm1uFw9uo-sSXBLFZJiQ@mail.gmail.com >
<20240124180515.GA2709722@nathanxps13 >
<20240314210010.GA3056455@nathanxps13 >
On Thu, Mar 14, 2024 at 04:00:10PM -0500, Nathan Bossart wrote:
> Separately, I suppose it's probably time to revert the temporary debugging
> code adding by commit 5ddf997. I can craft a patch for that, too.
As promised...
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
Attachments: download all (2 files, .tar.gz)
v1-0001-Revert-Temporary-patch-to-help-debug-pg_walsummar.patch (text/x-diff, 3.0K) download
From 3923e30acb1d4e21ce311b25edbcd8b96b4223d2 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <nathan@postgresql.org>
Date: Thu, 14 Mar 2024 20:36:48 -0500
Subject: [PATCH v1 1/2] Revert "Temporary patch to help debug pg_walsummary
test failures."
This reverts commit 5ddf9973477729cf161b4ad0a1efd52f4fea9c88.
---
src/backend/backup/walsummary.c | 7 -------
src/bin/pg_walsummary/t/002_blocks.pl | 14 --------------
2 files changed, 21 deletions(-)
diff --git a/src/backend/backup/walsummary.c b/src/backend/backup/walsummary.c
index 4d047e1c02..322ae3c3ad 100644
--- a/src/backend/backup/walsummary.c
+++ b/src/backend/backup/walsummary.c
@@ -252,15 +252,8 @@ RemoveWalSummaryIfOlderThan(WalSummaryFile *ws, time_t cutoff_time)
ereport(ERROR,
(errcode_for_file_access(),
errmsg("could not stat file \"%s\": %m", path)));
- /* XXX temporarily changed to debug buildfarm failures */
-#if 0
ereport(DEBUG2,
(errmsg_internal("removing file \"%s\"", path)));
-#else
- ereport(LOG,
- (errmsg_internal("removing file \"%s\" cutoff_time=%llu", path,
- (unsigned long long) cutoff_time)));
-#endif
}
/*
diff --git a/src/bin/pg_walsummary/t/002_blocks.pl b/src/bin/pg_walsummary/t/002_blocks.pl
index d4bae3d564..52d3bd8840 100644
--- a/src/bin/pg_walsummary/t/002_blocks.pl
+++ b/src/bin/pg_walsummary/t/002_blocks.pl
@@ -51,7 +51,6 @@ my $summarized_lsn = $node1->safe_psql('postgres', <<EOM);
SELECT MAX(end_lsn) AS summarized_lsn FROM pg_available_wal_summaries()
EOM
note("after insert, summarized through $summarized_lsn");
-note_wal_summary_dir("after insert", $node1);
# Update a row in the first block of the table and trigger a checkpoint.
$node1->safe_psql('postgres', <<EOM);
@@ -78,7 +77,6 @@ my @lines = split(/\n/, $details);
is(0+@lines, 1, "got exactly one new WAL summary");
my ($tli, $start_lsn, $end_lsn) = split(/\|/, $lines[0]);
note("examining summary for TLI $tli from $start_lsn to $end_lsn");
-note_wal_summary_dir("after new summary", $node1);
# Reconstruct the full pathname for the WAL summary file.
my $filename = sprintf "%s/pg_wal/summaries/%08s%08s%08s%08s%08s.summary",
@@ -86,7 +84,6 @@ my $filename = sprintf "%s/pg_wal/summaries/%08s%08s%08s%08s%08s.summary",
split(m@/@, $start_lsn),
split(m@/@, $end_lsn);
ok(-f $filename, "WAL summary file exists");
-note_wal_summary_dir("after existence check", $node1);
# Run pg_walsummary on it. We expect exactly two blocks to be modified,
# block 0 and one other.
@@ -96,16 +93,5 @@ note($stdout);
like($stdout, qr/FORK main: block 0$/m, "stdout shows block 0 modified");
is($stderr, '', 'stderr is empty');
is(0+@lines, 2, "UPDATE modified 2 blocks");
-note_wal_summary_dir("after pg_walsummary run", $node1);
done_testing();
-
-# XXX. Temporary debugging code.
-sub note_wal_summary_dir
-{
- my ($flair, $node) = @_;
-
- my $wsdir = sprintf "%s/pg_wal/summaries", $node->data_dir;
- my @wsfiles = grep { $_ ne '.' && $_ ne '..' } slurp_dir($wsdir);
- note("$flair pg_wal/summaries has: @wsfiles");
-}
--
2.25.1
v1-0002-Fix-possible-overflow-in-MaybeRemoveOldWalSummari.patch (text/x-diff, 1.8K) download
From 38dda89ee48736489d37e18dea186e90358468b0 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <nathan@postgresql.org>
Date: Thu, 14 Mar 2024 20:46:02 -0500
Subject: [PATCH v1 2/2] Fix possible overflow in MaybeRemoveOldWalSummaries().
---
src/backend/postmaster/walsummarizer.c | 4 ++--
src/backend/utils/misc/guc_tables.c | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/src/backend/postmaster/walsummarizer.c b/src/backend/postmaster/walsummarizer.c
index ec2874c18c..c820d1f9ed 100644
--- a/src/backend/postmaster/walsummarizer.c
+++ b/src/backend/postmaster/walsummarizer.c
@@ -139,7 +139,7 @@ static XLogRecPtr redo_pointer_at_last_summary_removal = InvalidXLogRecPtr;
* GUC parameters
*/
bool summarize_wal = false;
-int wal_summary_keep_time = 10 * 24 * 60;
+int wal_summary_keep_time = 10 * HOURS_PER_DAY * MINS_PER_HOUR;
static void WalSummarizerShutdown(int code, Datum arg);
static XLogRecPtr GetLatestLSN(TimeLineID *tli);
@@ -1474,7 +1474,7 @@ MaybeRemoveOldWalSummaries(void)
* Files should only be removed if the last modification time precedes the
* cutoff time we compute here.
*/
- cutoff_time = time(NULL) - 60 * wal_summary_keep_time;
+ cutoff_time = time(NULL) - wal_summary_keep_time * SECS_PER_MINUTE;
/* Get all the summaries that currently exist. */
wslist = GetWalSummaries(0, InvalidXLogRecPtr, InvalidXLogRecPtr);
diff --git a/src/backend/utils/misc/guc_tables.c b/src/backend/utils/misc/guc_tables.c
index 57d9de4dd9..1e71e7db4a 100644
--- a/src/backend/utils/misc/guc_tables.c
+++ b/src/backend/utils/misc/guc_tables.c
@@ -3293,9 +3293,9 @@ struct config_int ConfigureNamesInt[] =
GUC_UNIT_MIN,
},
&wal_summary_keep_time,
- 10 * 24 * 60, /* 10 days */
+ 10 * HOURS_PER_DAY * MINS_PER_HOUR, /* 10 days */
0,
- INT_MAX,
+ INT_MAX / SECS_PER_MINUTE,
NULL, NULL, NULL
},
--
2.25.1
view thread (40+ messages) latest in thread
Message-ID: <20240315015255.GA3248600@nathanxps13>
Permalink: ../20240315015255.GA3248600@nathanxps13/
Also on: postgresql.org/message-id/20240315015255.GA3248600@nathanxps13
copy link · copy postgr.es · copy git am
reply Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: nathandbossart@gmail.com, robertmhaas@gmail.com, boekewurm+postgres@gmail.com, pgsql-hackers@lists.postgresql.org
Subject: Re: cleanup patches for incremental backup
In-Reply-To: <20240315015255.GA3248600@nathanxps13>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox