On Tue, Dec 20, 2016 at 7:44 PM, Robert Haas <robertmhaas@gmail.com> wrote:
> On Tue, Dec 20, 2016 at 9:01 AM, Amit Kapila <amit.kapila16@gmail.com> wrote:
>> On Tue, Dec 20, 2016 at 7:11 PM, Robert Haas <robertmhaas@gmail.com> wrote:
>>> On Tue, Dec 20, 2016 at 4:51 AM, Amit Kapila <amit.kapila16@gmail.com> wrote:
>>>> We have mainly four actions for squeeze operation, add tuples to the
>>>> write page, empty overflow page, unlinks overflow page, make it free
>>>> by setting the corresponding bit in overflow page. Now, if we don't
>>>> log the changes to write page and freeing of overflow page as one
>>>> operation, then won't query on standby can either see duplicate tuples
>>>> or miss the tuples which are freed in overflow page.
>>>
>>> No, I think you could have two operations:
>>>
>>> 1. Move tuples from the "read" page to the "write" page.
>>>
>>> 2. Unlink the overflow page from the chain and mark it free.
>>>
>>> If we fail after step 1, the bucket chain might end with an empty
>>> overflow page, but that's OK.
>>
>> If there is an empty page in bucket chain, access to that page will
>> give an error (In WAL patch we are initializing the page instead of
>> making it completely empty, so we might not see an error in such a
>> case).
>
> It wouldn't be a new, uninitialized page. It would be empty of
> tuples, not all-zeroes.
>
AFAIU we initialize page as all-zeros, but I think you are envisioning
that we need to change it to a new uninitialized page.
>> What advantage do you see by splitting the operation?
>
> It's simpler. The code here is very complicated and trying to merge
> too many things into a single operation may make it even more
> complicated, increasing the risk of bugs and making the code hard to
> maintain without necessarily buying much performance.
>
Sure, if you find that way better, then we can change it, but the
current patch treats it as a single operation. If after looking the
patch you find it is better to change it into two operations, I will
do so.
--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com