From ec1616f4faa51b75c1c04a26ee4f8a8148d6a866 Mon Sep 17 00:00:00 2001 From: Alexandre Felipe Date: Sun, 4 Oct 2026 13:09:31 +0100 Subject: [PATCH] Remove ckpt_lck. This patch updates checkpointer.c removing the lock on the the checkpointer state. The ckpt_lck in CheckpointerShmem guards a 2-state automaton, with states IDLE and ACTIVE transitions STARTED, FAILED, and DONE, and language: (STARTED (FAILED | DONE))* The protected data is .ckpt_failed, .ckpt_done and .skpt_started. The state transitions are all handled by CheckpointerMain: FAILED 317 SpinLockAcquire(&CheckpointerShmem->ckpt_lck); 318 CheckpointerShmem->ckpt_failed++; 319 CheckpointerShmem->ckpt_done = CheckpointerShmem->ckpt_started; 320 SpinLockRelease(&CheckpointerShmem->ckpt_lck); STARTED: 418 if (do_checkpoint) 431 SpinLockAcquire(&CheckpointerShmem->ckpt_lck); 432 flags |= CheckpointerShmem->ckpt_flags; 433 CheckpointerShmem->ckpt_flags = 0; 434 CheckpointerShmem->ckpt_started++; 435 SpinLockRelease(&CheckpointerShmem->ckpt_lck); DONE 512 SpinLockAcquire(&CheckpointerShmem->ckpt_lck); 513 CheckpointerShmem->ckpt_done = CheckpointerShmem->ckpt_started; 514 SpinLockRelease(&CheckpointerShmem->ckpt_lck); So writing this on an automaton notation STARTED { output .flags .flags = 0; .ckpt_started = .ckpt_started + 1; } FAILED { .ckpt_failed = .ckpt_failed + 1; .ckpt_done = .ckpt_started; } DONE { .ckpt_done = .ckpt_started; } Besides that .ckpt_flags can be modified by calling RequestCheckpoint and Request uses the counters to wait for the checkpoint and detect failures. It uses the counters .ckpt_done, .ckpt_started, and .ckpt_failed REQUEST: Set ckpt_flags, save .ckpt_started and .ckpt_failed 1096 SpinLockAcquire(&CheckpointerShmem->ckpt_lck); 1098 old_failed = CheckpointerShmem->ckpt_failed; 1099 old_started = CheckpointerShmem->ckpt_started; 1100 CheckpointerShmem->ckpt_flags |= (flags | CHECKPOINT_REQUESTED); 1102 SpinLockRelease(&CheckpointerShmem->ckpt_lck); Wait for STARTED: (More precisely .* STARTED .*) 1150 for (;;) 1151 { 1152 SpinLockAcquire(&CheckpointerShmem->ckpt_lck); 1153 new_started = CheckpointerShmem->ckpt_started; 1154 SpinLockRelease(&CheckpointerShmem->ckpt_lck); 1155 1156 if (new_started != old_started) 1157 break; 1158 1159 ConditionVariableSleep(&CheckpointerShmem->start_cv, 1160 WAIT_EVENT_CHECKPOINT_START); 1161 } Wait for FAILED | DONE, (More precisely .* (FAILED | DONE)) 1168 for (;;) 1169 { 1170 int new_done; 1171 1172 SpinLockAcquire(&CheckpointerShmem->ckpt_lck); 1173 new_done = CheckpointerShmem->ckpt_done; 1174 new_failed = CheckpointerShmem->ckpt_failed; 1175 SpinLockRelease(&CheckpointerShmem->ckpt_lck); 1176 1177 if (new_done - new_started >= 0) 1178 break; 1179 1180 ConditionVariableSleep(&CheckpointerShmem->done_cv, 1181 WAIT_EVENT_CHECKPOINT_DONE); 1182 } And detect failures 1185 if (new_failed != old_failed) 1186 ereport(ERROR, 1187 (errmsg("checkpoint request failed"), 1188 errhint("Consult recent messages in the server log for details."))); Appart from that FirstCallSinceLastCheckpoint reads .ckpt_done 1520 SpinLockAcquire(&CheckpointerShmem->ckpt_lck); 1521 new_done = CheckpointerShmem->ckpt_done; 1522 SpinLockRelease(&CheckpointerShmem->ckpt_lck); RequestCheckpoint has a surprising feature, that with CHECKPOINT_WAIT it reports failures from unrelated checkpoints, e.g. FAILED STARTED DONE or STARTED DONE STARTED FAILED The second case is unlikely if checkpoints happens less than once per second, however, the first is more likely, if a checkpoint. The first only requires RequestCheckpoint to be called during a checkpiont that ends up failing. This behaviour actually, makes our first transformation much simpler We can transform the state machine to (STARTED FAILED? DONE)* STARTED { output .ckpt_flags .ckpt_flags = 0; .ckpt_started = .ckpt_started + 1; } FAILED { .ckpt_failed = .ckpt_failed + 1; } DONE { .ckpt_done = .ckpt_started; } All the checkpointing is controlled by the flags, RequestCheckpoint set flags, and CheckPoinerMain clears it and take the actions; .ckpt_start, .ckpt_done, .ckpt_failed is only used to detect the the end of a checkpoint in RequestCheckpoint. .ckpt_flags can has to be atomic, and serves as the communication channel between RequestCheckpoint and CheckpointerMain; .ckpt_started and .ckpt_failed is implemented as a counter but they are only used to detect that one or more START or FAILED transitions happened. To detect one or more ocurrences of an event a regular increment is good enough, a race between multiple increments will still result in at least one increment. Under a single checkpointer there is no race on those indicators, and they serve as counters until they wrap arround. There is still one possible race between STARTED and REQUEST, on this race we could miss an increment on .ckpt_started, and thus not detecting that the checkpointer saw our flags. This race will be addressed by storing a version number on the same word as the flags. As a minor behavioural change, RequestCheckpoint might show a different error message when a checkpoint started before the checkpoint request fails after it sets the flags. --- src/backend/postmaster/checkpointer.c | 135 +++++++++++++++----------- src/include/access/xlog.h | 12 +++ 2 files changed, 89 insertions(+), 58 deletions(-) diff --git a/src/backend/postmaster/checkpointer.c b/src/backend/postmaster/checkpointer.c index 580c7944119..7903fa9f734 100644 --- a/src/backend/postmaster/checkpointer.c +++ b/src/backend/postmaster/checkpointer.c @@ -77,25 +77,26 @@ * * The ckpt counters allow backends to watch for completion of a checkpoint * request they send. Here's how it works: - * * At start of a checkpoint, checkpointer reads (and clears) the request - * flags and increments ckpt_started, while holding ckpt_lck. + * * At start of a checkpoint, checkpointer sets ckpt_started != ckpt_done + * and reset ckpt_state, clearing all its flags and advancing its version. * * On completion of a checkpoint, checkpointer sets ckpt_done to * equal ckpt_started. - * * On failure of a checkpoint, checkpointer increments ckpt_failed + * * On failure of a checkpoint, checkpointer changes ckpt_failed * and sets ckpt_done to equal ckpt_started. * * The algorithm for backends is: - * 1. Record current values of ckpt_failed and ckpt_started, and - * set request flags, while holding ckpt_lck. + * 1. Record current values of ckpt_done, ckpt_failed and ckpt_started, + * then atomically set request flags saving the previous state. * 2. Send signal to request checkpoint. - * 3. Sleep until ckpt_started changes. Now you know a checkpoint has + * 3. If ckpt_state version didn't change since the flags were set, + * sleep until ckpt_started changes. Now you know a checkpoint has * begun since you started this algorithm (although *not* that it was * specifically initiated by your signal), and that it is using your flags. - * 4. Record new value of ckpt_started. - * 5. Sleep until ckpt_done >= saved value of ckpt_started. (Use modulo - * arithmetic here in case counters wrap around.) Now you know a - * checkpoint has started and completed, but not whether it was - * successful. + * 4. Record new value of ckpt_started. At this stage if ckpt_failed + * has changed since step 1, report the early error. + * 5. Sleep until ckpt_done - ckpt_started >= 0 value of ckpt_started, + * checkpoint has started and completed, even after ckpt_started, wraps + * around, but not whether it was successful. * 6. If ckpt_failed is different from the originally saved value, * assume request failed; otherwise it was definitely successful. * @@ -120,13 +121,11 @@ typedef struct { pid_t checkpointer_pid; /* PID (0 if not started) */ - slock_t ckpt_lck; /* protects all the ckpt_* fields */ + volatile int ckpt_started; /* advances when checkpoint starts */ + volatile int ckpt_done; /* advances when checkpoint done */ + volatile int ckpt_failed; /* advances when checkpoint fails */ - int ckpt_started; /* advances when checkpoint starts */ - int ckpt_done; /* advances when checkpoint done */ - int ckpt_failed; /* advances when checkpoint fails */ - - int ckpt_flags; /* checkpoint flags, as defined in xlog.h */ + pg_atomic_uint32 ckpt_state; /* checkpoint state, as defined in xlog.h */ ConditionVariable start_cv; /* signaled when ckpt_started advances */ ConditionVariable done_cv; /* signaled when ckpt_done advances */ @@ -314,10 +313,9 @@ CheckpointerMain(const void *startup_data, size_t startup_data_len) /* Warn any waiting backends that the checkpoint failed. */ if (ckpt_active) { - SpinLockAcquire(&CheckpointerShmem->ckpt_lck); CheckpointerShmem->ckpt_failed++; + pg_memory_barrier(); CheckpointerShmem->ckpt_done = CheckpointerShmem->ckpt_started; - SpinLockRelease(&CheckpointerShmem->ckpt_lck); ConditionVariableBroadcast(&CheckpointerShmem->done_cv); @@ -367,6 +365,8 @@ CheckpointerMain(const void *startup_data, size_t startup_data_len) { bool do_checkpoint = false; int flags = 0; + uint32 state = 0; + uint32 last_state = 0; pg_time_t now; int elapsed_secs; int cur_timeout; @@ -387,10 +387,10 @@ CheckpointerMain(const void *startup_data, size_t startup_data_len) /* * Detect a pending checkpoint request by checking whether the flags - * word in shared memory is nonzero. We shouldn't need to acquire the - * ckpt_lck for this. + * word in shared memory is nonzero. */ - if (((volatile CheckpointerShmemStruct *) CheckpointerShmem)->ckpt_flags) + state = pg_atomic_read_u32(&CheckpointerShmem->ckpt_state); + if (state != last_state) { do_checkpoint = true; chkpt_or_rstpt_requested = true; @@ -424,15 +424,24 @@ CheckpointerMain(const void *startup_data, size_t startup_data_len) do_restartpoint = RecoveryInProgress(); /* - * Atomically fetch the request flags to figure out what kind of a - * checkpoint we should perform, and increase the started-counter - * to acknowledge that we've started a new checkpoint. + * Increment ckpt_started, indicating to any waiting backend that + * holds a snapshot of it that we are taking their flags. */ - SpinLockAcquire(&CheckpointerShmem->ckpt_lck); - flags |= CheckpointerShmem->ckpt_flags; - CheckpointerShmem->ckpt_flags = 0; CheckpointerShmem->ckpt_started++; - SpinLockRelease(&CheckpointerShmem->ckpt_lck); + + /* + * CAS on the state, copying and clearing the flags, and advancing + * state version. The state version lets waiting backends who observed + * ckpt_started after the increment know that their flags have been + * taken. + */ + for(;;){ + pg_atomic_uint32 *ckpt_state = &CheckpointerShmem->ckpt_state; + last_state = CHECKPOINT_ROTATE_STATE(state); + if(pg_atomic_compare_exchange_u32(ckpt_state, &state, last_state)) + break; + } + flags |= state & CHECKPOINT_FLAGS_MASK; ConditionVariableBroadcast(&CheckpointerShmem->start_cv); @@ -509,9 +518,7 @@ CheckpointerMain(const void *startup_data, size_t startup_data_len) /* * Indicate checkpoint completion to any waiting backends. */ - SpinLockAcquire(&CheckpointerShmem->ckpt_lck); CheckpointerShmem->ckpt_done = CheckpointerShmem->ckpt_started; - SpinLockRelease(&CheckpointerShmem->ckpt_lck); ConditionVariableBroadcast(&CheckpointerShmem->done_cv); @@ -580,7 +587,8 @@ CheckpointerMain(const void *startup_data, size_t startup_data_len) * If any checkpoint flags have been set, redo the loop to handle the * checkpoint without sleeping. */ - if (((volatile CheckpointerShmemStruct *) CheckpointerShmem)->ckpt_flags) + state = pg_atomic_read_u32(&CheckpointerShmem->ckpt_state); + if (state != last_state) continue; /* @@ -767,13 +775,7 @@ CheckArchiveTimeout(void) static bool FastCheckpointRequested(void) { - volatile CheckpointerShmemStruct *cps = CheckpointerShmem; - - /* - * We don't need to acquire the ckpt_lck in this case because we're only - * looking at a single flag bit. - */ - if (cps->ckpt_flags & CHECKPOINT_FAST) + if (pg_atomic_read_u32(&CheckpointerShmem->ckpt_state) & CHECKPOINT_FAST) return true; return false; } @@ -984,7 +986,6 @@ CheckpointerShmemRequest(void *arg) static void CheckpointerShmemInit(void *arg) { - SpinLockInit(&CheckpointerShmem->ckpt_lck); CheckpointerShmem->max_requests = Min(NBuffers, MAX_CHECKPOINT_REQUESTS); CheckpointerShmem->head = CheckpointerShmem->tail = 0; ConditionVariableInit(&CheckpointerShmem->start_cv); @@ -1066,6 +1067,10 @@ RequestCheckpoint(int flags) int ntries; int old_failed, old_started; + int new_failed, + new_started, + new_done; + uint32 old_state; /* * If in a standalone backend, just do it ourselves. @@ -1085,22 +1090,34 @@ RequestCheckpoint(int flags) } /* - * Atomically set the request flags, and take a snapshot of the counters. - * When we see ckpt_started > old_started, we know the flags we set here - * have been seen by checkpointer. + * Take a snapshot of .ckpt_done, .ckpt_started, .ckpt_failed before we + * set the request flags, for later comparison. * * Note that we OR the flags with any existing flags, to avoid overriding * a "stronger" request by another backend. The flag senses must be * chosen to make this work! */ - SpinLockAcquire(&CheckpointerShmem->ckpt_lck); - - old_failed = CheckpointerShmem->ckpt_failed; old_started = CheckpointerShmem->ckpt_started; - CheckpointerShmem->ckpt_flags |= (flags | CHECKPOINT_REQUESTED); - - SpinLockRelease(&CheckpointerShmem->ckpt_lck); + old_failed = CheckpointerShmem->ckpt_failed; + old_state = pg_atomic_fetch_or_u32(&CheckpointerShmem->ckpt_state, flags | CHECKPOINT_REQUESTED); + /* + * ckpt_started might have changed since we observed it, in that case, + * checkpointer might have taken the flags before or after we set ours. + */ + new_started = CheckpointerShmem->ckpt_started; + if (new_started != old_started) + { + uint32 new_state = pg_atomic_read_membarrier_u32(&CheckpointerShmem->ckpt_state); + if (((new_state ^ old_state) & ~CHECKPOINT_FLAGS_MASK) == 0) + { + /* + * state version didn't change, so we will have to wait + * for the next checkpoint. + */ + old_started = new_started; + } + } /* * Set checkpointer's latch to request checkpoint. It's possible that the * checkpointer hasn't started yet, so we will retry a few times if @@ -1142,16 +1159,11 @@ RequestCheckpoint(int flags) */ if (flags & CHECKPOINT_WAIT) { - int new_started, - new_failed; - /* Wait for a new checkpoint to start. */ ConditionVariablePrepareToSleep(&CheckpointerShmem->start_cv); for (;;) { - SpinLockAcquire(&CheckpointerShmem->ckpt_lck); new_started = CheckpointerShmem->ckpt_started; - SpinLockRelease(&CheckpointerShmem->ckpt_lck); if (new_started != old_started) break; @@ -1161,6 +1173,16 @@ RequestCheckpoint(int flags) } ConditionVariableCancelSleep(); + new_failed = CheckpointerShmem->ckpt_failed; + new_done = CheckpointerShmem->ckpt_done; + /* + * Distinguish failures of checkoints that started before this request + * from failures of those that started after it. + */ + if (new_failed != old_failed && new_done == old_started) + ereport(ERROR, + (errmsg("checkpoint request failed before done"), + errhint("Consult recent messages in the server log for details."))); /* * We are waiting for ckpt_done >= new_started, in a modulo sense. */ @@ -1169,10 +1191,9 @@ RequestCheckpoint(int flags) { int new_done; - SpinLockAcquire(&CheckpointerShmem->ckpt_lck); new_done = CheckpointerShmem->ckpt_done; + pg_memory_barrier(); new_failed = CheckpointerShmem->ckpt_failed; - SpinLockRelease(&CheckpointerShmem->ckpt_lck); if (new_done - new_started >= 0) break; @@ -1517,9 +1538,7 @@ FirstCallSinceLastCheckpoint(void) int new_done; bool FirstCall = false; - SpinLockAcquire(&CheckpointerShmem->ckpt_lck); new_done = CheckpointerShmem->ckpt_done; - SpinLockRelease(&CheckpointerShmem->ckpt_lck); if (new_done != ckpt_done) FirstCall = true; diff --git a/src/include/access/xlog.h b/src/include/access/xlog.h index 7a590b7e1ea..ebd5db9d38e 100644 --- a/src/include/access/xlog.h +++ b/src/include/access/xlog.h @@ -174,6 +174,18 @@ extern PGDLLIMPORT bool XLOG_DEBUG; #define CHECKPOINT_CAUSE_XLOG 0x0080 /* XLOG consumption */ #define CHECKPOINT_CAUSE_TIME 0x0100 /* Elapsed time */ +/* Checkpoint state version. + * + * These bits are atomically rotated by the checkpointer when resetting + * the flags stored by the checkpoint requests. + */ +#define CHECKPOINT_NUM_FLAGS 9 /* number of bits used for flags */ +#define CHECKPOINT_VERSION_LSB (1 << CHECKPOINT_NUM_FLAGS) +#define CHECKPOINT_FLAGS_MASK (CHECKPOINT_VERSION_LSB - 1) +/* Clear flags and advance one version */ +#define CHECKPOINT_ROTATE_STATE(state) \ + (((state) + CHECKPOINT_VERSION_LSB) & ~CHECKPOINT_FLAGS_MASK) + /* * Flag bits for the record being inserted, set using XLogSetRecordFlags(). */ -- 2.53.0