Skip to content

fix(wire): open [MIN,WIRE] window — rolling upgrades possible - #244

Closed
EnRaiha wants to merge 1 commit into
NodeDB-Lab:mainfrom
EnRaiha:fix/wire-version-window
Closed

fix(wire): open [MIN,WIRE] window — rolling upgrades possible#244
EnRaiha wants to merge 1 commit into
NodeDB-Lab:mainfrom
EnRaiha:fix/wire-version-window

Conversation

@EnRaiha

@EnRaiha EnRaiha commented Aug 23, 2026

Copy link
Copy Markdown
Contributor

Fixes epic #165 High #5 (wire version hard-partition mid-upgrade). Replaces the exact-equality join gate with a range window [min_wire_version, CLUSTER_WIRE_FORMAT_VERSION] so mixed-version clusters can form (N-1 rolling upgrade).

  • MIN_WIRE_FORMAT_VERSION=1, WIRE_FORMAT_VERSION=2; doc rewritten for window semantics (schema vs transport explicitly separate).
  • wire_version_in_window() pure gate + effective floor (max(compile-time MIN, persisted ClusterSettings.min_wire_version)) — operator knob now enforced.
  • ClusterVersionView moved from nodedb (inverted dep) to nodedb-cluster/src/topology/version_view.rs; re-export shim; dangling rustdoc links fixed.
  • restart re-stamps self NodeInfo.wire_version after upgrade (raise-only, idempotent; pure helper, unit-tested without a live transport).
  • window_from_join_response() logs observed cluster version window.

Verification: nodedb-types+nodedb 6246 pass, cluster lib 1023/1023, clippy 0, zero exact-equality gates left, maya-gate clean 12/12. Note: 32 rustdoc broken-intra-doc-link errors are PRE-EXISTING in untouched files (tracked separately).

Part of #165.

…odeDB-Lab#165)

Replace the exact-equality join gate (handle_join.rs) with a range
window [min_wire_version, CLUSTER_WIRE_FORMAT_VERSION] so mixed-version
clusters can form (N-1 rolling upgrade):

- nodedb-types: MIN_WIRE_FORMAT_VERSION=1, WIRE_FORMAT_VERSION=2;
  doc essay rewritten for window semantics (schema vs transport versions
  explicitly separated; bump WIRE only with a wire-shape change).
- handle_join: wire_version_in_window() pure gate + effective floor
  (max(compile-time MIN, persisted ClusterSettings.min_wire_version))
  threaded through JoinFlow call sites; operator knob now enforced.
- ClusterVersionView moved from nodedb (inverted dep) to
  nodedb-cluster/src/topology/version_view.rs; nodedb re-exports a shim;
  dangling rustdoc links in topology.rs fixed.
- restart: self NodeInfo.wire_version re-stamped to current build on
  restart after upgrade (in-memory + persisted, raise-only, idempotent),
  extracted as pure restamp_self_wire_version() so the upgrade path is
  testable without a live QUIC transport.
- join: window_from_join_response() helper logs the observed cluster
  version window; rejection message now carries the accepted window.
- cluster_settings doc: knob enforcement documented at the gate.
- Tests: wire_version_in_window boundaries, mixed-window accept/reject,
  zero-version reject, restart re-stamp (stale→current, idempotent),
  view tests now LIVE (previously skipped when WIRE < 2), versions.rs
  reject_older flipped to older_in_window_accepted, handshake suite
  extended with N-1 accept + future reject.

Verification: nodedb-types+nodedb 6246 pass, nodedb-cluster lib
1023/1023, clippy -D warnings 0, zero exact-equality gates left,
maya-gate L1 clean 12/12. Rustdoc -D broken_intra_doc_links: 32
PRE-EXISTING errors in untouched files (applied_watcher, auth/bundle,
calvin/sequencer, forward, mirror, raft_loop builder/hooks) — not a
regression; fix tracked separately.

Refactor note (deferred, P2-GLM53-REVIEW-RESOLUTION Improvement 2):
VersionWindow newtype in nodedb-types + fold RPC_FRAME_VERSION into the
transport envelope; the 3 version systems stay separate but documented.
JoinResponse rkyv constraint keeps phase-1 wire-change-free (deferred
window echo in envelope-versioned structs post-1.0).
Copilot AI lite review requested due to automatic review settings August 23, 2026 22:23

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

EnRaiha added a commit to EnRaiha/nodedb that referenced this pull request Aug 24, 2026
Drill/fix-plan/GLM-resolution docs (commit dd0c316) written pre-fix are
now stale. Add resolution banners pointing to the implemented fixes:
SWIM 8886846 (PR NodeDB-Lab#243), wire window e60a853 (PR NodeDB-Lab#244), epoch fence
4f92959 (PR NodeDB-Lab#245), lease GC 16d9916 (PR NodeDB-Lab#246). Full verification +
refactor code in P2-REPORT.md.
@farhan-syah

Copy link
Copy Markdown
Member

Closing this one without taking the change, and the reason is different from the other four — the premise rather than the implementation.

nodedb-types/src/wire_version.rs carries a "DO NOT BUMP THIS BEFORE 1.0" section that sets out why the floor and ceiling are deliberately equal, and it anticipates this change specifically: "After 1.0, when real deployments exist and a genuine compatibility window is introduced, this becomes meaningful — bump it then, deliberately, and only alongside an actual MIN_WIRE_FORMAT_VERSION < WIRE_FORMAT_VERSION support window." That is precisely this PR, at precisely the point the doc says to hold off.

The governing fact is that there are no deployed clusters before 1.0, so there is no older peer a new build has to talk to. A hard partition mid-upgrade cannot happen when there is nothing to upgrade from. The doc also notes the consequence: while floor equals ceiling, every wire_version >= V feature gate is unreachable, because inside a cluster that exists all nodes are provably on the same version.

There is also a sequencing problem independent of the pre-1.0 policy. The change bumps WIRE_FORMAT_VERSION to 2 while opening the window to [1, 2], but nodes already running the current code are at 1 and still gate on exact equality. So a v2 node joining an existing v1 cluster is rejected by the v1 side, while a v1 node joining a v2 cluster is accepted. A rolling upgrade means upgrading nodes one at a time inside an existing cluster, which is the rejected direction. The window can only take effect once every node already runs window-capable code, so the order has to be: ship the window logic at 1/1 with no behavioural change, let it reach every node, then bump. Doing both together is the one ordering that cannot deliver the feature.

On the ClusterVersionView move: there is no inverted dependency to fix. The type lives in nodedb and is used only within nodedb; nodedb-cluster does not depend on nodedb. Moving a host-layer concept down into the cluster crate would make the layering worse rather than better.

One observation from the PR was right and has been fixed separately: ClusterSettings.min_wire_version is documented as a minimum peers must speak, and nothing enforces it — it appears only in its own definition and tests. Rather than wire it up, its doc now states plainly that it is reserved and inert while floor equals ceiling, and what it will mean once a window opens. An operator knob that silently does nothing is worth correcting on its own.

When the window is genuinely wanted after 1.0, the wire_version_in_window gate and the effective-floor rule from this branch are a reasonable starting point — the objection is to the timing and the simultaneous bump, not to that logic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants