On Tue, Dec 13, 2016 at 2:51 AM, Robert Haas <robertmhaas@gmail.com> wrote:
> On Sun, Dec 11, 2016 at 1:24 AM, Amit Kapila <amit.kapila16@gmail.com> wrote:
>>
>> With above fixes, the test ran successfully for more than a day.
>
> Instead of doing this:
>
> + _hash_chgbufaccess(rel, bucket_buf, HASH_WRITE, HASH_NOLOCK);
> + _hash_chgbufaccess(rel, bucket_buf, HASH_NOLOCK, HASH_WRITE);
>
> ...wouldn't it be better to just do MarkBufferDirty()? There's no
> real reason to release the lock only to reacquire it again, is there?
>
The reason is to make the operations consistent in master and standby.
In WAL patch, for clearing the SPLIT_CLEANUP flag, we write a WAL and
if we don't release the lock after writing a WAL, the operation can
appear on standby even before on master. Currently, without WAL,
there is no benefit of doing so and we can fix by using
MarkBufferDirty, however, I thought it might be simpler to keep it
this way as this is required for enabling WAL. OTOH, we can leave
that for WAL patch. Let me know which way you prefer?
> I don't think we should be afraid to call MarkBufferDirty() instead of
> going through these (fairly stupid) hasham-specific APIs.
>
Right and anyway we need to use it at many more call sites when we
enable WAL for hash index.
--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com