MIME-Version: 1.0
References: 
 <CAAJ_b94bqdWN3h2J-PzzzQ2Npbwct5ZQHggn_QoYGhC2rn-=WQ@mail.gmail.com>
 <CAAJ_b979p7Z-dBZhxBC3m2VPkYTvYzypMTWkYg2q-e1S5F_f-Q@mail.gmail.com>
 <CAAJ_b96csGv+TvdxK-zp1Zo6zrAxOJ4n-qnxiWO1f0Lk0X0N_g@mail.gmail.com>
 <dd8f7a20-f692-47cf-be79-fd6b54d4a1a6@dunslane.net>
 <CAAJ_b95k4yWsVSbptTS0SF3thZBuQURvFCCAKwwz-0yyZhbnTQ@mail.gmail.com>
 <d84d43bb-6d9a-477e-93bd-1f61a93451cc@dunslane.net>
 <72973471-8946-40c7-8b2d-8f95540d90e2@dunslane.net>
 <CAAJ_b94hkpAJ-Q8FKjGgpbRTmmXOd-agKo1Eii4yOH2N++N36Q@mail.gmail.com>
 <401bf08a-c8f1-48e2-9a30-78deaa9fa7c5@dunslane.net>
 <CAAJ_b94jrauD_FAegTnODszjnF0O+ZFLtXhXBSPhyThRUqSNVg@mail.gmail.com>
 <CAAJ_b94fiDcsN6k27hV9772kNjEbtZG5ESQvXL1KQoUcRcxrGA@mail.gmail.com>
 <CAAJ_b95JZ7SunSeCgxB3+pC+38B5smD9y4uubAGmZTNo0xtHog@mail.gmail.com>
 <CAN4CZFMn_9wgEG0q-9CCXynZ85FzVFoyVU=okWESA42U542ajw@mail.gmail.com>
 <CAAJ_b95Oj6Kb6YGsV42Gqy=N7GuOX+FMmEtUbS7NC6BvARN2mQ@mail.gmail.com>
 <CAAJ_b96AG+p+n+DZPc4isTDR_rD_-dYfVJGXhG0k2CG+kdzU9g@mail.gmail.com>
 <9a2be101-42c9-40dd-9860-aa12f06bf0e0@dunslane.net>
 <2178517.1774064942@sss.pgh.pa.us>
In-Reply-To: <2178517.1774064942@sss.pgh.pa.us>
From: Amul Sul <sulamul@gmail.com>
Date: Sat, 21 Mar 2026 11:49:28 +0530
Message-ID: 
 <CAAJ_b95L5J7bjRNDjRj6WgqFcQeaBD+JX3sAuxPA4uopqEThxA@mail.gmail.com>
Subject: Re: pg_waldump: support decoding of WAL inside tarfile
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Andrew Dunstan <andrew@dunslane.net>,
 Zsolt Parragi <zsolt.parragi@percona.com>,
	Robert Haas <robertmhaas@gmail.com>, Chao Li <li.evan.chao@gmail.com>,
	Jakub Wartak <jakub.wartak@enterprisedb.com>,
	PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Content-Type: multipart/mixed; boundary="000000000000f0ac82064d82ca25"
Archived-At: 
 <https://www.postgresql.org/message-id/CAAJ_b95L5J7bjRNDjRj6WgqFcQeaBD%2BJX3sAuxPA4uopqEThxA%40mail.gmail.com>
Precedence: bulk

--000000000000f0ac82064d82ca25
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Sat, Mar 21, 2026 at 9:19=E2=80=AFAM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> Andrew Dunstan <andrew@dunslane.net> writes:
> > Thanks, committed with very minor tweaks.
>
> Buildfarm members batta and hachi don't like this very much.
> They fail the pg_verifybackup tests like so:
>
> # Running: pg_verifybackup --exit-on-error /home/admin/batta/buildroot/HE=
AD/pgsql.build/src/bin/pg_verifybackup/tmp_check/t_008_untar_primary_data/b=
ackup/server-backup
> pg_waldump: error: could not find WAL in archive "base.tar.zst"
> pg_verifybackup: error: WAL parsing failed for timeline 1
>
> Only the zstd-compression case fails.  I've spent several hours trying
> to reproduce this, without any luck, although I can get a similar
> failure in only the gzip case if I build with --with-wal-blocksize=3D64.
> I do not have an explanation for the seeming cross-platform
> difference.  However after adding a lot of debug tracing, I believe
> I see the bug, or at least a related bug.  This bit in
> archive_waldump.c's init_archive_reader is where the error comes from:
>
>     /*
>      * Read until we have at least one full WAL page (XLOG_BLCKSZ bytes) =
from
>      * the first WAL segment in the archive so we can extract the WAL seg=
ment
>      * size from the long page header.
>      */
>     while (entry =3D=3D NULL || entry->buf->len < XLOG_BLCKSZ)
>     {
>         if (read_archive_file(privateInfo, XLOG_BLCKSZ) =3D=3D 0)
>             pg_fatal("could not find WAL in archive \"%s\"",
>                      privateInfo->archive_name);
>
>         entry =3D privateInfo->cur_file;
>     }
>
> That looks plausible but is in fact utterly broken when there's not a
> lot of WAL data in the archive, as there is not in this test case.
> There are at least two problems:
>

Thanks for the detailed debugging. I noticed the failure this morning
and had started investigating the issue, but in the meantime, I got
your helpful reply, which saved me a bunch of time and energy.

> 1. read_archive_file reads some data from the source WAL archive and
> shoves it into the astreamer decompression pipeline.  However, once it
> runs out of source data, it just returns zero and we fail immediately.
> This does not account for the possibility --- nay, certainty --- that
> there is data queued inside the decompression pipeline.  So this
> doesn't work if the data we need has been compressed into less than
> XLOG_BLCKSZ worth of compressed data.  (I suppose that the seeming
> cross-platform differences have to do with the effectiveness of the
> compression algorithm, but I don't really understand why it'd not be
> the same everywhere.)  We need to do astreamer_finalize once we run
> out of source data.  I think the cleanest place to handle that would
> be inside read_archive_file, but its return convention will need some
> rework if we want to put it there (because rc =3D=3D 0 shouldn't cause an
> immediate failure if we were able to finalize some more data).  As an
> ugly experiment I put an astreamer_finalize call into the rc =3D=3D 0 pat=
h
> of the above loop, but it still didn't work, because:
>
> 2. If the decompression pipeline reaches the end of the WAL file that
> we want, the ASTREAMER_MEMBER_TRAILER case in
> astreamer_waldump_content instantly resets privateInfo->cur_file to
> NULL.  Then the loop in init_archive_reader cannot exit successfully,
> and it will just read till the end of the archive and fail.
>
> I see that of the three callers of read_archive_file, only
> get_archive_wal_entry is aware of this possibility; but
> init_archive_reader certainly needs to deal with it and I bet
> read_archive_wal_page does too.  Moreover, get_archive_wal_entry's
> solution looks to me like a fragile kluge that probably doesn't work
> reliably either, the reason being that privateInfo->cur_file can
> change multiple times during a single call to read_archive_file,
> if the WAL data has been compressed sufficiently.  That whole API
> seems to need some rethinking, not to mention better documentation
> than the zero it has now.
>

I agree; init_archive_reader needs that handling, but
read_archive_wal_page doesn't need any fix. Since it only deals with
the current entry and already holds a reference to it, there is no
need to fetch it from the hash table again.

init_archive_reader has to scan the hash table because it doesn't
already have the specific WAL filename it is looking for, unlike
get_archive_wal_entry. Please have a look at the attached patch, which
tries to fix that.

> While I'm bitching: this error message "could not find WAL in archive
> \"%s\"" seems to me to be completely misleading and off-point.
>

I tried to improve that in the attached version.

regards,
Amul

--000000000000f0ac82064d82ca25
Content-Type: application/x-patch;
	name="0001-pg_waldump-buildfarm-fix.patch"
Content-Disposition: attachment;
	filename="0001-pg_waldump-buildfarm-fix.patch"
Content-Transfer-Encoding: base64
Content-ID: <f_mmzx0hd00>
X-Attachment-Id: f_mmzx0hd00

RnJvbSBiZGUzZmI0ZTMxMjVlZWQ3NDBiNWQ5NDlhOTkwYjRlMDZkMDE0OTlhIE1vbiBTZXAgMTcg
MDA6MDA6MDAgMjAwMQpGcm9tOiBBbXVsIFN1bCA8c3VsYW11bEBnbWFpbC5jb20+CkRhdGU6IFNh
dCwgMjEgTWFyIDIwMjYgMTE6MjI6NTAgKzA1MzAKU3ViamVjdDogW1BBVENIXSBwZ193YWxkdW1w
OiBIYW5kbGUgYXJjaGl2ZSBleGhhdXN0aW9uIGluCiBpbml0X2FyY2hpdmVfcmVhZGVyKCkuCgpX
aGVuIHJlYWRfYXJjaGl2ZV9maWxlKCkgcmV0dXJucyAwLCB0aGUgYXJjaGl2ZSBtYXkgaGF2ZSBh
bHJlYWR5CmJ1ZmZlcmVkIGEgY29tcGxldGUgV0FMIGZpbGUgaW50byB0aGUgaGFzaCB0YWJsZSBi
ZWZvcmUgZXhoYXVzdGluZwp0aGUgaW5wdXQuICBJbnN0ZWFkIG9mIGltbWVkaWF0ZWx5IHJlcG9y
dGluZyBhbiBlcnJvciwgc2VhcmNoIHRoZQpoYXNoIHRhYmxlIGZvciBhbiBlbnRyeSBjb250YWlu
aW5nIGF0IGxlYXN0IHNpemVvZihYTG9nTG9uZ1BhZ2VIZWFkZXIpCmJ5dGVzLiAgUmVwb3J0IGEg
c3BlY2lmaWMgZXJyb3IgaWYgYSBXQUwgZW50cnkgZXhpc3RzIGJ1dCBpcyB0b28Kc2hvcnQgKHRy
dW5jYXRlZC9jb3JydXB0KSwgb3IgYSBnZW5lcmljIGVycm9yIGlmIG5vIFdBTCB3YXMgZm91bmQK
YXQgYWxsLgoKQWxzbyB0aWdodGVuIHRoZSBsb29wIGNvbmRpdGlvbiB0byBjaGVjayBmb3Igc2l6
ZW9mKFhMb2dMb25nUGFnZUhlYWRlcikKcmF0aGVyIHRoYW4gWExPR19CTENLU1osIHNpbmNlIG9u
bHkgdGhlIGxvbmcgcGFnZSBoZWFkZXIgaXMgbmVlZGVkCmF0IHRoaXMgc3RhZ2UuCi0tLQogc3Jj
L2Jpbi9wZ193YWxkdW1wL2FyY2hpdmVfd2FsZHVtcC5jIHwgNTEgKysrKysrKysrKysrKysrKysr
KysrKysrKy0tLQogMSBmaWxlIGNoYW5nZWQsIDQ3IGluc2VydGlvbnMoKyksIDQgZGVsZXRpb25z
KC0pCgpkaWZmIC0tZ2l0IGEvc3JjL2Jpbi9wZ193YWxkdW1wL2FyY2hpdmVfd2FsZHVtcC5jIGIv
c3JjL2Jpbi9wZ193YWxkdW1wL2FyY2hpdmVfd2FsZHVtcC5jCmluZGV4IGIwNzhjMmQ2OTYwLi41
YmQxZmFmM2Q5NSAxMDA2NDQKLS0tIGEvc3JjL2Jpbi9wZ193YWxkdW1wL2FyY2hpdmVfd2FsZHVt
cC5jCisrKyBiL3NyYy9iaW4vcGdfd2FsZHVtcC9hcmNoaXZlX3dhbGR1bXAuYwpAQCAtMTc2LDEz
ICsxNzYsNTYgQEAgaW5pdF9hcmNoaXZlX3JlYWRlcihYTG9nRHVtcFByaXZhdGUgKnByaXZhdGVJ
bmZvLAogCSAqIHRoZSBmaXJzdCBXQUwgc2VnbWVudCBpbiB0aGUgYXJjaGl2ZSBzbyB3ZSBjYW4g
ZXh0cmFjdCB0aGUgV0FMIHNlZ21lbnQKIAkgKiBzaXplIGZyb20gdGhlIGxvbmcgcGFnZSBoZWFk
ZXIuCiAJICovCi0Jd2hpbGUgKGVudHJ5ID09IE5VTEwgfHwgZW50cnktPmJ1Zi0+bGVuIDwgWExP
R19CTENLU1opCisJd2hpbGUgKGVudHJ5ID09IE5VTEwgfHwgZW50cnktPnJlYWRfbGVuIDwgc2l6
ZW9mKFhMb2dMb25nUGFnZUhlYWRlcikpCiAJewogCQlpZiAocmVhZF9hcmNoaXZlX2ZpbGUocHJp
dmF0ZUluZm8sIFhMT0dfQkxDS1NaKSA9PSAwKQotCQkJcGdfZmF0YWwoImNvdWxkIG5vdCBmaW5k
IFdBTCBpbiBhcmNoaXZlIFwiJXNcIiIsCi0JCQkJCSBwcml2YXRlSW5mby0+YXJjaGl2ZV9uYW1l
KTsKKwkJeworCQkJQXJjaGl2ZWRXQUxfaXRlcmF0b3IgaXRlcjsKKwkJCUFyY2hpdmVkV0FMRmls
ZSAqZSA9IE5VTEw7CiAKLQkJZW50cnkgPSBwcml2YXRlSW5mby0+Y3VyX2ZpbGU7CisJCQllbnRy
eSA9IE5VTEw7CisKKwkJCS8qCisJCQkgKiByZWFkX2FyY2hpdmVfZmlsZSgpIHJldHVybmVkIDAs
IG1lYW5pbmcgdGhlIGFyY2hpdmUgaXMKKwkJCSAqIGV4aGF1c3RlZC4gIEhvd2V2ZXIsIGEgc3Vm
ZmljaWVudGx5IGNvbXByZXNzZWQgYXJjaGl2ZSBtYXkgaGF2ZQorCQkJICogYWxyZWFkeSByZWFk
IGEgY29tcGxldGUgV0FMIGZpbGUgYW5kIGluc2VydGVkIGl0IGludG8gdGhlIGhhc2gKKwkJCSAq
IHRhYmxlIGJlZm9yZSByZXR1cm5pbmcuICBTZWFyY2ggdGhlIGhhc2ggdGFibGUgZm9yIGFueSBl
bnRyeQorCQkJICogdGhhdCBhbHJlYWR5IGhhcyBlbm91Z2ggYnVmZmVyZWQgZGF0YSB0byBjb250
YWluIHRoZSBsb25nIHBhZ2UKKwkJCSAqIGhlYWRlcjsgaWYgbm9uZSBpcyBmb3VuZCwgdGhlIGFy
Y2hpdmUgY29udGFpbnMgbm8gdXNhYmxlIFdBTC4KKwkJCSAqLworCQkJQXJjaGl2ZWRXQUxfc3Rh
cnRfaXRlcmF0ZShwcml2YXRlSW5mby0+YXJjaGl2ZV93YWxfaHRhYiwgJml0ZXIpOworCQkJd2hp
bGUgKChlID0gQXJjaGl2ZWRXQUxfaXRlcmF0ZShwcml2YXRlSW5mby0+YXJjaGl2ZV93YWxfaHRh
YiwKKwkJCQkJCQkJCQkJJml0ZXIpKSAhPSBOVUxMKQorCQkJeworCQkJCWlmIChlLT5yZWFkX2xl
biA+PSBzaXplb2YoWExvZ0xvbmdQYWdlSGVhZGVyKSkKKwkJCQl7CisJCQkJCWVudHJ5ID0gZTsK
KwkJCQkJYnJlYWs7CisJCQkJfQorCQkJfQorCisJCQlpZiAoZW50cnkgPT0gTlVMTCkKKwkJCXsK
KwkJCQkvKgorCQkJCSAqIEEgV0FMIGZpbGUgd2FzIGZvdW5kIGluIHRoZSBoYXNoIHRhYmxlIGJ1
dCBpdCBkb2VzIG5vdAorCQkJCSAqIGNvbnRhaW4gZW5vdWdoIGRhdGEgdG8gcmVhZCB0aGUgbG9u
ZyBwYWdlIGhlYWRlciwKKwkJCQkgKiBpbmRpY2F0aW5nIGEgdHJ1bmNhdGVkIG9yIGNvcnJ1cHQg
V0FMIHNlZ21lbnQuCisJCQkJICovCisJCQkJaWYgKGUgIT0gTlVMTCkKKwkJCQkJcGdfZmF0YWwo
ImNvdWxkIG5vdCByZWFkIGZpbGUgXCIlc1wiIGZyb20gXCIlc1wiIGFyY2hpdmU6IHJlYWQgJWQg
b2YgJWQiLAorCQkJCQkJCSBlLT5mbmFtZSwgcHJpdmF0ZUluZm8tPmFyY2hpdmVfbmFtZSwgZS0+
cmVhZF9sZW4sCisJCQkJCQkJIChpbnQpIHNpemVvZihYTG9nTG9uZ1BhZ2VIZWFkZXIpKTsKKwor
CQkJCS8qCisJCQkJICogVGhlIGhhc2ggdGFibGUgY29udGFpbnMgbm8gV0FMIGVudHJpZXMgYXQg
YWxsLCBtZWFuaW5nIHRoZQorCQkJCSAqIGFyY2hpdmUgaG9sZHMgbm8gV0FMIGRhdGEuCisJCQkJ
ICovCisJCQkJcGdfZmF0YWwoImNvdWxkIG5vdCBmaW5kIFdBTCBpbiBhcmNoaXZlIFwiJXNcIiIs
CisJCQkJCQkgcHJpdmF0ZUluZm8tPmFyY2hpdmVfbmFtZSk7CisJCQl9CisJCX0KKwkJZWxzZQor
CQkJZW50cnkgPSBwcml2YXRlSW5mby0+Y3VyX2ZpbGU7CiAJfQogCiAJLyogRXh0cmFjdCB0aGUg
V0FMIHNlZ21lbnQgc2l6ZSBmcm9tIHRoZSBsb25nIHBhZ2UgaGVhZGVyICovCi0tIAoyLjQ3LjEK
Cg==
--000000000000f0ac82064d82ca25--