Hi, On 2022-08-09 17:24:35 +0900, Kyotaro Horiguchi wrote: > At Mon, 8 Aug 2022 11:53:19 -0400, Greg Stark <st...@mit.edu> wrote in > > I'm trying to wrap my head around the shared memory stats collector > > infrastructure from > > <20220406030008.2qxipjxo776dw...@alap3.anarazel.de> committed in > > 5891c7a8ed8f2d3d577e7eea34dacff12d7b6bbd. > > > > I have one question about locking -- afaics there's nothing protecting > > reading the shared memory stats. There is an lwlock protecting > > concurrent updates of the shared memory stats, but that lock isn't > > taken when we read the stats. Are we ok relying on atomic 64-bit reads > > or is there something else going on that I'm missing?
Yes, that's not right. Not sure how it ended up that way. There was a lot of refactoring and pushing down the locking into different places, I guess it got lost somewhere on the way :(. It's unlikely to be a large problem, but we should fix it. > If I'm not missing something, it's strange that pgstat_lock_entry() > only takes LW_EXCLUSIVE. I think it makes some sense, given that there's a larger number of callers for that in various stats-emitting code. Perhaps we could just add a separate function with a _shared() suffix? > The atached changes the interface of > pgstat_lock_entry() but there's only one user since another read of > shared stats entry is not using reference. Thus the interface change > might be too much. If I just add bare LWLockAcquire/Release() to > pgstat_fetch_entry,the amount of the patch will be reduced. Could you try the pgstat_lock_entry_shared() approach? Greetings, Andres Freund
