On Sat, Mar 21, 2026 at 5:51 PM Andrew Dunstan <[email protected]> wrote:
>
>
> On 2026-03-21 Sa 2:34 AM, Tom Lane wrote:
>
> Michael Paquier <[email protected]> writes:
>
> On Fri, Mar 20, 2026 at 11:49:02PM -0400, Tom Lane wrote:
>
> Buildfarm members batta and hachi don't like this very much.
>
> I did not look at what's happening on the host, but it seems like a
> safe bet to assume that we are not seeing many failures in the
> buildfarm because we don't have many animals that have the idea to add
> --with-zstd to their build configuration, like these two ones.
>
> That may be part of the story, but only part.  I spent a good deal of
> time trying to reproduce batta & hachi's configurations locally, on
> several different platforms, but still couldn't duplicate what they
> are showing.
>
>
>
>
>
> Yeah, I haven't been able to reproduce it either. But while investigating I 
> found a couple of issues. We neglected to add one of the tests to 
> meson.build, and we neglected to close some files, causing errors on windows.
>

While the proposed fix of closing the file pointer before returning is
correct, we also need to ensure the file is reopened in the next call
to spill any remaining buffered data. I’ve made a small update to
Andrew's 0001 patch to handle this. Also, changes to meson.build don't
seem to be needed as we haven't committed that file yet (unless I am
missing something).

I’ve also reattached the other patches so they don't get lost: v2-0002
is Andrew's patch for the archive streamer, and v2-0003 is the patch I
posted previously [1].


Regards,
Amul

1] 
http://postgr.es/m/caaj_b95l5j7bjrndjrj6wgqfcqeabd+jx3sauxpa4uopqet...@mail.gmail.com
From 322fd5b96e9739937c587460b2780308705f5a83 Mon Sep 17 00:00:00 2001
From: Amul Sul <[email protected]>
Date: Sat, 21 Mar 2026 20:34:15 +0530
Subject: [PATCH v2 1/3] Fix-pg_waldump-archive-reader-file-handle-leak-and-r

---
 src/bin/pg_waldump/archive_waldump.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/src/bin/pg_waldump/archive_waldump.c b/src/bin/pg_waldump/archive_waldump.c
index b078c2d6960..1e9ae637940 100644
--- a/src/bin/pg_waldump/archive_waldump.c
+++ b/src/bin/pg_waldump/archive_waldump.c
@@ -474,7 +474,16 @@ get_archive_wal_entry(const char *fname, XLogDumpPrivate *privateInfo)
 		entry = ArchivedWAL_lookup(privateInfo->archive_wal_htab, fname);
 
 		if (entry != NULL)
+		{
+			/*
+			 * Found the target segment. Close any open spill file handle to
+			 * avoid a leak; any remaining data for that segment will be
+			 * written when the file is reopened in a subsequent call.
+			 */
+			if (write_fp != NULL)
+				fclose(write_fp);
 			return entry;
+		}
 
 		/*
 		 * Capture the current entry before calling read_archive_file(),
@@ -508,8 +517,8 @@ get_archive_wal_entry(const char *fname, XLogDumpPrivate *privateInfo)
 		 */
 		Assert(strcmp(fname, entry->fname) != 0);
 
-		/* Create a temporary file if one does not already exist */
-		if (!entry->spilled)
+		/* Open a spill file for this segment if we haven't already */
+		if (!write_fp)
 		{
 			write_fp = prepare_tmp_write(entry->fname, privateInfo);
 			entry->spilled = true;
@@ -631,7 +640,7 @@ prepare_tmp_write(const char *fname, XLogDumpPrivate *privateInfo)
 	snprintf(fpath, MAXPGPATH, "%s/%s", TmpWalSegDir, fname);
 
 	/* Open the spill file for writing */
-	file = fopen(fpath, PG_BINARY_W);
+	file = fopen(fpath, PG_BINARY_A);
 	if (file == NULL)
 		pg_fatal("could not create file \"%s\": %m", fpath);
 
-- 
2.47.1

From 40e613592ab819c1b8346afe435babf0b212b9ef Mon Sep 17 00:00:00 2001
From: Amul Sul <[email protected]>
Date: Sat, 21 Mar 2026 19:48:48 +0530
Subject: [PATCH v2 2/3] Fix-astreamer-decompressor-finalize-to-send-correct

---
 src/fe_utils/astreamer_gzip.c | 9 +++++----
 src/fe_utils/astreamer_lz4.c  | 9 +++++----
 src/fe_utils/astreamer_zstd.c | 2 +-
 3 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/src/fe_utils/astreamer_gzip.c b/src/fe_utils/astreamer_gzip.c
index 2e080c37a58..df392f67cab 100644
--- a/src/fe_utils/astreamer_gzip.c
+++ b/src/fe_utils/astreamer_gzip.c
@@ -347,10 +347,11 @@ astreamer_gzip_decompressor_finalize(astreamer *streamer)
 	 * End of the stream, if there is some pending data in output buffers then
 	 * we must forward it to next streamer.
 	 */
-	astreamer_content(mystreamer->base.bbs_next, NULL,
-					  mystreamer->base.bbs_buffer.data,
-					  mystreamer->base.bbs_buffer.maxlen,
-					  ASTREAMER_UNKNOWN);
+	if (mystreamer->bytes_written > 0)
+		astreamer_content(mystreamer->base.bbs_next, NULL,
+						  mystreamer->base.bbs_buffer.data,
+						  mystreamer->bytes_written,
+						  ASTREAMER_UNKNOWN);
 
 	astreamer_finalize(mystreamer->base.bbs_next);
 }
diff --git a/src/fe_utils/astreamer_lz4.c b/src/fe_utils/astreamer_lz4.c
index 2bc32b42879..605c188007b 100644
--- a/src/fe_utils/astreamer_lz4.c
+++ b/src/fe_utils/astreamer_lz4.c
@@ -397,10 +397,11 @@ astreamer_lz4_decompressor_finalize(astreamer *streamer)
 	 * End of the stream, if there is some pending data in output buffers then
 	 * we must forward it to next streamer.
 	 */
-	astreamer_content(mystreamer->base.bbs_next, NULL,
-					  mystreamer->base.bbs_buffer.data,
-					  mystreamer->base.bbs_buffer.maxlen,
-					  ASTREAMER_UNKNOWN);
+	if (mystreamer->bytes_written > 0)
+		astreamer_content(mystreamer->base.bbs_next, NULL,
+						  mystreamer->base.bbs_buffer.data,
+						  mystreamer->bytes_written,
+						  ASTREAMER_UNKNOWN);
 
 	astreamer_finalize(mystreamer->base.bbs_next);
 }
diff --git a/src/fe_utils/astreamer_zstd.c b/src/fe_utils/astreamer_zstd.c
index f26abcfd0fa..4b43ab795e3 100644
--- a/src/fe_utils/astreamer_zstd.c
+++ b/src/fe_utils/astreamer_zstd.c
@@ -347,7 +347,7 @@ astreamer_zstd_decompressor_finalize(astreamer *streamer)
 	if (mystreamer->zstd_outBuf.pos > 0)
 		astreamer_content(mystreamer->base.bbs_next, NULL,
 						  mystreamer->base.bbs_buffer.data,
-						  mystreamer->base.bbs_buffer.maxlen,
+						  mystreamer->zstd_outBuf.pos,
 						  ASTREAMER_UNKNOWN);
 
 	astreamer_finalize(mystreamer->base.bbs_next);
-- 
2.47.1

From a62de1b7b467a037651a2e1bb3820a390227ce78 Mon Sep 17 00:00:00 2001
From: Amul Sul <[email protected]>
Date: Sat, 21 Mar 2026 20:57:23 +0530
Subject: [PATCH v2 3/3] pg_waldump: Handle archive exhaustion in
 init_archive_reader().

When read_archive_file() returns 0, the archive may have already
buffered a complete WAL file into the hash table before exhausting
the input.  Instead of immediately reporting an error, search the
hash table for an entry containing at least sizeof(XLogLongPageHeader)
bytes.  Report a specific error if a WAL entry exists but is too
short (truncated/corrupt), or a generic error if no WAL was found
at all.

Also tighten the loop condition to check for sizeof(XLogLongPageHeader)
rather than XLOG_BLCKSZ, since only the long page header is needed
at this stage.
---
 src/bin/pg_waldump/archive_waldump.c | 55 ++++++++++++++++++++++++++--
 1 file changed, 51 insertions(+), 4 deletions(-)

diff --git a/src/bin/pg_waldump/archive_waldump.c b/src/bin/pg_waldump/archive_waldump.c
index 1e9ae637940..dbc1751fb3c 100644
--- a/src/bin/pg_waldump/archive_waldump.c
+++ b/src/bin/pg_waldump/archive_waldump.c
@@ -176,13 +176,60 @@ init_archive_reader(XLogDumpPrivate *privateInfo,
 	 * the first WAL segment in the archive so we can extract the WAL segment
 	 * size from the long page header.
 	 */
-	while (entry == NULL || entry->buf->len < XLOG_BLCKSZ)
+	while (entry == NULL || entry->read_len < sizeof(XLogLongPageHeader))
 	{
 		if (read_archive_file(privateInfo, XLOG_BLCKSZ) == 0)
-			pg_fatal("could not find WAL in archive \"%s\"",
-					 privateInfo->archive_name);
+		{
+			ArchivedWAL_iterator iter;
+			ArchivedWALFile *e = NULL;
+			ArchivedWALFile *short_entry = NULL;
 
-		entry = privateInfo->cur_file;
+			entry = NULL;
+
+			/*
+			 * read_archive_file() returned 0, meaning the archive is
+			 * exhausted.  However, a sufficiently compressed archive may have
+			 * already read a complete WAL file and inserted it into the hash
+			 * table before returning.  Search the hash table for any entry
+			 * that already has enough buffered data to contain the long page
+			 * header; if none is found, the archive contains no usable WAL.
+			 */
+			ArchivedWAL_start_iterate(privateInfo->archive_wal_htab, &iter);
+			while ((e = ArchivedWAL_iterate(privateInfo->archive_wal_htab,
+											&iter)) != NULL)
+			{
+				if (e->read_len >= sizeof(XLogLongPageHeader))
+				{
+					entry = e;
+					break;
+				}
+				/* Remember a short entry in case we need to report it */
+				short_entry = e;
+			}
+
+			if (entry == NULL)
+			{
+				/*
+				 * A WAL file was found in the hash table but it does not
+				 * contain enough data to read the long page header,
+				 * indicating a truncated or corrupt WAL segment.
+				 */
+				if (short_entry != NULL)
+					pg_fatal("could not read file \"%s\" from \"%s\" archive: read %d of %d",
+							 short_entry->fname, privateInfo->archive_name,
+							 short_entry->read_len,
+							 (int) sizeof(XLogLongPageHeader));
+
+				/*
+				 * The hash table contains no WAL entries at all, meaning the
+				 * archive holds no WAL data.
+				 */
+				pg_fatal("could not find WAL in archive \"%s\"",
+						 privateInfo->archive_name);
+			}
+		}
+		else
+			entry = privateInfo->cur_file;
 	}
 
 	/* Extract the WAL segment size from the long page header */
-- 
2.47.1

Reply via email to