On Mon, May 06, 2024 at 06:55:46PM +0300, m.litsa...@postgrespro.ru wrote: > as a first step I have introduced the `SharedRecoveryDataFlags` bitmask > instead of three boolean SharedHotStandbyActive, SharedPromoteIsTriggered > and SharedStandbyModeRequested flags (the last one from my previous patch) > and made minimal updates in corresponding code based on that change.
Thanks for the patch. /* - * Local copy of SharedHotStandbyActive variable. False actually means "not + * Local copy of XLR_HOT_STANDBY_ACTIVE flag. False actually means "not * known, need to check the shared state". */ static bool LocalHotStandbyActive = false; /* - * Local copy of SharedPromoteIsTriggered variable. False actually means "not + * Local copy of XLR_PROMOTE_IS_TRIGGERED flag. False actually means "not * known, need to check the shared state". */ static bool LocalPromoteIsTriggered = false; It's a bit strange to have a bitwise set of flags in shmem while we keep these local copies as booleans. Perhaps it would be cleaner to merge both local variables into a single bits32 store? + uint32 SharedRecoveryDataFlags; I'd switch to bits32 for flags here. +bool +StandbyModeIsRequested(void) +{ + /* + * Spinlock is not needed here because XLR_STANDBY_MODE_REQUESTED flag + * can only be read after startup process is done. + */ + return (XLogRecoveryCtl->SharedRecoveryDataFlags & XLR_STANDBY_MODE_REQUESTED) != 0; +} How about introducing a single wrapper function that returns the whole value SharedRecoveryDataFlags, with the flags published in a header? Sure, XLR_HOT_STANDBY_ACTIVE is not really exciting because being able to query a standby implies it, but XLR_PROMOTE_IS_TRIGGERED could be interesting? Then this could be used with a function that returns a text[] array with all the states retrieved? The refactoring pieces and the function pieces should be split, for clarity. -- Michael
signature.asc
Description: PGP signature