On Thu, Sep 15, 2016 at 7:25 PM, Robert Haas <robertmhaas@gmail.com> wrote:
> On Thu, Sep 15, 2016 at 2:13 AM, Amit Kapila <amit.kapila16@gmail.com> wrote:
>> One other point, I would like to discuss is that currently, we have a
>> concept for tracking active hash scans (hashscan.c) which I think is
>> mainly to protect splits when the backend which is trying to split has
>> some scan open. You can read "Other Notes" section of
>> access/hash/README for further details. I think after this patch we
>> don't need that mechanism for splits because we always retain a pin on
>> bucket buffer till all the tuples are fetched or scan is finished
>> which will defend against a split by our own backend which tries to
>> ensure cleanup lock on bucket.
>
> Hmm, yeah. It seems like we can remove it.
>
>> However, we might need it for vacuum
>> (hashbulkdelete), if we want to get rid of cleanup lock in vacuum,
>> once we have a page-at-a-time scan mode implemented for hash indexes.
>> If you agree with above analysis, then we can remove the checks for
>> _hash_has_active_scan from both vacuum and split path and also remove
>> corresponding code from hashbegin/end scan, but retain that hashscan.c
>> for future improvements.
>
> Do you have a plan for that? I'd be inclined to just blow away
> hashscan.c if we don't need it any more, unless you're pretty sure
> it's going to get reused. It's not like we can't pull it back out of
> git if we decide we want it back after all.
>
I do want to work on it, but it is always possible that due to some
other work this might get delayed. Also, I think there is always a
chance that while doing that work, we face some problem due to which
we might not be able to use that optimization. So I will go with your
suggestion of removing hashscan.c and it's usage for now and then if
required we will pull it back. If nobody else thinks otherwise, I
will update this in next patch version.
--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com