Kevin Grittner <kgri...@ymail.com> wrote: > Kyotaro HORIGUCHI <horiguchi.kyot...@lab.ntt.co.jp> wrote:
>> Would you mind rewriting the comment there like this? >> >> - /* The buffer is still pinned, but not locked. Lock it now. */ >> + /* I still hold the pin on the buffer, but not locked. Lock it now. */ >> Or would you mind renaming the macro as "BTScanPosIsPinnedByMe" >> or something like, or anyway to notify the reader that the pin >> should be mine there? > > I see your point, although those first person singular pronouns > used like that make me a little uncomfortable; I'll change the > comment and/or macro name, but I'll work on the name some more. After thinking it over, I think that the "BTScanPos" part of the macro name is enough of a hint that it is concerned with the actions of this scan; it is the comment that needs the change. I went with: /* * We have held the pin on this page since we read the index tuples, * so all we need to do is lock it. The pin will have prevented * re-use of any TID on the page, so there is no need to check the * LSN. */ >> + To handle the cases where it is safe to release the pin after >> + reading the index page, the LSN of the index page is read along >> + with the index entries before the shared lock is released, and we >> + do not attempt to flag index tuples as dead if the page is not >> + pinned and the LSN does not match. >> >> I suppose that the sentence like following is more directly >> describing about the mechanism and easier to find the >> correnponsing code, if it is correct. >> >>> To handle the cases where a index page has unpinned when >>> trying to mark the unused index tuples on the page as dead, >>> the LSN of the index page is remembered when reading the index >>> page for index tuples, and we do not attempt to flag index >>> tuples as dead if the page is not pinned and the LSN does not >>> match. > > Will reword that part to try to make it clearer. That sentence was full of "passive voice", which didn't help any. I changed it to: | So that we can handle the cases where we attempt LP_DEAD flagging | for a page after we have released its pin, we remember the LSN of | the index page when we read the index tuples from it; we do not | attempt to flag index tuples as dead if the we didn't hold the | pin the entire time and the LSN has changed. Do these changes seem clear? Because these changes are just to a comment and a README file, I'm posting a patch-on-patch v3a (to be applied on top of v3). -- Kevin Grittner EDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
bt-nopin-v3a.patch
Description: invalid/octet-stream
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers