From 5a959f7c1b7f1bdb0f907b3f50338e73f7eadd48 Mon Sep 17 00:00:00 2001 From: SqlRush Date: Sat, 18 Jul 2026 12:04:25 +0800 Subject: [PATCH 1/2] fix(cluster): preserve conditional lock native paths Only require an exact PCM X grant when clustering is active and the buffer is tracked. Retained images and live ownership transitions remain fail-closed. Add behavior coverage for runtime-off, untracked, tracked, retained, and transition cases. --- src/backend/storage/buffer/bufmgr.c | 7 +++--- src/include/cluster/cluster_pcm_x_bufmgr.h | 12 ++++++++++ src/test/cluster_unit/test_cluster_pcm_own.c | 23 +++++++++++++++----- 3 files changed, 33 insertions(+), 9 deletions(-) diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c index 66116e950c..cf269efd39 100644 --- a/src/backend/storage/buffer/bufmgr.c +++ b/src/backend/storage/buffer/bufmgr.c @@ -8427,9 +8427,10 @@ ConditionalLockBuffer(Buffer buffer) * GRANT_PENDING must not modify protocol-owned bytes. */ buf_state = LockBufHdr(buf); - blocked = cluster_bufmgr_pcm_x_retained_image_locked(buf, buf_state) - || buf->pcm_state != (uint8) PCM_STATE_X - || cluster_pcm_own_flags_get(buf->buf_id) != 0; + blocked = !cluster_pcm_x_conditional_lock_allowed( + cluster_pcm_is_active(), cluster_bufmgr_should_pcm_track(buf), + cluster_bufmgr_pcm_x_retained_image_locked(buf, buf_state), buf->pcm_state, + cluster_pcm_own_flags_get(buf->buf_id)); UnlockBufHdr(buf, buf_state); if (blocked) { diff --git a/src/include/cluster/cluster_pcm_x_bufmgr.h b/src/include/cluster/cluster_pcm_x_bufmgr.h index 904789b55d..6bd548d26c 100644 --- a/src/include/cluster/cluster_pcm_x_bufmgr.h +++ b/src/include/cluster/cluster_pcm_x_bufmgr.h @@ -165,6 +165,18 @@ cluster_pcm_x_cached_cover_bypasses_queue(bool local_cache, bool requested_x, ui return local_cache && requested_x && pcm_state == (uint8)PCM_STATE_X && flags == 0; } +/* ConditionalLockBuffer cannot initiate a PCM conversion. Preserve native + * PostgreSQL behavior while PCM is inactive and for relations outside the + * coherence domain; an active tracked page must already hold exact X. Live + * transition/retained evidence remains closed regardless of runtime state. */ +static inline bool +cluster_pcm_x_conditional_lock_allowed(bool runtime_active, bool tracked, bool retained_image, + uint8 pcm_state, uint32 flags) +{ + return !retained_image && flags == 0 + && (!runtime_active || !tracked || pcm_state == (uint8)PCM_STATE_X); +} + typedef ClusterPcmOwnSnapshot ClusterPcmOwnEvictionCapture; typedef enum ClusterPcmXGrantReservationKind { diff --git a/src/test/cluster_unit/test_cluster_pcm_own.c b/src/test/cluster_unit/test_cluster_pcm_own.c index bef70a7d75..ebf48284a0 100644 --- a/src/test/cluster_unit/test_cluster_pcm_own.c +++ b/src/test/cluster_unit/test_cluster_pcm_own.c @@ -1211,17 +1211,15 @@ UT_TEST(test_retained_image_release_and_writeback_gates_are_exact) if (conditional != NULL) { const char *content = strstr(conditional, "LWLockConditionalAcquire(BufferDescriptorGetContentLock(buf)"); - const char *ownership = strstr(conditional, "buf->pcm_state != (uint8) PCM_STATE_X"); - const char *flags = strstr(conditional, "cluster_pcm_own_flags_get(buf->buf_id) != 0"); + const char *ownership = strstr(conditional, "cluster_pcm_x_conditional_lock_allowed("); const char *release = strstr(conditional, "LWLockRelease(BufferDescriptorGetContentLock(buf))"); UT_ASSERT_NOT_NULL(content); UT_ASSERT_NOT_NULL(ownership); - UT_ASSERT_NOT_NULL(flags); UT_ASSERT_NOT_NULL(release); - if (content != NULL && ownership != NULL && flags != NULL && release != NULL) - UT_ASSERT(content < ownership && ownership < flags && flags < release); + if (content != NULL && ownership != NULL && release != NULL) + UT_ASSERT(content < ownership && ownership < release); } if (resident_stamp != NULL) { const char *content = strstr( @@ -1659,6 +1657,18 @@ UT_TEST(test_current_image_shape_accepts_monotone_xcur_after_x_to_s_yield) UT_ASSERT(!cluster_pcm_x_current_image_shape((uint8)PCM_STATE_S, (uint8)BUF_TYPE_XCUR, false)); } +UT_TEST(test_conditional_lock_preserves_native_off_and_enforces_tracked_x) +{ + UT_ASSERT(cluster_pcm_x_conditional_lock_allowed(false, true, false, (uint8)PCM_STATE_N, 0)); + UT_ASSERT(cluster_pcm_x_conditional_lock_allowed(true, false, false, (uint8)PCM_STATE_N, 0)); + UT_ASSERT(!cluster_pcm_x_conditional_lock_allowed(true, true, false, (uint8)PCM_STATE_N, 0)); + UT_ASSERT(!cluster_pcm_x_conditional_lock_allowed(true, true, false, (uint8)PCM_STATE_S, 0)); + UT_ASSERT(cluster_pcm_x_conditional_lock_allowed(true, true, false, (uint8)PCM_STATE_X, 0)); + UT_ASSERT(!cluster_pcm_x_conditional_lock_allowed(false, false, true, (uint8)PCM_STATE_X, 0)); + UT_ASSERT(!cluster_pcm_x_conditional_lock_allowed(false, false, false, (uint8)PCM_STATE_X, + PCM_OWN_FLAG_GRANT_PENDING)); +} + UT_TEST(test_queue_passive_n_mirror_is_never_gcs_ship_authority) { static const char *const probe_contract[] @@ -1977,7 +1987,7 @@ UT_TEST(test_lockbuffer_pcm_x_writer_ledger_is_distinct_and_brackets_content_aut int main(void) { - UT_PLAN(44); + UT_PLAN(45); UT_RUN(test_shmem_initializes_complete_entry); UT_RUN(test_begin_abort_is_exact_and_monotonic); UT_RUN(test_invalid_live_flag_shapes_are_corrupt_not_busy); @@ -2017,6 +2027,7 @@ main(void) UT_RUN(test_queue_holder_snapshot_by_tag_is_mapping_and_header_exact); UT_RUN(test_queue_passive_pinned_s_release_serializes_bytes_and_ownership); UT_RUN(test_current_image_shape_accepts_monotone_xcur_after_x_to_s_yield); + UT_RUN(test_conditional_lock_preserves_native_off_and_enforces_tracked_x); UT_RUN(test_queue_installed_image_publication_is_exact_and_content_locked); UT_RUN(test_queue_self_source_handoff_is_single_lifecycle_and_readonly_drain); UT_RUN(test_queue_passive_n_mirror_is_never_gcs_ship_authority); From 06d24467e788e585162c5fcec5a880b1565180e0 Mon Sep 17 00:00:00 2001 From: SqlRush Date: Sat, 18 Jul 2026 12:04:25 +0800 Subject: [PATCH 2/2] fix(cluster): count recovering PCM-X image fetches Expose retryable RESOURCE_RECOVERING image-fetch denials independently from master-not-holder retries and roll the gcs_recovery dump baseline. --- src/backend/cluster/cluster_debug.c | 2 ++ src/backend/cluster/cluster_gcs_block.c | 27 ++++++++++++++----- src/include/cluster/cluster_gcs_block.h | 1 + .../t/251_gcs_pcm_warm_recovery.pl | 12 ++++----- src/test/cluster_unit/test_cluster_debug.c | 5 ++++ .../cluster_unit/test_cluster_gcs_block.c | 6 +++++ 6 files changed, 41 insertions(+), 12 deletions(-) diff --git a/src/backend/cluster/cluster_debug.c b/src/backend/cluster/cluster_debug.c index 791e58d534..85134ca511 100644 --- a/src/backend/cluster/cluster_debug.c +++ b/src/backend/cluster/cluster_debug.c @@ -2411,6 +2411,8 @@ dump_gcs(ReturnSetInfo *rsinfo) /* PGRAC: spec-4.7 D6 — 8 NEW counter rows for GCS/PCM warm recovery. */ emit_row(rsinfo, "gcs_recovery", "block_resources_recovering", fmt_int64((int64)cluster_gcs_get_recovery_block_resources_recovering())); + emit_row(rsinfo, "gcs_recovery", "pcm_x_image_fetch_recovering_retry_count", + fmt_int64((int64)cluster_gcs_get_pcm_x_image_fetch_recovering_retry_count())); emit_row(rsinfo, "gcs_recovery", "buffers_redeclared", fmt_int64((int64)cluster_gcs_get_recovery_buffers_redeclared())); emit_row(rsinfo, "gcs_recovery", "block_state_rebuilt", diff --git a/src/backend/cluster/cluster_gcs_block.c b/src/backend/cluster/cluster_gcs_block.c index 3afc094ba3..90791d1a8f 100644 --- a/src/backend/cluster/cluster_gcs_block.c +++ b/src/backend/cluster/cluster_gcs_block.c @@ -297,13 +297,16 @@ typedef struct ClusterGcsBlockShared { clean_page_xfer_stale_holder_recover_count; /* RESERVED Stage 6 (was DENIED recover) */ pg_atomic_uint64 clean_page_xfer_third_party_denied_count; /* 3-node third-party master DENY */ /* PGRAC: spec-4.7 D6 — GCS/PCM warm-recovery observability (dump category - * 'gcs_recovery'). 8 counters per spec §2.4. */ + * 'gcs_recovery'). */ pg_atomic_uint64 recovery_block_resources_recovering; /* phase_for_tag → RECOVERING hits */ - pg_atomic_uint64 recovery_buffers_redeclared; /* survivor re-declare sent (D2) */ - pg_atomic_uint64 recovery_block_state_rebuilt; /* master rebuild applied (D2/D3) */ - pg_atomic_uint64 recovery_redo_boundary_waits; /* redo gate: not yet covered (D5) */ - pg_atomic_uint64 recovery_redo_boundary_reached; /* redo gate: covered (D5) */ - pg_atomic_uint64 recovery_stale_block_drop; /* re-declare dropped: off-epoch/bad (D2) */ + /* PCM-X requester received a retryable RESOURCE_RECOVERING denial while + * fetching its generation-exact holder image. */ + pg_atomic_uint64 pcm_x_image_fetch_recovering_retry_count; + pg_atomic_uint64 recovery_buffers_redeclared; /* survivor re-declare sent (D2) */ + pg_atomic_uint64 recovery_block_state_rebuilt; /* master rebuild applied (D2/D3) */ + pg_atomic_uint64 recovery_redo_boundary_waits; /* redo gate: not yet covered (D5) */ + pg_atomic_uint64 recovery_redo_boundary_reached; /* redo gate: covered (D5) */ + pg_atomic_uint64 recovery_stale_block_drop; /* re-declare dropped: off-epoch/bad (D2) */ pg_atomic_uint64 recovery_ambiguous_owner_failclosed; /* not-double-X conflict (D3) */ pg_atomic_uint64 recovery_before_boundary_failclosed; /* served-before-redo gate fail (D5) */ /* PGRAC: spec-2.36 D3 (HC116) — master broadcast invalidate slot. @@ -600,6 +603,7 @@ cluster_gcs_block_shmem_init(void) pg_atomic_init_u64(&ClusterGcsBlock->clean_page_xfer_third_party_denied_count, 0); /* PGRAC: spec-4.7 D6 — 8 NEW warm-recovery counters init. */ pg_atomic_init_u64(&ClusterGcsBlock->recovery_block_resources_recovering, 0); + pg_atomic_init_u64(&ClusterGcsBlock->pcm_x_image_fetch_recovering_retry_count, 0); pg_atomic_init_u64(&ClusterGcsBlock->recovery_buffers_redeclared, 0); pg_atomic_init_u64(&ClusterGcsBlock->recovery_block_state_rebuilt, 0); pg_atomic_init_u64(&ClusterGcsBlock->recovery_redo_boundary_waits, 0); @@ -3379,6 +3383,9 @@ cluster_gcs_pcm_x_fetch_image_and_install(BufferDesc *buf, const PcmXLocalHandle || reply.status == (uint8)GCS_BLOCK_REPLY_DENIED_RESOURCE_RECOVERING) { if (reply.status == (uint8)GCS_BLOCK_REPLY_DENIED_MASTER_NOT_HOLDER) pg_atomic_fetch_add_u64(&ClusterGcsBlock->block_master_not_holder_count, 1); + else + pg_atomic_fetch_add_u64( + &ClusterGcsBlock->pcm_x_image_fetch_recovering_retry_count, 1); continue; } if (!cluster_pcm_x_image_fetch_reply_exact(&reply, reply_block, &progress_now, @@ -14480,6 +14487,14 @@ cluster_gcs_get_recovery_block_resources_recovering(void) ? pg_atomic_read_u64(&ClusterGcsBlock->recovery_block_resources_recovering) : 0; } + +uint64 +cluster_gcs_get_pcm_x_image_fetch_recovering_retry_count(void) +{ + return ClusterGcsBlock + ? pg_atomic_read_u64(&ClusterGcsBlock->pcm_x_image_fetch_recovering_retry_count) + : 0; +} uint64 cluster_gcs_get_recovery_buffers_redeclared(void) { diff --git a/src/include/cluster/cluster_gcs_block.h b/src/include/cluster/cluster_gcs_block.h index d4f02ce8fc..ce2d1ddaf5 100644 --- a/src/include/cluster/cluster_gcs_block.h +++ b/src/include/cluster/cluster_gcs_block.h @@ -3513,6 +3513,7 @@ extern void cluster_gcs_block_send_pi_discard_invalidate(BufferTag tag, int32 ta /* PGRAC: spec-4.7 D6 — 8 warm-recovery observability accessors. */ extern uint64 cluster_gcs_get_recovery_block_resources_recovering(void); +extern uint64 cluster_gcs_get_pcm_x_image_fetch_recovering_retry_count(void); extern uint64 cluster_gcs_get_recovery_buffers_redeclared(void); extern uint64 cluster_gcs_get_recovery_block_state_rebuilt(void); extern uint64 cluster_gcs_get_recovery_redo_boundary_waits(void); diff --git a/src/test/cluster_tap/t/251_gcs_pcm_warm_recovery.pl b/src/test/cluster_tap/t/251_gcs_pcm_warm_recovery.pl index 0813f66980..5c60950cae 100644 --- a/src/test/cluster_tap/t/251_gcs_pcm_warm_recovery.pl +++ b/src/test/cluster_tap/t/251_gcs_pcm_warm_recovery.pl @@ -69,7 +69,7 @@ # L2/L3 (sig b/c) DEFERRED to D1-D5 acceptance + cluster_unit (L239, # see finding above). SKIPed here with reason. # Lobs (D6/observability, FLIPPED) gcs_recovery dump category exposes the -# 8 warm-recovery counters under category='gcs_recovery'. +# warm-recovery counters under category='gcs_recovery'. # # Portions Copyright (c) 1996-2024, PostgreSQL Global Development Group # Portions Copyright (c) 1994, Regents of the University of California @@ -215,19 +215,19 @@ # 8 warm-recovery counters: block_resources_recovering / buffers_redeclared / # block_state_rebuilt / redo_boundary_waits / redo_boundary_reached / # stale_block_drop / ambiguous_owner_failclosed / before_boundary_failclosed). -# spec-2.41 D7 adds 2 redo-coverage serve-gate counters to the same category -# (redo_coverage_required_lsn_zero_count / redo_coverage_gate_block_count) → 10. +# spec-2.41 D7 adds 2 redo-coverage serve-gate counters to the same category; +# PCM-X adds one image-fetch RESOURCE_RECOVERING retry counter → 11. # ---------- is($triple->node0->safe_psql('postgres', q{SELECT count(*) FROM cluster_dump_state() WHERE category = 'gcs_recovery'}), - '10', - 'Lobs (D6 flipped + spec-2.41 D7): gcs_recovery dump category exposes 10 ' + '11', + 'Lobs (D6 flipped + spec-2.41 D7 + PCM-X): gcs_recovery dump category exposes 11 ' . 'counters — 8 warm-recovery (block_resources_recovering / buffers_redeclared / ' . 'block_state_rebuilt / redo_boundary_waits / redo_boundary_reached / ' . 'stale_block_drop / ambiguous_owner_failclosed / before_boundary_failclosed) ' . '+ 2 redo-coverage serve-gate (redo_coverage_required_lsn_zero_count / ' - . 'redo_coverage_gate_block_count)'); + . 'redo_coverage_gate_block_count) + 1 PCM-X image-fetch recovering retry'); $triple->stop_triple; diff --git a/src/test/cluster_unit/test_cluster_debug.c b/src/test/cluster_unit/test_cluster_debug.c index c7f08e9b00..be7c737f80 100644 --- a/src/test/cluster_unit/test_cluster_debug.c +++ b/src/test/cluster_unit/test_cluster_debug.c @@ -1650,6 +1650,11 @@ cluster_gcs_get_recovery_block_resources_recovering(void) return 0; } uint64 +cluster_gcs_get_pcm_x_image_fetch_recovering_retry_count(void) +{ + return 0; +} +uint64 cluster_gcs_get_recovery_buffers_redeclared(void) { return 0; diff --git a/src/test/cluster_unit/test_cluster_gcs_block.c b/src/test/cluster_unit/test_cluster_gcs_block.c index 3b17342fae..673b725333 100644 --- a/src/test/cluster_unit/test_cluster_gcs_block.c +++ b/src/test/cluster_unit/test_cluster_gcs_block.c @@ -2189,6 +2189,7 @@ UT_TEST(test_pcm_x_requester_fetch_revalidates_queue_and_reservation_before_inst const char *install_publish; const char *install_runtime_after; const char *recovering_retry; + const char *recovering_retry_counter; const char *reply_validation; char *reply_handler; @@ -2208,11 +2209,16 @@ UT_TEST(test_pcm_x_requester_fetch_revalidates_queue_and_reservation_before_inst UT_ASSERT_NOT_NULL(strstr(fetch, "gcs_block_pcm_x_install_reserved_image_exact(")); UT_ASSERT_NOT_NULL(strstr(fetch, "gcs_block_release_slot(slot)")); recovering_retry = strstr(fetch, "GCS_BLOCK_REPLY_DENIED_RESOURCE_RECOVERING"); + recovering_retry_counter = strstr(fetch, "pcm_x_image_fetch_recovering_retry_count"); reply_validation = strstr(fetch, "cluster_pcm_x_image_fetch_reply_exact("); UT_ASSERT_NOT_NULL(recovering_retry); + UT_ASSERT_NOT_NULL(recovering_retry_counter); UT_ASSERT(recovering_retry == NULL || recovering_retry < fetch_end); + UT_ASSERT(recovering_retry_counter == NULL || recovering_retry_counter < fetch_end); UT_ASSERT(recovering_retry == NULL || reply_validation == NULL || recovering_retry < reply_validation); + UT_ASSERT(recovering_retry_counter == NULL || reply_validation == NULL + || recovering_retry_counter < reply_validation); backoff_branch = strstr(fetch, "if (retry_attempt > 0)"); backoff_wait = backoff_branch != NULL ? strstr(backoff_branch, "(void)WaitLatch(MyLatch") : NULL;