H wiring: block_recovery.go now uses runtime.PendingCoordinator
- Removed local pendingRecoveryExecution type + store/take/peek/has/cancel
- ExecutePendingCatchUp/Rebuild delegate to coord.TakeCatchUp/TakeRebuild
- Shutdown uses coord.CancelAll
- Added CancelAll to PendingCoordinator
I wiring: executeCatchUpPlan/executeRebuildPlan replaced
- ExecutePendingCatchUp now calls rt.ExecuteCatchUpPlan with RecoveryManager
as RecoveryCallbacks (OnCatchUpCompleted/OnRebuildCompleted)
- ExecutePendingRebuild follows same pattern
- Local executeCatchUpPlan/executeRebuildPlan methods removed
J structural: legacy no-core branches extracted
- executeLegacyCatchUp: wraps rt.ExecuteCatchUpPlan for v2Core==nil path
- executeLegacyRebuild: wraps rt.ExecuteRebuildPlan for v2Core==nil path
- Clear "LEGACY NO-CORE COMPATIBILITY" section with structural separation
- runCatchUp/runRebuild now branch cleanly: legacy helper vs core coordinator
Test updates: pendingRecoveryExecution → rt.PendingExecution, field casing,
Plan type assertions.
Validation: all P4, P16B, and ApplyAssignments tests pass.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add explicit "LEGACY NO-CORE COMPATIBILITY" section header in
block_recovery.go marking HandleAssignmentResult and
HandleRemovedAssignments as compatibility-only entry points.
The comment block explicitly states:
- These are for pre-Phase-16 no-core paths and older tests
- Core-present paths use StartRecoveryTask + ExecutePending*
- These should NOT be strengthened into semantic-authority proofs
No behavioral change — structural labeling only. All validation passes.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Task F (Pinner):
- block_recovery.go: removed pinnerShimForRecovery (11 lines of pure
pass-through). v2bridge.Pinner structurally satisfies bridge.BlockVolPinner
(same method signatures), so it's passed directly.
Task G (Executor):
- Already clean. v2bridge.Executor is used directly without any shim —
structurally satisfies engine.CatchUpIO and engine.RebuildIO.
No code changes needed.
After Task E+F+G: zero shim types remain in block_recovery.go.
v2bridge Reader/Pinner/Executor all satisfy sw-block contracts directly.
Validation:
- go test ./weed/storage/blockvol/v2bridge/ -run "TestPinner_|TestExecutor_|TestBridge_" → PASS
- go test ./weed/server/ -run "TestP4_|TestP16B_" → PASS (8 tests)
- go test ./sw-block/bridge/blockvol/... → PASS
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Reader backend-binding extraction:
- v2bridge/reader.go: Reader.ReadState() now returns bridge.BlockVolState
directly instead of a local v2bridge.BlockVolState mirror type.
Removed the local BlockVolState type entirely.
- block_recovery.go: removed readerShimForRecovery (12 lines of 1:1
field copying). Reader is now passed directly as bridge.BlockVolReader.
Before: v2bridge.Reader → v2bridge.BlockVolState → readerShim → bridge.BlockVolState
After: v2bridge.Reader → bridge.BlockVolState (direct)
v2bridge now imports sw-block/bridge/blockvol for the contract type
(control.go already did this, reader.go now follows the same pattern).
Validation:
- go test ./sw-block/bridge/blockvol/... → PASS
- go test ./weed/storage/blockvol/v2bridge/ -run "TestReader_" → PASS
- go test ./weed/server/ -run "TestP4_|TestP16B_" → PASS (8 tests)
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
16B widened from catch-up-only to catch-up + rebuild:
- StartRebuildCommand: core emits rebuild command, adapter executes
- Fail-closed: pending rebuild does not run without fresh command
- Recovery observations close back into core projection
New proofs:
- StartRebuildCommand_ConsumesPendingPlanAndUpdatesProjection
- RunRebuild_FailClosedWithoutFreshStartRebuildCommand
Review docs:
- phase-16-rev3-review.md: widened 16B review object
- phase-16-rev3-manager-rereview.md: manager challenge response
- phase-16-checkpoint-review.md: updated
Non-claims: not full recovery-loop closure, not end-to-end
failover/publication, not launch readiness.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add computed VolumeMode to BlockVolumeEntry with 5 normalized modes:
- allocated_only: RF=1, no replicas (standalone)
- bootstrap_pending: RF>1 but replicas not yet ready (first-write pending)
- publish_healthy: all replicas ready, no transport degradation
- degraded: replication impaired but recoverable
- needs_rebuild: unrecoverable gap, rebuild required
Code changes:
- master_block_registry.go: computeVolumeMode() called from
recomputeReplicaState(), VolumeMode field on BlockVolumeEntry
- master_server_handlers_block.go: VolumeMode exposed in REST API
- blockapi/types.go: VolumeMode field in VolumeInfo
- testrunner types: VolumeMode for scenario assertions
7 tests prove mode normalization:
- AllocatedOnly, BootstrapPending (2 cases), PublishHealthy,
Degraded, NeedsRebuild, SurfaceConsistency (transition proof)
Interpretation rule: current integrated tests validate V1 runtime
under V2 constraints, not a completed V2 runtime (Phase 14 scope).
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Bug: After failover promotes a replica to primary, the old primary
re-registers via heartbeat as a replica (lower epoch). But the master
never sent an updated Primary assignment to the new primary with the
re-registered replica's addresses. The new primary had 0 shippers →
replication dead. sync_all barrier passed vacuously.
Root cause: upsertServerAsReplica (heartbeat reconciliation) added the
re-registered server to Replicas[] but didn't (a) populate DataAddr/
CtrlAddr from heartbeat info, or (b) trigger a primary assignment
refresh.
Fix:
- master_block_registry.go: upsertServerAsReplica now copies DataAddr/
CtrlAddr from heartbeat info and sets NeedsPrimaryRefresh flag.
UpdateFullHeartbeat returns HeartbeatResult with PrimaryRefreshNeeded
entries. DrainPrimaryRefreshNeeded collects and clears the flag.
- master_block_failover.go: add enqueuePrimaryRefresh — builds a
Primary assignment with all current replica addresses and enqueues it.
- master_grpc_server.go: heartbeat handler processes PrimaryRefreshNeeded
entries after UpdateFullHeartbeat.
Gate test: TestPromote_AssignmentHasReplicaAddrs now PASSES —
after promote + re-register, the new primary gets an assignment with
replicaDataAddr=vs1:14260 and replicaAddrs=1.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Investigation result:
- Dual-BlockVol hypothesis: DISPROVEN (one instance per path, correct wiring)
- Root cause: adapter wiring bug in test allocator
soak_test.go blockVSAllocate returned ReplicaDataAddr = "vs2:9333:14260"
(server + ":port" where server already has a port → three colons, invalid)
This caused setupReplicaReceiver to fail silently → no data replicated
Root cause classification: adapter/test-harness bug
- NOT a backend data visibility bug
- NOT a core-rule gap
- The engine read path works correctly (TestSyncAll_FullRoundTrip passes)
Code changes:
- qa_block_soak_test.go: fix allocator to use host:port (not server:port),
use deterministic FNV-hashed ports matching production ReplicationPorts
- qa_block_cp13_8a_test.go: 2 new integration tests proving replica reads
work through both ReadLBA and adapter.ReadAt, before and after promotion
Remaining contradiction for CP13-8 scenario on real hardware:
- The production weed cluster uses ReplicationPorts (deterministic) which
should not have this bug. If CP13-8 still fails on m01/M02, the cause
is different from this test-harness issue and needs a separate investigation.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- master_block_registry.go: minor role-handling fixes
- qa_failover_role_test.go: new failover role test
- testrunner/actions/devops.go: new devops action helpers
- recovery-baseline-failover.yaml: scenario alignment
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Two fixes:
1. Rename advertisedIP → advertisedHost throughout, relax contract from
"always a real IP" to "routable host from -ip flag (IP or resolvable
hostname)". This matches the actual -ip flag semantics which accepts
both IP addresses and server names.
2. Add TestCP13_2_BlockService_AdvertisedHost_NotOpaqueID that hits the
actual production wiring: BlockService with opaque localServerID +
routable advertisedHost → setupReplicaReceiver → verify exported
addresses use the routable host, not the opaque ID.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Bug: setupReplicaReceiver derived the advertised host from localServerID,
which can be an opaque string (from -id flag, e.g., "my-custom-server-id").
This would publish unusable endpoints like "my-custom-server-id:14260".
Fix:
- volume_server_block.go: add advertisedIP field (always a real IP from
-ip flag), use it instead of localServerID for replica canonicalization
- volume.go: wire *v.ip → blockService.SetAdvertisedIP() at startup
- blockvol.go: StartReplicaReceiver variadic advertisedHost unchanged
Proof (sync_all_bug_test.go TestBug3, 4 sub-cases):
- fallback: wildcard bind without advertisedHost → outbound-IP
- advertisedHost: explicit IP appears in exported addresses
- StartReplicaReceiver_API: public API forwards host correctly
- opaque_identity_not_routable: proves opaque string produces
non-routable address, confirming production must use advertisedIP
Identity vs transport separation preserved:
- localServerID: stable identity for V2 control (may be opaque)
- advertisedIP: routable IP for transport endpoints (always real IP)
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Problem: StartReplicaReceiver didn't forward advertisedHost to
NewReplicaReceiver, so wildcard-bind listeners relied on outbound-IP
fallback for canonicalization. On multi-NIC hosts this could select
the wrong interface, leaking non-routable addresses into replication
truth.
Fix:
- blockvol.go: StartReplicaReceiver now accepts optional advertisedHost
variadic param and forwards it to NewReplicaReceiver
- volume_server_block.go: setupReplicaReceiver extracts host from
localServerID (the canonical VS identity) and passes it as
advertisedHost — wildcard-bind addresses now resolve to the
authoritative server IP, not outbound-IP fallback
Proof (sync_all_bug_test.go TestBug3, upgraded from PASS* to PASS):
- fallback: wildcard bind without advertisedHost still produces ip:port
- advertisedHost: explicit host appears in exported DataAddr/CtrlAddr
- StartReplicaReceiver_API: public API forwards advertisedHost correctly
What CP13-2 does NOT change:
- No reconnect handshake changes (CP13-5)
- No retention policy changes (CP13-6)
- No rebuild behavior changes (CP13-7)
- No barrier protocol changes (CP13-3)
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Finding 1: ProcessAssignments now calls v2Orchestrator.ProcessAssignment
- BlockService.v2Orchestrator field (RecoveryOrchestrator)
- ProcessAssignment result logged at glog V(1)
- No more `_ = intent` — engine state actually changes
Finding 2: localServerID documented as interim
- BlockService.localServerID = listenAddr (transport-shaped)
- Field doc explicitly states: INTERIM, should be registry-assigned
- Used only for replica/rebuild local identity
3 integration tests (qa_block_v2bridge_test.go):
- CreatesEngineSender: ProcessAssignment → engine has sender + session
- EpochBump: epoch 1 → invalidate → epoch 2 → new session
- AddressChange: same ServerID, different IP → sender preserved,
endpoint updated
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Finding 1: Identity no longer address-derived
- ReplicaAddr.ServerID field added (stable server identity from registry)
- BlockVolumeAssignment.ReplicaServerID field added (scalar RF=2 path)
- ControlBridge uses ServerID, NOT address, for ReplicaID
- Missing ServerID → replica skipped (fail closed), logged
Finding 2: Wired into real ProcessAssignments
- BlockService.v2Bridge field initialized in StartBlockService
- ProcessAssignments converts each assignment via v2Bridge.ConvertAssignment
BEFORE existing V1 processing (parallel, not replacing yet)
- Logged at glog V(1)
Finding 3: Fail-closed on missing identity
- Empty ServerID in ReplicaAddrs → replica skipped with log
- Empty ReplicaServerID in scalar path → no replica created
- Test: MissingServerID_FailsClosed verifies both paths
7 tests: StableServerID, AddressChange_IdentityPreserved,
MultiReplica_StableServerIDs, MissingServerID_FailsClosed,
EpochFencing_IntegratedPath, RebuildAssignment, ReplicaAssignment
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
AllocateBlockVolumeResponse used bs.ListenAddr() to derive replica
addresses. When the VS binds to ":port" (no explicit IP), host
resolved to empty string, producing ":dataPort" as the replica
address. This ":port" propagated through master assignments to both
primary and replica sides.
Now canonicalizes empty/wildcard host using PreferredOutboundIP()
before constructing replication addresses. Also exported
PreferredOutboundIP for use by the server package.
This is the source fix — all downstream paths (heartbeat, API
response, assignment) inherit the canonical address.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
setupReplicaReceiver now reads back canonical addresses from
the ReplicaReceiver (which applies CP13-2 canonicalization)
instead of storing raw assignment addresses in replStates.
This fixes the API-level leak where replica_data_addr showed
":port" instead of "ip:port" in /block/volumes responses,
even though the engine-level CP13-2 fix was working.
New BlockVol.ReplicaReceiverAddr() returns canonical addresses
from the running receiver. Falls back to assignment addresses
if receiver didn't report.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Same-epoch reconciliation now trusts reported roles first:
- one claims primary, other replica → trust roles
- both claim primary → WALHeadLSN heuristic tiebreak
- both claim replica → keep existing, log ambiguity
Replaced addServerAsReplica with upsertServerAsReplica: checks
for existing replica entry by server name before appending.
Prevents duplicate ReplicaInfo rows during restart/replay windows.
2 new tests: role-trusted same-epoch, duplicate replica prevention.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
When a second server reports the same volume during master restart,
UpdateFullHeartbeat now uses epoch-based tie-breaking instead of
first-heartbeat-wins:
1. Higher epoch wins as primary — old entry demoted to replica
2. Same epoch — higher WALHeadLSN wins (heuristic, warning logged)
3. Lower epoch — added as replica
Applied in both code paths: the auto-register branch (no entry
exists yet for this name) and the unlinked-server branch (entry
exists but this server is not in it).
This is a deterministic reconstruction improvement, not ground
truth. The long-term fix is persisting authoritative volume state.
5 new tests covering all reconciliation scenarios.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Lookup() and ListAll() now return value copies (not pointers to
internal registry state). Callers can no longer mutate registry
entries without holding a lock.
Added clone() on BlockVolumeEntry with deep-copied Replicas slice.
Added UpdateEntry(name, func(*BlockVolumeEntry)) for locked mutation.
ListByServer() also returns copies.
Migrated 1 production mutation (ReplicaPlacement + Preset in create
handler) and ~20 test mutations to use UpdateEntry.
5 new copy-correctness tests: Lookup returns copy, Replicas slice
isolated, ListAll returns copies, UpdateEntry mutates, UpdateEntry
not-found error.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
New POST /block/volume/plan endpoint returns full placement preview:
resolved policy, ordered candidate list, selected primary/replicas,
and per-server rejection reasons with stable string constants.
Core design: evaluateBlockPlacement() is a pure function with no
registry/topology dependency. gatherPlacementCandidates() is the
single topology bridge point. Plan and create share the same planner —
parity contract is same ordered candidate list for same cluster state.
Create path refactored: uses evaluateBlockPlacement() instead of
PickServer(), iterates all candidates (no 3-retry cap), recomputes
replica order after primary fallback. rf_not_satisfiable severity
is durability-mode-aware (warning for best_effort, error for strict).
15 unit tests + 20 QA adversarial tests.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Preset system: ResolvePolicy resolves named presets (database, general,
throughput) with per-field overrides into concrete volume parameters.
Create path now uses resolved policy instead of ad-hoc validation.
New /block/volume/resolve diagnostic endpoint for dry-run resolution.
Review fix 1 (MED): HasNVMeCapableServer now derives NVMe capability
from server-level heartbeat attribute (block_nvme_addr proto field)
instead of scanning volume entries. Fixes false "no NVMe" warning on
fresh clusters with NVMe-capable servers but no volumes yet.
Review fix 2 (LOW): /block/volume/resolve no longer proxied to leader —
read-only diagnostic endpoint can be served by any master.
Engine fix: ReadLBA retry loop closes stale dirty-map race when WAL
entry is recycled between lookup and read.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Six-task checkpoint hardening the promotion and failover paths:
T1: 4-gate candidate evaluation (heartbeat freshness, WAL lag, role,
server liveness) with structured rejection reasons.
T2: Orphaned-primary re-evaluation on replica reconnect (B-06/B-08).
T3: Deferred timer safety — epoch validation prevents stale timers
from firing on recreated/changed volumes (B-07).
T4: Rebuild addr cleanup on promotion (B-11), NVMe publication
refresh on heartbeat, and preflight endpoint wiring.
T5: Manual promote API — POST /block/volume/{name}/promote with
force flag, target server selection, and structured rejection
response. Shared applyPromotionLocked/finalizePromotion helpers
eliminate duplication between auto and manual paths.
T6: Read-only preflight endpoint (GET /block/volume/{name}/preflight)
and blockapi client wrappers (Preflight, Promote).
BUG-T5-1: PromotionsTotal counter moved to finalizePromotion (shared
by both auto and manual paths) to prevent metrics divergence.
24 files changed, ~6500 lines added. 42 new QA adversarial tests.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
B-09: ExpandBlockVolume re-reads the registry entry after acquiring
the expand inflight lock. Previously it used the entry from the
initial Lookup, which could be stale if failover changed VolumeServer
or Replicas between Lookup and PREPARE.
B-10: UpdateFullHeartbeat stale-cleanup now skips entries with
ExpandInProgress=true. Previously a primary VS restart during
coordinated expand would delete the entry (path not in heartbeat),
orphaning the volume and stranding the expand coordinator.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Two-phase prepare/commit/cancel protocol ensures all replicas expand
atomically. Standalone volumes use direct-commit (unchanged behavior).
Engine: PrepareExpand/CommitExpand/CancelExpand with on-disk
PreparedSize+ExpandEpoch in superblock, crash recovery clears stale
prepare state on open, v.mu serializes concurrent expand operations.
Proto: 3 new RPCs (PrepareExpand/CommitExpand/CancelExpandBlockVolume).
Coordinator: expandClean flag pattern — ReleaseExpandInflight only on
clean success or full cancel. Partial replica commit failure calls
MarkExpandFailed (keeps ExpandInProgress=true, suppresses heartbeat
size updates). ClearExpandFailed for manual reconciliation.
Registry: AcquireExpandInflight records PendingExpandSize+ExpandEpoch.
ExpandFailed state blocks new expands until cleared.
Tests: 15 engine + 4 VS + 10 coordinator + heartbeat suppression
regression + updated QA CP82/durability tests with prepare/commit mocks.
Also includes CP11A-1 remaining: QA storage profile tests, QA
io_backend config tests, testrunner perf-baseline scenarios and
coordinated-expand actions.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
ReplicaInfo now carries NvmeAddr/NQN. Fields are populated during
replica allocation (tryCreateOneReplica), updated from replica
heartbeats, and copied in PromoteBestReplica. This ensures master
lookup returns correct NVMe endpoints immediately after failover,
without waiting for the first post-promotion heartbeat.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add nvme_addr and nqn fields to proto messages (AllocateBlockVolume,
CreateBlockVolume, LookupBlockVolume, BlockVolumeInfoMessage), wire
through volume server → master registry → CSI driver. Volume servers
report NVMe address in heartbeats when NVMe target is running. CSI
MasterVolumeClient now populates NvmeAddr/NQN from master responses,
enabling NVMe/TCP via the master-backend path.
Proto files regenerated with protoc 29.5.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add ProcessBlockVolumeAssignments to BlockVolumeStore and wire
AssignmentSource/AssignmentCallback into the heartbeat collector's
Run() loop. Assignments are fetched and applied each tick after
status collection.
Bug fixes:
- BUG-CP4B3-1: TOCTOU between GetBlockVolume and HandleAssignment.
Added withVolume() helper that holds RLock across lookup+operation,
preventing RemoveBlockVolume from closing the volume mid-assignment.
- BUG-CP4B3-2: Data race on callback fields read by Run() goroutine.
Made StatusCallback/AssignmentSource/AssignmentCallback private,
added cbMu mutex and SetXxx() setter methods. Lock held only for
load/store, not during callback execution.
7 dev tests + 13 QA adversarial tests = 20 new tests.
972 total unit tests, all passing.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
BlockVolumeHeartbeatCollector periodically collects block volume status
via callback (standalone, no gRPC wiring yet). Store() accessor on
BlockService. Three bugs found by QA and fixed: Stop-before-Run deadlock
(BUG-CP4B2-1), zero interval panic (BUG-CP4B2-2), callback panic crashes
goroutine (BUG-CP4B2-3). 12 new tests (3 dev + 9 QA adversarial).
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
pingqiuandClaude Opus 4.6
Chris LuandGitHub
Chris Lu
Konstantin LebedevandGitHub