On Wed, Jun 22, 2016 at 8:48 PM, Robert Haas <robertmhaas@gmail.com> wrote:
> On Wed, Jun 22, 2016 at 5:14 AM, Amit Kapila <amit.kapila16@gmail.com> wrote:
>> We can do it in the way as you are suggesting, but there is another thing
>> which we need to consider here. As of now, the patch tries to finish the
>> split if it finds split-in-progress flag in either old or new bucket. We
>> need to lock both old and new buckets to finish the split, so it is quite
>> possible that two different backends try to lock them in opposite order
>> leading to a deadlock. I think the correct way to handle is to always try
>> to lock the old bucket first and then new bucket. To achieve that, if the
>> insertion on new bucket finds that split-in-progress flag is set on a
>> bucket, it needs to release the lock and then acquire the lock first on old
>> bucket, ensure pincount is 1 and then lock new bucket again and ensure that
>> pincount is 1. I have already maintained the order of locks in scan (old
>> bucket first and then new bucket; refer changes in _hash_first()).
>> Alternatively, we can try to finish the splits only when someone tries to
>> insert in old bucket.
>
> Yes, I think locking buckets in increasing order is a good solution.
Okay.
> I also think it's fine to only try to finish the split when the insert
> targets the old bucket. Finishing the split enables us to remove
> tuples from the old bucket, which lets us reuse space instead of
> accelerating more. So there is at least some potential benefit to the
> backend inserting into the old bucket. On the other hand, a process
> inserting into the new bucket derives no direct benefit from finishing
> the split.
>
makes sense, will change that way and will add a comment why we are
just doing it for old bucket.
--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com