Hi, On Mon, Oct 05, 2026 at 02:52:41PM +0900, Michael Paquier wrote: > It still feels a bit weird to call a callback from another callback, > but I don't quite see how we can avoid that.
FWIW, a similar pattern already exists in shutdown_validator_library(), where a memory context reset callback calls ValidatorCallbacks->shutdown_cb(). It makes sense to me here too: the reset callback decides when cleanup is required, while segment_close knows how to perform it, avoiding duplication of the cleanup logic. === 1 + if (state->seg.ws_file != -1) + state->routine.segment_close(state); The comment above segment_close() says that ws_file shall be set to a negative number, while both this callback and XLogReaderFree() check for -1. I wonder if they should test >= 0 instead? === 2 +/* + * Register a memory reset callback, closing a segment, if necessary. + * + * This is useful when opening a segment with BasicOpenFile(), to guarantee + * that the segment is closed before XLogReaderFree() is reached. + */ +void +XLogReaderRegisterReset(XLogReaderState *state) Worth mentioning that this is intended for descriptors not managed by another cleanup mechanism, such as those returned by BasicOpenFile()? Otherwise, using it with OpenTransientFile() could result in segment_close() being called with a stale descriptor. Regards, -- Bertrand Drouvot PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
