public inbox for [email protected]  
help / color / mirror / Atom feed
misplaced GUC in pqcomm.h -- where to put actual common variable though...?
3+ messages / 2 participants
[nested] [flat]

* misplaced GUC in pqcomm.h -- where to put actual common variable though...?
@ 2023-03-22 02:43 Greg Stark <[email protected]>
  0 siblings, 0 replies; 3+ messages in thread

From: Greg Stark @ 2023-03-22 02:43 UTC (permalink / raw)
  To: PostgreSQL Hackers <[email protected]>

So back in 2002 in 7.3 there was a commit 2c6b34d9598 which added a
GUC db_user_namespace which is stored in a variable Db_user_namespace.
All that seems fine except...

The variable this GUC is stored in is Db_user_namespace which... is
actually declared in pqcomm.h which is intended to be "Definitions
common to frontends and backends".

Afaics it's never actually defined in any FE code, neither libpq nor
any clients. I was a bit surprised this isn't producing a warning
about an extern declaration that's never defined but I guess that's
not actually that unusual.

The actual variable is defined in the backend in postmaster.c. I'm
guessing this declaration can just move to libpq/libpq.h which
(counterintuitively) is for the backend iiuc.

I don't think this causes any actual problems aside from namespace
pollution but it confused me.  I found this because I was looking for
where to put the ALPN protocol version which (at least at the moment)
would be the same for the server and client. But as far as I can tell
it would be the only variable (other than the above) declared in both
and that means there's no particularly handy place to put the
definition.

--
greg






^ permalink  raw  reply  [nested|flat] 3+ messages in thread

* [PATCH v1 2/8] convert ParallelBitmapHeapState->state to an atomic
@ 2026-07-09 18:38 Nathan Bossart <[email protected]>
  0 siblings, 0 replies; 3+ messages in thread

From: Nathan Bossart @ 2026-07-09 18:38 UTC (permalink / raw)

---
 src/backend/executor/nodeBitmapHeapscan.c | 21 ++++++---------------
 1 file changed, 6 insertions(+), 15 deletions(-)

diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
index 83d6478bc2b..f2bb487bf10 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -79,7 +79,6 @@ typedef enum
 /* ----------------
  *	 ParallelBitmapHeapState information
  *		tbmiterator				iterator for scanning current pages
- *		mutex					mutual exclusion for state
  *		state					current state of the TIDBitmap
  *		cv						conditional wait variable
  * ----------------
@@ -87,8 +86,7 @@ typedef enum
 typedef struct ParallelBitmapHeapState
 {
 	dsa_pointer tbmiterator;
-	slock_t		mutex;
-	SharedBitmapState state;
+	pg_atomic_uint32 state;
 	ConditionVariable cv;
 } ParallelBitmapHeapState;
 
@@ -228,9 +226,7 @@ BitmapHeapNext(BitmapHeapScanState *node)
 static inline void
 BitmapDoneInitializingSharedState(ParallelBitmapHeapState *pstate)
 {
-	SpinLockAcquire(&pstate->mutex);
-	pstate->state = BM_FINISHED;
-	SpinLockRelease(&pstate->mutex);
+	pg_atomic_write_membarrier_u32(&pstate->state, BM_FINISHED);
 	ConditionVariableBroadcast(&pstate->cv);
 }
 
@@ -480,11 +476,8 @@ BitmapShouldInitializeSharedState(ParallelBitmapHeapState *pstate)
 
 	while (1)
 	{
-		SpinLockAcquire(&pstate->mutex);
-		state = pstate->state;
-		if (pstate->state == BM_INITIAL)
-			pstate->state = BM_INPROGRESS;
-		SpinLockRelease(&pstate->mutex);
+		state = BM_INITIAL;
+		pg_atomic_compare_exchange_u32(&pstate->state, &state, BM_INPROGRESS);
 
 		/* Exit if bitmap is done, or if we're the leader. */
 		if (state != BM_INPROGRESS)
@@ -538,9 +531,7 @@ ExecBitmapHeapInitializeDSM(BitmapHeapScanState *node,
 
 	pstate->tbmiterator = 0;
 
-	/* Initialize the mutex */
-	SpinLockInit(&pstate->mutex);
-	pstate->state = BM_INITIAL;
+	pg_atomic_init_u32(&pstate->state, BM_INITIAL);
 
 	ConditionVariableInit(&pstate->cv);
 
@@ -565,7 +556,7 @@ ExecBitmapHeapReInitializeDSM(BitmapHeapScanState *node,
 	if (dsa == NULL)
 		return;
 
-	pstate->state = BM_INITIAL;
+	pg_atomic_write_u32(&pstate->state, BM_INITIAL);
 
 	if (DsaPointerIsValid(pstate->tbmiterator))
 		tbm_free_shared_area(dsa, pstate->tbmiterator);
-- 
2.50.1 (Apple Git-155)


--Ot5x3ffkWEuxilWO
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment;
	filename=v1-0003-convert-FixedParallelState-last_xlog_end-to-an-at.patch



^ permalink  raw  reply  [nested|flat] 3+ messages in thread

* [PATCH v1 2/8] convert ParallelBitmapHeapState->state to an atomic
@ 2026-07-09 18:38 Nathan Bossart <[email protected]>
  0 siblings, 0 replies; 3+ messages in thread

From: Nathan Bossart @ 2026-07-09 18:38 UTC (permalink / raw)

---
 src/backend/executor/nodeBitmapHeapscan.c | 21 ++++++---------------
 1 file changed, 6 insertions(+), 15 deletions(-)

diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
index 83d6478bc2b..f2bb487bf10 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -79,7 +79,6 @@ typedef enum
 /* ----------------
  *	 ParallelBitmapHeapState information
  *		tbmiterator				iterator for scanning current pages
- *		mutex					mutual exclusion for state
  *		state					current state of the TIDBitmap
  *		cv						conditional wait variable
  * ----------------
@@ -87,8 +86,7 @@ typedef enum
 typedef struct ParallelBitmapHeapState
 {
 	dsa_pointer tbmiterator;
-	slock_t		mutex;
-	SharedBitmapState state;
+	pg_atomic_uint32 state;
 	ConditionVariable cv;
 } ParallelBitmapHeapState;
 
@@ -228,9 +226,7 @@ BitmapHeapNext(BitmapHeapScanState *node)
 static inline void
 BitmapDoneInitializingSharedState(ParallelBitmapHeapState *pstate)
 {
-	SpinLockAcquire(&pstate->mutex);
-	pstate->state = BM_FINISHED;
-	SpinLockRelease(&pstate->mutex);
+	pg_atomic_write_membarrier_u32(&pstate->state, BM_FINISHED);
 	ConditionVariableBroadcast(&pstate->cv);
 }
 
@@ -480,11 +476,8 @@ BitmapShouldInitializeSharedState(ParallelBitmapHeapState *pstate)
 
 	while (1)
 	{
-		SpinLockAcquire(&pstate->mutex);
-		state = pstate->state;
-		if (pstate->state == BM_INITIAL)
-			pstate->state = BM_INPROGRESS;
-		SpinLockRelease(&pstate->mutex);
+		state = BM_INITIAL;
+		pg_atomic_compare_exchange_u32(&pstate->state, &state, BM_INPROGRESS);
 
 		/* Exit if bitmap is done, or if we're the leader. */
 		if (state != BM_INPROGRESS)
@@ -538,9 +531,7 @@ ExecBitmapHeapInitializeDSM(BitmapHeapScanState *node,
 
 	pstate->tbmiterator = 0;
 
-	/* Initialize the mutex */
-	SpinLockInit(&pstate->mutex);
-	pstate->state = BM_INITIAL;
+	pg_atomic_init_u32(&pstate->state, BM_INITIAL);
 
 	ConditionVariableInit(&pstate->cv);
 
@@ -565,7 +556,7 @@ ExecBitmapHeapReInitializeDSM(BitmapHeapScanState *node,
 	if (dsa == NULL)
 		return;
 
-	pstate->state = BM_INITIAL;
+	pg_atomic_write_u32(&pstate->state, BM_INITIAL);
 
 	if (DsaPointerIsValid(pstate->tbmiterator))
 		tbm_free_shared_area(dsa, pstate->tbmiterator);
-- 
2.50.1 (Apple Git-155)


--Ot5x3ffkWEuxilWO
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment;
	filename=v1-0003-convert-FixedParallelState-last_xlog_end-to-an-at.patch



^ permalink  raw  reply  [nested|flat] 3+ messages in thread


end of thread, other threads:[~2026-07-09 18:38 UTC | newest]

Thread overview: 3+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2023-03-22 02:43 misplaced GUC in pqcomm.h -- where to put actual common variable though...? Greg Stark <[email protected]>
2026-07-09 18:38 [PATCH v1 2/8] convert ParallelBitmapHeapState->state to an atomic Nathan Bossart <[email protected]>
2026-07-09 18:38 [PATCH v1 2/8] convert ParallelBitmapHeapState->state to an atomic Nathan Bossart <[email protected]>

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox