At Fri, 11 Mar 2022 15:49:49 +0900 (JST), Kyotaro Horiguchi <horikyota....@gmail.com> wrote in > At Fri, 11 Mar 2022 15:30:30 +0900 (JST), Kyotaro Horiguchi > <horikyota....@gmail.com> wrote in > > Thanks! I looked into dynahash part.
Then I looked into bufmgr part. It looks fine to me but I have some comments on code comments. > * To change the association of a valid buffer, we'll need to > have > * exclusive lock on both the old and new mapping partitions. > if (oldFlags & BM_TAG_VALID) We don't take lock on the new mapping partition here. + * Clear out the buffer's tag and flags. We must do this to ensure that + * linear scans of the buffer array don't think the buffer is valid. We + * also reset the usage_count since any recency of use of the old content + * is no longer relevant. + * + * We are single pinner, we hold buffer header lock and exclusive + * partition lock (if tag is valid). Given these statements it is safe to + * clear tag since no other process can inspect it to the moment. This comment is a merger of the comments from InvalidateBuffer and BufferAlloc. But I think what we need to explain here is why we invalidate the buffer here despite of we are going to reuse it soon. And I think we need to state that the old buffer is now safe to use for the new tag here. I'm not sure the statement is really correct but clearing-out actually looks like safer. > Now it is safe to use victim buffer for new tag. Invalidate the > buffer before releasing header lock to ensure that linear scans of > the buffer array don't think the buffer is valid. It is safe > because it is guaranteed that we're the single pinner of the buffer. > That pin also prevents the buffer from being stolen by others until > we reuse it or return it to freelist. So I want to revise the following comment. - * Now it is safe to use victim buffer for new tag. + * Now reuse victim buffer for new tag. > * Make sure BM_PERMANENT is set for buffers that must be written at > every > * checkpoint. Unlogged buffers only need to be written at shutdown > * checkpoints, except for their "init" forks, which need to be treated > * just like permanent relations. > * > * The usage_count starts out at 1 so that the buffer can survive one > * clock-sweep pass. But if you think the current commet is fine, I don't insist on the comment chagnes. regards. -- Kyotaro Horiguchi NTT Open Source Software Center
