Greetings, * Nathan Bossart (nathandboss...@gmail.com) wrote: > On Mon, Jun 12, 2023 at 07:24:18PM -0400, Stephen Frost wrote: > > * Nathan Bossart (nathandboss...@gmail.com) wrote: > >> Is it? I see uses in GiST indexing (62401db), so it's not immediately > >> obvious to me how it is debugging-only. If it is, then I think this patch > >> ought to clearly document it so that nobody else tries to use it for > >> non-debugging-only stuff. > > > > I don't see it as a debugging value. I'm not sure where that came > > from..? We do use it in places and if anything, I expect it to be used > > more, not less, in the future as a persistant generally increasing > > value (could go backwards on a crash-restart but in such case all > > unlogged tables are truncated). > > This is my understanding as well. > > >> My concern would be whether GetFakeLSNForUnloggedRel or CreateCheckPoint > >> might see an old value of unloggedLSN. From the following note in > >> README.barrier, it sounds like this would be a problem even if we ensured > >> full barrier semantics: > > Never mind. I think I'm forgetting that the atomics support in Postgres > deals with cache coherency. > > > The concern around unlogged LSN, imv anyway, is less about shared memory > > access and making sure that all callers understand that it can move > > backwards on a crash/restart. I don't think that's an issue for current > > users but we just need to make sure to try and comment sufficiently > > regarding that such that new users understand that about this particular > > value. > > Right.
Awesome. Was there any other feedback on this change which basically is just getting rid of a spinlock and replacing it with using atomics? It's still in needs-review status but there's been a number of comments/reviews (drive-by, at least) but without any real ask for any changes to be made. Thanks! Stephen
signature.asc
Description: PGP signature
