Horton
Menu

Re: pg_waldump: support decoding of WAL inside tarfile

amul sul <sulamul@gmail.com>

View thread
From: Amul Sul <sulamul@gmail.com>
To: Robert Haas <robertmhaas@gmail.com>
Cc: Chao Li <li.evan.chao@gmail.com>, Jakub Wartak <jakub.wartak@enterprisedb.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2026-02-10T09:36:12Z
Lists: pgsql-hackers

Attachments

On Wed, Feb 4, 2026 at 6:39 PM Amul Sul <sulamul@gmail.com> wrote:
>
> On Wed, Jan 28, 2026 at 2:41 AM Robert Haas <robertmhaas@gmail.com> wrote:
> >
> > On Tue, Jan 27, 2026 at 7:07 AM Amul Sul <sulamul@gmail.com> wrote:
> > > In the attached version, I am using the WAL segment name as the hash
> > > key, which is much more straightforward. I have rewritten
> > > read_archive_wal_page(), and it looks much cleaner than before. The
> > > logic to discard irrelevant WAL files is still within
> > > get_archive_wal_entry. I added an explanation for setting cur_wal to
> > > NULL, which is now handled in the separate function I mentioned
> > > previously.
> > >
> > > Kindly have a look at the attached version; let me know if you are
> > > still not happy with the current approach for filtering/discarding
> > > irrelevant WAL segments. It isn't much different from the previous
> > > version, but I have tried to keep it in a separate routine for better
> > > code readability, with comments to make it easier to understand. I
> > > also added a comment for ArchivedWALFile.
> >
> > I feel like the division of labor between get_archive_wal_entry() and
> > read_archive_wal_page() is odd. I noticed this in the last version,
> > too, and it still seems to be the case. get_archive_wal_entry() first
> > calls ArchivedWAL_lookup(). If that finds an entry, it just returns.
> > If it doesn't, it loops until an entry for the requested file shows up
> > and then returns it. Then control returns to read_archive_wal_page()
> > which loops some more until we have all the data we need for the
> > requested file. But it seems odd to me to have two separate loops
> > here. I think that the first loop is going to call read_archive_file()
> > until we find the beginning of the file that we care about and then
> > the second one is going to call read_archive_file() some more until we
> > have read enough of it to satisfy the request. It feels odd to me to
> > do it that way, as if we told somebody to first wait until 9 o'clock
> > and then wait another 30 minutes, instead of just telling them to wait
> > until 9:30. I realize it's not quite the same thing, because apart
> > from calling read_archive_file(), the two loops do different things,
> > but I still think it looks odd.
> >
> > + /*
> > + * Ignore if the timeline is different or the current segment is not
> > + * the desired one.
> > + */
> > + XLogFromFileName(entry->fname, &curSegTimeline, &curSegNo, WalSegSz);
> > + if (privateInfo->timeline != curSegTimeline ||
> > + privateInfo->startSegNo > curSegNo ||
> > + privateInfo->endSegNo < curSegNo ||
> > + segno > curSegNo)
> > + {
> > + free_archive_wal_entry(entry->fname, privateInfo);
> > + continue;
> > + }
> >
> > The comment doesn't match the code. If it did, the test would be
> > (privateInfo->timeline != curSegTimeline || segno != curSegno). But
> > instead the segno test is > rather than !=, and the checks against
> > startSegNo and endSegNo aren't explained at all. I think I understand
> > why the segno test uses > rather than !=, but it's the point of the
> > comment to explain things like that, rather than leaving the reader to
> > guess. And I don't know why we also need to test startSegNo and
> > endSegNo.
> >
> > I also wonder what the point is of doing XLogFromFileName() on the
> > fname provided by the caller and then again on entry->fname. Couldn't
> > you just compare the strings?
> >
> > Again, the division of labor is really odd here. It's the job of
> > astreamer_waldump_content() to skip things that aren't WAL files at
> > all, but it's the job of get_archive_wal_entry() to skip things that
> > are WAL files but not the one we want. I disagree with putting those
> > checks in completely separate parts of the code.
> >
>
> Keeping the timeline and segment start-end range checks inside the
> archive streamer creates a circular dependency that cannot be resolved
> without a 'dirty hack'. We must read the first available WAL file page
> to determine the wal_segment_size before it can calculate the target
> segment range. Moving the checks inside the streamer would make it
> impossible to process that initial file, as the necessary filtering
> parameters --  would still be unknown which would need to be skipped
> for the first read somehow. What if later we realized that the first
> WAL file which was allowed to be streamed by skipping that check is
> irrelevant and doesn't fall under the start-end segment range?
>

Please have a look at the attached version, specifically patch 0005.
In astreamer_waldump_content(), I have moved the WAL file filtration
check from get_archive_wal_entry(). This check will be skipped during
the initial read in init_archive_reader(), which instead performs it
explicitly once it determines the WAL segment size and the start/end
segments.

To access the WAL segment size inside astreamer_waldump_content(), I
have moved the WAL segment size variable into the XLogDumpPrivate
structure in the separate 0004 patch.

Regards,
Amul

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Use size_t instead of Size in pg_waldump

  2. More tar portability adjustments.

  3. Further harden tests that might use not-so-compatible tar versions.

  4. Harden astreamer tar parsing logic against archives it can't handle.

  5. Fix pg_waldump/t/001_basic.pl with BSD tar on ZFS.

  6. Remove a low-value, high-risk optimization in pg_waldump.

  7. Fix misuse of simplehash.h hash operations in pg_waldump.

  8. Fix file descriptor leakages in pg_waldump.

  9. Fix poorly-sized buffers in astreamer compression modules.

  10. Remove read_archive_file()'s "count" parameter.

  11. Report detailed errors from XLogFindNextRecord() failures.

  12. Fix assorted bugs in archive_waldump.c.

  13. Remove nonfunctional tar file trailer size check.

  14. Fix finalization of decompressor astreamers.

  15. Move tar detection and compression logic to common.

  16. pg_verifybackup: Enable WAL parsing for tar-format backups

  17. pg_waldump: Add support for reading WAL from tar archives

  18. pg_waldump: Preparatory refactoring for tar archive WAL decoding.

  19. pg_waldump: Remove file-level global WalSegSz.

  20. pg_verifybackup: Verify tar-format backups.