On Thu, 12 May 2022 at 04:40, Andres Freund <and...@anarazel.de> wrote: > > On 2022-05-11 09:39:48 -0700, Jeff Davis wrote: > > On Wed, 2022-05-11 at 15:24 +0100, Simon Riggs wrote: > > > [PATCH: rmgr_001.v1.patch] > > > > > > [PATCH: rmgr_002.v1.patch] > > > > Thank you. Both of these look like good ideas, and I will commit them > > in a few days assuming that nobody else sees a problem. > > What exactly is the use case here? Without passing in information about > whether recovery will be performed etc, it's not at all clear how callbacks > could something useful?
Sure, happy to do it that way. [PATCH: rmgr_002.v2.patch] > I don't think we should allocate a bunch of memory contexts to just free them > immediately after? Didn't seem a problem, but I've added code to use the flag requested above. > > > It occurs to me that any use of WAL presumes that Checkpoints exist > > > and do something useful. However, the custom rmgr interface doesn't > > > allow you to specify any actions on checkpoint, so ends up being > > > limited in scope. So I think we also need an rm_checkpoint() call - > > > which would be a no-op for existing rmgrs. > > > [PATCH: rmgr_003.v1.patch] > > > > I also like this idea, but can you describe the intended use case? I > > looked through CheckPointGuts() and I'm not sure what else a custom AM > > might want to do. Maybe sync special files in a way that's not handled > > with RegisterSyncRequest()? > > I'm not happy with the idea of random code being executed in the middle of > CheckPointGuts(), without any documentation of what is legal to do at that > point. The "I'm not happy.." ship has already sailed with pluggable rmgrs. Checkpoints exist for one purpose - as the starting place for recovery. Why would we allow pluggable recovery without *also* allowing pluggable checkpoints? >To actually be useful we'd likely need multiple calls to such an rmgr > callback, with a parameter where in CheckPointGuts() we are. Right now the > sequencing is explicit in CheckPointGuts(), but with the proposed callback, > that'd not be the case anymore. It is useful without the extra complexity you mention. I see multiple uses for the rm_checkpoint() point proposed and I've been asked multiple times for a checkpoint hook. Any rmgr that services crash recovery for a non-smgr based storage system would need this because the current checkpoint code only handles flushing to disk for smgr-based approaches. That is orthogonal to other code during checkpoint, so it stands alone quite well. -- Simon Riggs http://www.EnterpriseDB.com/
rmgr_002.v2.patch
Description: Binary data
