Re: [PATCH] XLogReadRecord returns pointer to currently read page

From: Heikki Linnakangas <hlinnaka(at)iki(dot)fi>
To: Andrey Lepikhov <a(dot)lepikhov(at)postgrespro(dot)ru>, PostgreSQL-Dev <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [PATCH] XLogReadRecord returns pointer to currently read page
Date: 2018-10-22 19:53:36
Message-ID: 57759569-fa21-4a09-9f79-b1d257a48849@iki.fi
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message David Rowley 2018-10-22 20:31:19 Re: Small performance tweak to run-time partition pruning
Previous Message Jung, Jinho 2018-10-22 18:08:19 Re: Regarding query minimizer (simplifier)