On Tue, Oct 30, 2018 at 6:16 AM Andres Freund <andres@anarazel.de> wrote:
Hi,
Magnus cornered me at pgconf.eu and asked me whether I could prototype the "barriers" I'd been talking about in the online checksumming thread.
The problem there was to make sure that all processes, backends and auxiliary processes have seen the new state of checksums being enabled, and aren't currently in the process of writing a new page out.
The current prototype solves that by requiring a restart, but that strikes me as a far too large hammer.
The attached patch introduces "global barriers" (name was invented in a overcrowded hotel lounge, so ...), which allow to wait for such a change to be absorbed by all backends.
I've only tested the code with gdb, but that seems to work:
p WaitForGlobalBarrier(EmitGlobalBarrier(GLOBBAR_CHECKSUM))
waits until all backends (including bgwriter, checkpointers, walwriters, bgworkers, ...) have accepted interrupts at least once. Multiple such requests are coalesced.
I decided to wait until interrupts are actually process, rather than just the signal received, because that means the system is in a well defined state. E.g. there's no pages currently being written out.
For the checksum enablement patch you'd do something like;
and after that you should be able to set it to a perstistent mode.
I chose to use procsignals to send the signals, a global uint64 globalBarrierGen, and per-backend barrierGen, barrierFlags, with the latter keeping track which barriers have been requested. There likely seem to be other usecases.
The patch definitely is in a prototype stage. At the very least it needs a high-level comment somewhere, and some of the lower-level code needs to be cleaned up.
One thing I wasn't happy about is how checksum internals have to absorb barrier requests - that seems unavoidable, but I'd hope for something more global than just BufferSync().
Comments?
Finally getting back to this one.
In re-reading this, I notice there are a lot of references to Intterrupt (with two t). I'm guessing this is just a spelling error, and not something that actually conveys some meaning?
Can you elaborate on what you mean with:
+ /* XXX: need a more principled approach here */
Is that the thing you refer to above about "checksum internals"?
Also in checking we figured it'd be nice to have a wait event for this, since a process can potentially get stuck in an infinite loop waiting for some other process if it's misbehaving. Kind of like the attached?
