Date: Wed, 14 Oct 2020 15:52:43 +0900
From: Michael Paquier <michael@paquier.xyz>
To: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Cc: movead.li@highgo.ca, pgsql-hackers@postgresql.org, ahsan.hadi@highgo.ca
Subject: Re: Wrong statistics for size of XLOG_SWITCH during pg_waldump.
Message-ID: <20201014065243.GA12403@paquier.xyz>
References: <2020101009495929215052@highgo.ca>
 <20201012011212.GC2346@paquier.xyz>
 <2020101209463463842666@highgo.ca>
 <20201014.102944.102635524623180309.horikyota.ntt@gmail.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="GvXjxJ+pjyke8COw"
Content-Disposition: inline
In-Reply-To: <20201014.102944.102635524623180309.horikyota.ntt@gmail.com>
Precedence: bulk


--GvXjxJ+pjyke8COw
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

On Wed, Oct 14, 2020 at 10:29:44AM +0900, Kyotaro Horiguchi wrote:
> The reason is the function XLogDumpRecordLen is a common function
> among all kind of LOG records, not belongs only to XLOG_SWICH. And the
> junk_len is not useful for other than XLOG_SWITCH.  Descriptions
> specifc to XLOG_SWITCH is provided by xlog_desc().

Yeah.  In its current shape, it means that only pg_waldump would be
able to know this information.  If you make this information part of
xlogdesc.c, any consumer of the WAL record descriptions would be able
to show this information, so it would provide a consistent output for
any kind of tools.

On top of that, it seems to me that the calculation used in the patch
is wrong in two aspects at quick glance:
1) startSegNo and endSegNo point always to two different segments with
a XLOG_SWITCH record, so you should check that ReadRecPtr is not at a
segment border instead before extracting SizeOfXLogLongPHD, no?
2) This stuff should also check after the case of a WAL *page* border
where you'd need to adjust based on SizeOfXLogShortPHD instead.
--
Michael

--GvXjxJ+pjyke8COw
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQIzBAABCgAdFiEEG72nH6vTowiyblFKnvQgOdbyQH0FAl+GoDsACgkQnvQgOdby
QH0bdg//U71AjpxwD/zCx88zf/E8JfquJTMz0FakyXrcLu6EgRl7WOruVtu8dL2/
7OBde6ExbyxcfE7/2QSOp6ShC3Tb/QSaa/KSaAg0PSi9KLATZWCz2U0H9LB/KLNJ
zwAMvrtDufY1lnHKJEXGgJQ0o8fhw6SsAiFA5/9e07JFI6KiYXIPRVsVmfbZeiDq
mf21bUjbDyx1osO8F2l2eShzD6w3gfUGrinHIHhuLF4pG1z0/eLW7RRFhlalSKaD
Q6ua/uwDttFx+0Jj3XzR9EY/4KAf14PtwflIH6X+pKfjgRan36B1GFykykmEAUqy
oyCx0xpIQjf/5Yaq3WGTubEKGdipPecqi9/baTWJSc+/2Ts0A1Tz4r6f4ii6MS6g
kedTuwq706DgtXhcwmDaFG79sr5IWc+9sricMuYMP6m2ySWHnuyRQe+oNCwkFsCq
3Imr/IggynA5x8D1Tuffh6oUSyK42NWEcLMiW42MSPHV7rQUufdWBabfCdGyPzR2
pGdjyYcKWude4s2v+LNzLTPWUEGD8hyvPLXP0ns1pW/Rg8WjynE9tXiH7l+KDsn3
mUpz/ox8zHb9lm03M3YM5GLLiODRVvLcvyjeovMQW7aTJa6wV/ThoxVzN2SBUG9O
NaEPXpNrzxtvjxuHkOvNGPgbaDFPAfzde4s6kb1JM6+Ktei7eOY=
=03YF
-----END PGP SIGNATURE-----

--GvXjxJ+pjyke8COw--