В Вс, 06/02/2022 в 19:34 +0300, Michail Nikolaev пишет: > Hello, Yura. > > A one additional moment: > > > 1332: Assert((oldFlags & (BM_PIN_COUNT_WAITER | BM_IO_IN_PROGRESS)) == 0); > > 1333: CLEAR_BUFFERTAG(buf->tag); > > 1334: buf_state &= ~(BUF_FLAG_MASK | BUF_USAGECOUNT_MASK); > > 1335: UnlockBufHdr(buf, buf_state); > > I think there is no sense to unlock buffer here because it will be > locked after a few moments (and no one is able to find it somehow). Of > course, it should be unlocked in case of collision.
UnlockBufHdr actually writes buf_state. Until it called, buffer is in intermediate state and it is ... locked. We have to write state with BM_TAG_VALID cleared before we call BufTableDelete and release oldPartitionLock to maintain consistency. Perhaps, it could be cheated, and there is no harm to skip state write at this point. But I'm not so confident to do it. > > BTW, I still think is better to introduce some kind of > hash_update_hash_key and use it. > > It may look like this: > > // should be called with oldPartitionLock acquired > // newPartitionLock hold on return > // oldPartitionLock and newPartitionLock are not taken at the same time > // if newKeyPtr is present - existingEntry is removed > bool hash_update_hash_key_or_remove( > HTAB *hashp, > void *existingEntry, > const void *newKeyPtr, > uint32 newHashValue, > LWLock *oldPartitionLock, > LWLock *newPartitionLock > ); Interesting suggestion, thanks. I'll think about. It has downside of bringing LWLock knowdlege to dynahash.c . But otherwise looks smart. --------- regards, Yura Sokolov