Hi,
On 2021-01-29 12:40:18 -0300, Alvaro Herrera wrote: > On 2020-Aug-31, Andres Freund wrote: > > > Wouldn't the better fix here be to allow reading of individual members > > without a lock? E.g. by wrapping each in a 64bit atomic. > > So I've been playing with this and I'm annoyed about having two > datatypes to represent Write/Flush positions: > > typedef struct XLogwrtRqst > { > XLogRecPtr Write; /* last byte + 1 to write out */ > XLogRecPtr Flush; /* last byte + 1 to flush */ > } XLogwrtRqst; > > typedef struct XLogwrtResult > { > XLogRecPtr Write; /* last byte + 1 written out */ > XLogRecPtr Flush; /* last byte + 1 flushed */ > } XLogwrtResult; > > Don't they look, um, quite similar? I am strongly tempted to remove > that distinction, since it seems quite pointless, and introduce a > different one: Their spelling drives me nuts. Like, one is *Rqst, the other *Result? Comeon. > Now, I do wonder if there's a point in keeping LogwrtResult as a local > variable at all; maybe since the ones in shared memory are going to use > unlocked access, we don't need it anymore? I'd prefer to defer that > decision to after this patch is done, since ISTM that it'd merit more > careful benchmarking. I think doing that might be a bit harmful - we update LogwrtResult fairly granularly in XLogWrite(). Doing that in those small steps in shared memory will increase the likelihood of cache misses in other backends. Greetings, Andres Freund