On Mon, May 06, 2024 at 06:55:46PM +0300, m.litsarev@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
