Prevent race between snapshot-driven KV initialisation and early transactions during startup#8060
Prevent race between snapshot-driven KV initialisation and early transactions during startup#8060achamayou wants to merge 12 commits into
Conversation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Prevent KV-backed RPC dispatch until startup snapshot state, version, and history are coherent, while preserving explicitly transactionless node commands. Add focused frontend coverage and exercise pending-node, join, and recovery lifecycle behavior. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Refresh consensus after readiness publication and prevent queued join or endorsement paths from retrying over partially installed snapshot state. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR addresses the startup race described in #5141 by introducing a KV “readiness” gate so that frontends don’t construct KV transactions while the store/snapshot/history/consensus state is still being assembled (notably during snapshot installation). Until the store is coherent, only explicitly-marked transactionless command endpoints are dispatched; KV-backed endpoints return 503 FrontendNotOpen.
Changes:
- Add a shared atomic
StoreReadinessstate in the KV store and publish transitions from the node state machine (create/join/recovery/snapshot install). - Update RPC dispatch to check frontend openness + store readiness before constructing a transaction, and add a transaction-free command dispatch path.
- Update unit/e2e tests and architecture docs to reflect the new startup behavior (including retrying transient 503s during polling).
Custom instructions used:
.github/copilot-instructions.md.github/instructions/reviewing.instructions.md
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/infra/network.py | Treats transient non-200 responses as retryable when polling node state/network during startup. |
| tests/governance.py | Updates expectations for a join-time request during non-ready startup. |
| tests/e2e_common_endpoints.py | Adds an e2e assertion that /node/version remains available while a KV-backed endpoint is rejected. |
| src/node/rpc/test/frontend_test.cpp | Adds a unit test validating readiness gating behavior and command endpoint availability. |
| src/node/rpc/node_frontend.h | Converts readiness endpoints to command endpoints so they can run without a KV tx. |
| src/node/rpc/frontend.h | Implements the transaction-free command dispatch path and readiness checks before tx creation. |
| src/node/node_state.h | Publishes readiness transitions through create/join/recovery/snapshot install flows and tightens failure handling. |
| src/kv/store.h | Introduces StoreReadiness and atomic readiness accessors on the store. |
| src/endpoints/endpoint_registry.cpp | Adds “find endpoint without KV” and explicit command endpoint execution support. |
| include/ccf/endpoint.h | Adds endpoint execution mode + a dedicated command handler slot. |
| include/ccf/endpoint_registry.h | Exposes new endpoint lookup/dispatch helpers used by the frontend’s no-KV path. |
| doc/architecture/request_flow.rst | Updates request flow diagrams to reflect readiness check ordering. |
| doc/architecture/kv_readiness.rst | New architecture doc describing the readiness gate and lifecycle. |
| doc/architecture/index.rst | Adds the new KV readiness doc to the architecture index. |
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
DescriptionComparing this branch (#8060) against the trend of the last 30 Each chart plots every benchmark as an axis, with values normalized so 100 is the EWMA baseline of recent Axis labels show this branch's value and its difference from the main EWMA baseline, where 0% is on the baseline. They are coloured green where this branch improves on the baseline, red where it regresses, and grey where the difference is within one std dev of the baseline (within noise). Higher is better for throughput and rate, lower for latency and memory. Throughput (tx/s)---
config:
radar:
width: 620
height: 620
marginTop: 90
marginRight: 220
marginBottom: 60
marginLeft: 220
axisLabelFactor: 1.12
curveTension: 0.08
theme: base
themeCSS: |
.radarCurve-0{fill:color-mix(in srgb, #62B5E5 13%, var(--color-canvas-default,var(--bgColor-default,#fff)))!important;fill-opacity:1!important;stroke:none!important;stroke-width:0!important}
.radarCurve-1{fill:color-mix(in srgb, #62B5E5 40%, var(--color-canvas-default,var(--bgColor-default,#fff)))!important;fill-opacity:1!important;stroke:none!important;stroke-width:0!important}
.radarCurve-2{fill:color-mix(in srgb, #62B5E5 13%, var(--color-canvas-default,var(--bgColor-default,#fff)))!important;fill-opacity:1!important;stroke:none!important;stroke-width:0!important}
.radarCurve-3{fill:var(--color-canvas-default,var(--bgColor-default,#fff))!important;fill-opacity:1!important;stroke:none!important;stroke-width:0!important}
.radarCurve-4{stroke-width:2px!important}
.radarAxisLabel,.radarTitle{fill:var(--color-fg-default,var(--fgColor-default,#111827))!important;color:var(--color-fg-default,var(--fgColor-default,#111827))!important}
.radarAxisLabel:nth-of-type(1){fill:#808A94!important}
.radarAxisLabel:nth-of-type(2){fill:#2DA44E!important}
.radarAxisLabel:nth-of-type(3){fill:#808A94!important}
.radarAxisLabel:nth-of-type(4){fill:#808A94!important}
.radarAxisLabel:nth-of-type(5){fill:#808A94!important}
.radarAxisLabel:nth-of-type(6){fill:#2DA44E!important}
.radarAxisLabel:nth-of-type(7){fill:#808A94!important}
themeVariables:
cScale0: "#62B5E5"
cScale1: "#62B5E5"
cScale2: "#62B5E5"
cScale3: "#62B5E5"
cScale4: "#008FD3"
radar:
axisColor: "#9CA3AF"
graticuleColor: "#E5E7EB"
graticuleOpacity: 0
axisStrokeWidth: 1
curveOpacity: 0
---
radar-beta
axis b0["Basic: 67,887 tx/s ▬ +1%"]
axis b1["Basic Blocking: 1,046 tx/s ▲ 4%"]
axis b2["Basic JS: 4,837 tx/s ▬ +1%"]
axis b3["Basic Multi-Threaded: 88,015 tx/s ▬ +1%"]
axis b4["Historical Queries: 212,329 tx/s ▬ +2%"]
axis b5["Logging: 64,442 tx/s ▲ 5%"]
axis b6["Logging JWT: 10,390 tx/s ▬ +2%"]
curve stddev2_high["main EWMA + 2 std dev"]{103.39, 103.74, 103.34, 103.03, 110.14, 105.07, 105.50}
curve stddev1_high["main EWMA + 1 std dev"]{101.70, 101.87, 101.67, 101.51, 105.07, 102.54, 102.75}
curve stddev1_low["main EWMA - 1 std dev"]{98.30, 98.13, 98.33, 98.49, 94.93, 97.46, 97.25}
curve stddev2_low["main EWMA - 2 std dev"]{96.61, 96.26, 96.66, 96.97, 89.86, 94.93, 94.50}
curve branch["#8060"]{101.08, 103.50, 101.44, 101.13, 101.68, 104.73, 101.74}
graticule polygon
max 118
min 82
ticks 0
showLegend false
Latency (ms)---
config:
radar:
width: 620
height: 620
marginTop: 90
marginRight: 220
marginBottom: 60
marginLeft: 220
axisLabelFactor: 1.12
curveTension: 0.08
theme: base
themeCSS: |
.radarCurve-0{fill:color-mix(in srgb, #62B5E5 13%, var(--color-canvas-default,var(--bgColor-default,#fff)))!important;fill-opacity:1!important;stroke:none!important;stroke-width:0!important}
.radarCurve-1{fill:color-mix(in srgb, #62B5E5 40%, var(--color-canvas-default,var(--bgColor-default,#fff)))!important;fill-opacity:1!important;stroke:none!important;stroke-width:0!important}
.radarCurve-2{fill:color-mix(in srgb, #62B5E5 13%, var(--color-canvas-default,var(--bgColor-default,#fff)))!important;fill-opacity:1!important;stroke:none!important;stroke-width:0!important}
.radarCurve-3{fill:var(--color-canvas-default,var(--bgColor-default,#fff))!important;fill-opacity:1!important;stroke:none!important;stroke-width:0!important}
.radarCurve-4{stroke-width:2px!important}
.radarAxisLabel,.radarTitle{fill:var(--color-fg-default,var(--fgColor-default,#111827))!important;color:var(--color-fg-default,var(--fgColor-default,#111827))!important}
.radarAxisLabel:nth-of-type(1){fill:#808A94!important}
.radarAxisLabel:nth-of-type(2){fill:#808A94!important}
.radarAxisLabel:nth-of-type(3){fill:#808A94!important}
themeVariables:
cScale0: "#62B5E5"
cScale1: "#62B5E5"
cScale2: "#62B5E5"
cScale3: "#62B5E5"
cScale4: "#008FD3"
radar:
axisColor: "#9CA3AF"
graticuleColor: "#E5E7EB"
graticuleOpacity: 0
axisStrokeWidth: 1
curveOpacity: 0
---
radar-beta
axis b0["Commit Latency 16ms: 4.45 ms ▬ -16%"]
axis b1["Commit Latency 1ms: 1.81 ms ▬ +2%"]
axis b2["Commit Latency 256ms: 208 ms ▬ 0%"]
curve stddev2_high["main EWMA + 2 std dev"]{149.93, 107.17, 100.95}
curve stddev1_high["main EWMA + 1 std dev"]{124.97, 103.58, 100.48}
curve stddev1_low["main EWMA - 1 std dev"]{75.03, 96.42, 99.52}
curve stddev2_low["main EWMA - 2 std dev"]{50.07, 92.83, 99.05}
curve branch["#8060"]{84.11, 101.93, 100.21}
graticule polygon
max 185
min 15
ticks 0
showLegend false
Memory (bytes)---
config:
radar:
width: 620
height: 620
marginTop: 90
marginRight: 220
marginBottom: 60
marginLeft: 220
axisLabelFactor: 1.12
curveTension: 0.08
theme: base
themeCSS: |
.radarCurve-0{fill:color-mix(in srgb, #62B5E5 13%, var(--color-canvas-default,var(--bgColor-default,#fff)))!important;fill-opacity:1!important;stroke:none!important;stroke-width:0!important}
.radarCurve-1{fill:color-mix(in srgb, #62B5E5 40%, var(--color-canvas-default,var(--bgColor-default,#fff)))!important;fill-opacity:1!important;stroke:none!important;stroke-width:0!important}
.radarCurve-2{fill:color-mix(in srgb, #62B5E5 13%, var(--color-canvas-default,var(--bgColor-default,#fff)))!important;fill-opacity:1!important;stroke:none!important;stroke-width:0!important}
.radarCurve-3{fill:var(--color-canvas-default,var(--bgColor-default,#fff))!important;fill-opacity:1!important;stroke:none!important;stroke-width:0!important}
.radarCurve-4{stroke-width:2px!important}
.radarAxisLabel,.radarTitle{fill:var(--color-fg-default,var(--fgColor-default,#111827))!important;color:var(--color-fg-default,var(--fgColor-default,#111827))!important}
.radarAxisLabel:nth-of-type(1){fill:#808A94!important}
.radarAxisLabel:nth-of-type(2){fill:#808A94!important}
.radarAxisLabel:nth-of-type(3){fill:#808A94!important}
.radarAxisLabel:nth-of-type(4){fill:#808A94!important}
.radarAxisLabel:nth-of-type(5){fill:#E5484D!important}
.radarAxisLabel:nth-of-type(6){fill:#E5484D!important}
themeVariables:
cScale0: "#62B5E5"
cScale1: "#62B5E5"
cScale2: "#62B5E5"
cScale3: "#62B5E5"
cScale4: "#008FD3"
radar:
axisColor: "#9CA3AF"
graticuleColor: "#E5E7EB"
graticuleOpacity: 0
axisStrokeWidth: 1
curveOpacity: 0
---
radar-beta
axis b0["Basic: 84.9 MiB ▬ -3%"]
axis b1["Basic Blocking: 71.2 MiB ▬ 0%"]
axis b2["Basic JS: 70.4 MiB ▬ 0%"]
axis b3["Basic Multi-Threaded: 89.8 MiB ▬ 0%"]
axis b4["Logging: 77 MiB ▲ 2%"]
axis b5["Logging JWT: 68.8 MiB ▲ 1%"]
curve stddev2_high["main EWMA + 2 std dev"]{107.05, 100.70, 103.66, 102.12, 102.16, 101.19}
curve stddev1_high["main EWMA + 1 std dev"]{103.53, 100.35, 101.83, 101.06, 101.08, 100.59}
curve stddev1_low["main EWMA - 1 std dev"]{96.47, 99.65, 98.17, 98.94, 98.92, 99.41}
curve stddev2_low["main EWMA - 2 std dev"]{92.95, 99.30, 96.34, 97.88, 97.84, 98.81}
curve branch["#8060"]{96.64, 100.38, 99.85, 100.33, 101.82, 101.02}
graticule polygon
max 112
min 88
ticks 0
showLegend false
Rate (ops/s)---
config:
radar:
width: 620
height: 620
marginTop: 90
marginRight: 220
marginBottom: 60
marginLeft: 220
axisLabelFactor: 1.12
curveTension: 0.08
theme: base
themeCSS: |
.radarCurve-0{fill:color-mix(in srgb, #62B5E5 13%, var(--color-canvas-default,var(--bgColor-default,#fff)))!important;fill-opacity:1!important;stroke:none!important;stroke-width:0!important}
.radarCurve-1{fill:color-mix(in srgb, #62B5E5 40%, var(--color-canvas-default,var(--bgColor-default,#fff)))!important;fill-opacity:1!important;stroke:none!important;stroke-width:0!important}
.radarCurve-2{fill:color-mix(in srgb, #62B5E5 13%, var(--color-canvas-default,var(--bgColor-default,#fff)))!important;fill-opacity:1!important;stroke:none!important;stroke-width:0!important}
.radarCurve-3{fill:var(--color-canvas-default,var(--bgColor-default,#fff))!important;fill-opacity:1!important;stroke:none!important;stroke-width:0!important}
.radarCurve-4{stroke-width:2px!important}
.radarAxisLabel,.radarTitle{fill:var(--color-fg-default,var(--fgColor-default,#111827))!important;color:var(--color-fg-default,var(--fgColor-default,#111827))!important}
.radarAxisLabel:nth-of-type(1){fill:#808A94!important}
.radarAxisLabel:nth-of-type(2){fill:#808A94!important}
.radarAxisLabel:nth-of-type(3){fill:#2DA44E!important}
.radarAxisLabel:nth-of-type(4){fill:#2DA44E!important}
.radarAxisLabel:nth-of-type(5){fill:#808A94!important}
.radarAxisLabel:nth-of-type(6){fill:#808A94!important}
themeVariables:
cScale0: "#62B5E5"
cScale1: "#62B5E5"
cScale2: "#62B5E5"
cScale3: "#62B5E5"
cScale4: "#008FD3"
radar:
axisColor: "#9CA3AF"
graticuleColor: "#E5E7EB"
graticuleOpacity: 0
axisStrokeWidth: 1
curveOpacity: 0
---
radar-beta
axis b0["CHAMP get: 38,009,688 ops/s ▬ +4%"]
axis b1["CHAMP put: 5,444,289 ops/s ▬ +2%"]
axis b2["KV deserialisation: 1,628,930 ops/s ▲ 2%"]
axis b3["KV serialisation: 1,492,537 ops/s ▲ 3%"]
axis b4["KV snapshot deserialis...: 4,216 ops/s ▬ +1%"]
axis b5["KV snapshot serialisation: 4,347 ops/s ▬ -5%"]
curve stddev2_high["main EWMA + 2 std dev"]{119.46, 115.84, 102.16, 102.40, 101.87, 112.25}
curve stddev1_high["main EWMA + 1 std dev"]{109.73, 107.92, 101.08, 101.20, 100.94, 106.12}
curve stddev1_low["main EWMA - 1 std dev"]{90.27, 92.08, 98.92, 98.80, 99.06, 93.88}
curve stddev2_low["main EWMA - 2 std dev"]{80.54, 84.16, 97.84, 97.60, 98.13, 87.75}
curve branch["#8060"]{103.65, 102.10, 102.38, 103.42, 100.85, 95.18}
graticule polygon
max 134
min 66
ticks 0
showLegend false
|
CCF opens RPC frontends while create, join, and recovery startup may still be assembling the KV Store. During snapshot installation, maps, the Store version, history, and consensus can become visible at different moments, so a concurrent KV-backed request may create a transaction against partially initialised state. This is the race tracked in #5141.
This change adds a shared atomic Store readiness state and checks it before constructing a transaction. Until startup state is coherent, frontends dispatch only explicitly transactionless, unauthenticated, non-forwarded command endpoints and return
503 FrontendNotOpenfor KV-backed requests. Create, join, and recovery paths publishReadyonly after consensus, history, and snapshot setup completes; snapshot installation failures are terminal rather than retried over mutated state.Coverage includes frontend unit tests for every non-ready state and readiness recovery, plus an end-to-end pending-node check proving that transactionless
/node/versionremains available while a KV-backed endpoint is rejected. The existing logging, recovery, and reconfiguration suites exercise normal joins, snapshot joins, and recovery paths.Follow-up: frontend dependency caches
Review also identified a pre-existing race in the lazy raw-pointer caches for history and consensus in
RpcFrontendandEndpointRegistry: concurrent workers may attempt to populate the same cache. The history portion is already tracked by #6636. The readiness gate prevents requests from observing a partially initialised Store, but it deliberately does not redesign ownership or publication of these frontend caches, so that work is out of scope for this PR.We evaluated fetching owning references on demand, publishing the dependencies during startup, and using atomic caches. A follow-up PR will remove the unused registry history cache and fetch owning history references on demand; consensus cache removal will then use Store-backed, request-local ownership rather than adding further duplicated cache state. This keeps the startup fix focused while addressing the independent cache race with an ownership-safe design.
Closes #5141