Thank you for reviewing.
From: Michael Paquier [mailto:michael@paquier.xyz]
> It seems to me that the consolidation of the page read should happen directly
> in xlogreader.c and not even in one of its callbacks so as no garbage data
> is presented back to the caller using its own XLogReader.
> I think that you need to initialize XLogReaderState->readBuf directly in
> ReadPageInternal() before reading a page and you should be good. With your
> patch you get visibly only one portion of things addressed, what of other
> code paths using xlogreader.c's APIs like pg_rewind, 2PC code and such?
ReadPageInternal() doesn't know where the end of valid WAL is, so it cannot determine where to do memset(0). For
example,in non-streaming cases, it reads the entire WAL block into readbuf, including the garbage.
/*
* If the current segment is being streamed from master, calculate how
* much of the current page we have received already. We know the
* requested record has been received, but this is for the benefit of
* future calls, to allow quick exit at the top of this function.
*/
if (readSource == XLOG_FROM_STREAM)
{
if (((targetPagePtr) / XLOG_BLCKSZ) != (receivedUpto / XLOG_BLCKSZ))
readLen = XLOG_BLCKSZ;
else
readLen = receivedUpto % XLogSegSize - targetPageOff;
}
else
readLen = XLOG_BLCKSZ;
So we have to zero-fill the empty space of a WAL block before writing it. Currently, the master does that in
AdvanceXLInsertBuffer(),StartupXLOG(), XLogFileCopy(). The cascading standby receives WAL from the master block by
block,so it doesn't suffer from the garbage. pg_receivexlog zero-fills a new WAL file.
> Please note that your patch has some unnecessary garbage in two places:
Ouch, cleaned up.
> + if (zbuffer == NULL)
> + zbuffer = palloc0(XLOG_BLCKSZ);
> You could just use a static buffer which is MemSet'd with 0 only if necessary.
I wanted to allocate memory only when necessary. Currently, only the cascaded standby needs this. To that end, I
movedthe memory allocation code to a right place. Thanks for letting me notice this.
Regards
Takayuki Tsunakawa