On 22/10/2018 20:54, Andrey Lepikhov wrote:
On 22.10.2018 2:06, Heikki Linnakangas wrote:
On 17/08/2018 06:47, Andrey Lepikhov wrote:
I propose the patch for fix one small code defect.
The XLogReadRecord() function reads the pages of a WAL segment that
contain a WAL-record. Then it creates a readRecordBuf buffer in private
memory of a backend and copy record from the pages to the readRecordBuf
buffer. Pointer 'record' is set to the beginning of the readRecordBuf
buffer.
But if the WAL-record is fully placed on one page, the XLogReadRecord()
function forgets to bind the "record" pointer with the beginning of the
readRecordBuf buffer. In this case, XLogReadRecord() returns a pointer
to an internal xlog page. This patch fixes the defect.
Hmm. I agree it looks weird the way it is. But I wonder, why do we even
copy the record to readRecordBuf, if it fits on the WAL page? Returning
a pointer to the xlog page buffer seems OK in that case. What if we
remove the memcpy(), instead?
It depends on the PostgreSQL roadmap. If we want to compress main data
and encode something to reduce a WAL-record size, than the
representation of the WAL-record in shared buffers (packed) and in local
memory (unpacked) will be different and the patch is needed.
I'd expect the decompression to read from the on-disk buffer, and unpack
to readRecordBuf, I still don't see a need to copy the packed record to
readRecordBuf. If there is a need for that, though, the patch that
implements the packing or compression can add the memcpy() where it
needs it.
- Heikki
