From: Tom Lane <tgl@sss.pgh.pa.us>
To: Alvaro Herrera <alvherre@2ndquadrant.com>
cc: pgbf@twiska.com, Kyotaro Horiguchi <horikyota.ntt@gmail.com>,
        masao.fujii@oss.nttdata.com, pgsql-hackers@lists.postgresql.org
Subject: Re: min_safe_lsn column in pg_replication_slots view
In-reply-to: <1773306.1594256635@sss.pgh.pa.us>
References: <20200709005332.GA1986@alvherre.pgsql>
 <1773306.1594256635@sss.pgh.pa.us>
Comments: In-reply-to Tom Lane <tgl@sss.pgh.pa.us>
	message dated "Wed, 08 Jul 2020 21:03:55 -0400"
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <2008606.1594331766.1@sss.pgh.pa.us>
Content-Transfer-Encoding: quoted-printable
Date: Thu, 09 Jul 2020 17:56:06 -0400
Message-ID: <2008607.1594331766@sss.pgh.pa.us>
Precedence: bulk

... or on the other hand, maybe these animals are just showing more
sensitivity than others to an actual code bug.  skink is showing
valgrind failures in this very area, on both HEAD and v13:

https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=3Dskink&dt=3D2020-=
07-08%2021%3A13%3A02

=3D=3D3166208=3D=3D VALGRINDERROR-BEGIN
=3D=3D3166208=3D=3D Conditional jump or move depends on uninitialised valu=
e(s)
=3D=3D3166208=3D=3D    at 0x28618D: KeepLogSeg (xlog.c:9627)
=3D=3D3166208=3D=3D    by 0x296AC5: GetWALAvailability (xlog.c:9533)
=3D=3D3166208=3D=3D    by 0x4FFECB: pg_get_replication_slots (slotfuncs.c:=
356)
=3D=3D3166208=3D=3D    by 0x3C762F: ExecMakeTableFunctionResult (execSRF.c=
:234)
=3D=3D3166208=3D=3D    by 0x3D9A9A: FunctionNext (nodeFunctionscan.c:95)
=3D=3D3166208=3D=3D    by 0x3C81D6: ExecScanFetch (execScan.c:133)
=3D=3D3166208=3D=3D    by 0x3C81D6: ExecScan (execScan.c:199)
=3D=3D3166208=3D=3D    by 0x3D99A9: ExecFunctionScan (nodeFunctionscan.c:2=
70)
=3D=3D3166208=3D=3D    by 0x3C5072: ExecProcNodeFirst (execProcnode.c:450)
=3D=3D3166208=3D=3D    by 0x3BD35E: ExecProcNode (executor.h:245)
=3D=3D3166208=3D=3D    by 0x3BD35E: ExecutePlan (execMain.c:1646)
=3D=3D3166208=3D=3D    by 0x3BE039: standard_ExecutorRun (execMain.c:364)
=3D=3D3166208=3D=3D    by 0x3BE102: ExecutorRun (execMain.c:308)
=3D=3D3166208=3D=3D    by 0x559669: PortalRunSelect (pquery.c:912)
=3D=3D3166208=3D=3D  Uninitialised value was created by a stack allocation
=3D=3D3166208=3D=3D    at 0x296A84: GetWALAvailability (xlog.c:9523)
=3D=3D3166208=3D=3D =

=3D=3D3166208=3D=3D VALGRINDERROR-END

and even the most cursory look at the code confirms that there's a
real bug here.  KeepLogSeg expects *logSegNo to be defined on entry,
but GetWALAvailability hasn't bothered to initialize oldestSlotSeg.
It is not clear to me which one is in the wrong; the comment for
KeepLogSeg isn't particularly clear on this.

			regards, tom lane