On Wed, Nov 9, 2016 at 9:10 PM, Robert Haas <robertmh...@gmail.com> wrote: > On Wed, Nov 9, 2016 at 9:04 AM, Amit Kapila <amit.kapil...@gmail.com> wrote: > > I think we can give a brief explanation right in the code comment. I > referred to "decreasing the TIDs"; you are referring to "moving tuples > around". But I think that moving the tuples to different locations is > not the problem. I think the problem is that a tuple might be > assigned a lower spot in the item pointer array >
I think we both understand the problem and it is just matter of using different words. I will go with your suggestion and will try to slightly adjust the README as well so that both places use same terminology. >>> + /* >>> + * Acquiring cleanup lock to clear the split-in-progress flag ensures >>> that >>> + * there is no pending scan that has seen the flag after it is cleared. >>> + */ >>> + _hash_chgbufaccess(rel, bucket_obuf, HASH_NOLOCK, HASH_WRITE); >>> + opage = BufferGetPage(bucket_obuf); >>> + oopaque = (HashPageOpaque) PageGetSpecialPointer(opage); >>> >>> I don't understand the comment, because the code *isn't* acquiring a >>> cleanup lock. >> >> Oops, this is ramnant from one of the design approach to clear these >> flags which was later discarded due to issues. I will change this to >> indicate Exclusive lock. > > Of course, an exclusive lock doesn't guarantee anything like that... > Right, but we don't need that guarantee (there is no pending scan that has seen the flag after it is cleared) to clear the flags. It was written in one of the previous patches where I was exploring the idea of using cleanup lock to clear the flags and then don't use the same during vacuum. However, there were some problems in that design and I have changed the code, but forgot to update the comment. -- With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
