mirrors/seaweedfs
1
0
Fork 0
mirror of https://github.com/seaweedfs/seaweedfs.git synced 2026-09-20 15:04:37 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: CP13-1 baseline — remove checkpoint closure claims, fix stale inventory

Browse Source 
- phase-13-log.md: mark pre-baseline inventory table as superseded,
  point to phase-13-cp1-baseline.md for authoritative results
- phase-13-cp1-baseline.md: replace "CP13-X done" language with neutral
  "current code passes this test; suggests behavior may already exist"
  — checkpoint closure still requires dedicated review
- Expand remaining-open-checkpoints section: CP13-2/5/6/7 all still
  require review, main fails cluster around CP13-5 but CP13-7 and
  part of CP13-6 also remain open

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
pingqiu
2026-04-02 17:16:27 -07:00
co-authored by Claude Opus 4.6
parent 600dac6029
commit 90425b588e
2 changed files with 39 additions and 82 deletions
Download Patch File Download Diff File Expand all files Collapse all files
sw-block/.private/phase/phase-13-cp1-baseline.md
+31 -18
View File
@@ -21,11 +21,11 @@ Protocol changes in this checkpoint: NONE — test-first baseline only
| Result | Test | Reason |
|--------|------|--------|
| PASS | `TestReplicaProgress_BarrierUsesFlushedLSN` | barrier now gates on replicaFlushedLSN (CP13-3 done) |
| PASS | `TestReplicaProgress_FlushedLSNMonotonicWithinEpoch` | flushedLSN monotonic within epoch (CP13-3 done) |
| PASS | `TestReplicaProgress_BarrierUsesFlushedLSN` | current code passes this test; suggests CP13-3 behavior may already exist |
| PASS | `TestReplicaProgress_FlushedLSNMonotonicWithinEpoch` | current code passes this test; suggests CP13-3 behavior may already exist |
| PASS | `TestBarrier_RejectsReplicaNotInSync` | barrier rejects non-InSync replica |
| PASS | `TestBarrier_EpochMismatchRejected` | barrier rejects epoch mismatch |
| PASS | `TestBarrier_DuringCatchup_Rejected` | barrier rejected during CatchingUp state (CP13-4 done) |
| PASS | `TestBarrier_DuringCatchup_Rejected` | current code passes this test; suggests CP13-4 behavior may already exist |
| PASS | `TestBarrier_ReplicaSlowFsync_Timeout` | barrier timeout on slow replica |
| PASS | `TestBarrierResp_FlushedLSN_Roundtrip` | barrier response wire format carries flushedLSN |
| PASS | `TestBarrierResp_BackwardCompat_1Byte` | backward compat with old 1-byte response |
@@ -36,14 +36,14 @@ Protocol changes in this checkpoint: NONE — test-first baseline only
| PASS | `TestShipperGroup_MinReplicaFlushedLSN` | group computes min flushedLSN across replicas |
| PASS | `TestDistSync_SyncAll_NilGroup_Succeeds` | sync_all with no replicas succeeds locally |
| PASS | `TestDistSync_SyncAll_AllDegraded_Fails` | sync_all fails when all replicas degraded |
| PASS | `TestBug2_SyncAll_SyncCache_AfterDegradedShipperRecovers` | after recovery, catch-up + barrier succeeds (CP13-5 done) |
| PASS | `TestBug2_SyncAll_SyncCache_AfterDegradedShipperRecovers` | current code passes this test; suggests CP13-5 behavior may already exist |
| PASS | `TestBug1_SyncAll_WriteDuringDegraded_SyncCacheMustFail` | SyncCache correctly fails during degraded |
## Category 3: Reconnect / Catch-up
| Result | Test | Reason |
|--------|------|--------|
| PASS | `TestReconnect_CatchupFromRetainedWal` | reconnect + WAL catch-up works (CP13-5 done) |
| PASS | `TestReconnect_CatchupFromRetainedWal` | current code passes this test; suggests CP13-5 catch-up behavior may already exist |
| PASS* | `TestReconnect_GapBeyondRetainedWal_NeedsRebuild` | correctly fails SyncCache after large gap, but does NOT assert NeedsRebuild state transition — asserts barrier failure only → CP13-5+CP13-7 |
| PASS | `TestReconnect_EpochChangeDuringCatchup_Aborts` | catch-up aborts on epoch change |
| PASS | `TestReconnect_CatchupTimeout_TransitionsDegraded` | catch-up timeout → degraded |
@@ -58,8 +58,8 @@ Protocol changes in this checkpoint: NONE — test-first baseline only
| Result | Test | Reason |
|--------|------|--------|
| PASS | `TestWalRetention_RequiredReplicaBlocksReclaim` | replica-aware WAL retention works (CP13-6 done) |
| PASS | `TestWalRetention_TimeoutTriggersNeedsRebuild` | retention timeout → NeedsRebuild (CP13-6 done) |
| PASS | `TestWalRetention_RequiredReplicaBlocksReclaim` | current code passes this test; suggests CP13-6 retention behavior may already exist |
| PASS | `TestWalRetention_TimeoutTriggersNeedsRebuild` | current code passes this test; suggests CP13-6 timeout behavior may already exist |
| PASS* | `TestWalRetention_MaxBytesTriggersNeedsRebuild` | passes but logs "max-bytes retention trigger not implemented yet" — shipper stays Degraded, does not transition to NeedsRebuild → CP13-6 |
| FAIL | `TestAdversarial_NeedsRebuildBlocksAllPaths` | **gap: after large WAL gap, shipper stays Degraded instead of NeedsRebuild; Ship/Barrier not blocked** → CP13-5+CP13-7 |
| FAIL | `TestAdversarial_CatchupDoesNotOverwriteNewerData` | **gap: catch-up after disconnect fails at barrier level** — catch-up doesn't complete, so newer-data safety not actually exercised → CP13-5 |
@@ -81,20 +81,33 @@ Protocol changes in this checkpoint: NONE — test-first baseline only
## Failure → Checkpoint Mapping
| FAIL Test | Root Cause | Closes In |
|-----------|-----------|-----------|
| `TestAdversarial_ReconnectUsesHandshakeNotBootstrap` | degraded shipper reconnects but doesn't catch up before barrier | CP13-5 |
| `TestAdversarial_CatchupMultipleDisconnects` | repeated disconnect/reconnect cycles don't recover | CP13-5 |
| `TestAdversarial_NeedsRebuildBlocksAllPaths` | shipper stays Degraded after large gap, should be NeedsRebuild | CP13-5 + CP13-7 |
| `TestAdversarial_CatchupDoesNotOverwriteNewerData` | catch-up fails, so newer-data safety not exercised | CP13-5 |
| FAIL Test | Root Cause | Expected to close in |
|-----------|-----------|----------------------|
| `TestAdversarial_ReconnectUsesHandshakeNotBootstrap` | degraded shipper reconnects but doesn't catch up before barrier | CP13-5 (reconnect handshake) |
| `TestAdversarial_CatchupMultipleDisconnects` | repeated disconnect/reconnect cycles don't recover | CP13-5 (reconnect handshake) |
| `TestAdversarial_NeedsRebuildBlocksAllPaths` | shipper stays Degraded after large gap, should be NeedsRebuild | CP13-5 (gap detection) + CP13-7 (rebuild fallback) |
| `TestAdversarial_CatchupDoesNotOverwriteNewerData` | catch-up fails at barrier, newer-data safety not exercised | CP13-5 (catch-up protocol) |
Main remaining failures cluster around CP13-5 (reconnect/catch-up), but CP13-7 (rebuild fallback) and part of CP13-6 (max-bytes retention) also remain open.
## PASS* → Checkpoint Mapping
| PASS* Test | Why Not Full Proof | Closes In |
|------------|-------------------|-----------|
| `TestBug3_ReplicaAddr_MustBeIPPort_WildcardBind` | documents gap, doesn't prove fix | CP13-2 |
| `TestReconnect_GapBeyondRetainedWal_NeedsRebuild` | asserts barrier failure, not NeedsRebuild state | CP13-5 + CP13-7 |
| `TestWalRetention_MaxBytesTriggersNeedsRebuild` | logs "not implemented", shipper stays Degraded | CP13-6 |
| PASS* Test | Why Not Full Proof | Expected to close in |
|------------|-------------------|----------------------|
| `TestBug3_ReplicaAddr_MustBeIPPort_WildcardBind` | documents gap, doesn't prove fix | CP13-2 (canonical addressing) |
| `TestReconnect_GapBeyondRetainedWal_NeedsRebuild` | asserts barrier failure, not NeedsRebuild state transition | CP13-5 (gap detection) + CP13-7 (rebuild fallback) |
| `TestWalRetention_MaxBytesTriggersNeedsRebuild` | logs "not implemented", shipper stays Degraded | CP13-6 (max-bytes retention) |
## Remaining Open Checkpoints
This baseline does NOT close any checkpoint. Checkpoint closure requires dedicated review per checkpoint. The baseline only records which tests pass or fail on current code.
Tests passing on current code **suggests** the behavior may already exist, but does not constitute checkpoint acceptance. The following checkpoints still require dedicated review:
- **CP13-2** (canonical addressing): 1 PASS* test documents the gap
- **CP13-5** (reconnect/catch-up): 2 FAILs + 1 PASS* directly expose missing protocol
- **CP13-6** (WAL retention): 1 PASS* exposes missing max-bytes trigger
- **CP13-7** (rebuild fallback): 1 FAIL + 1 PASS* expose missing NeedsRebuild transition
## What Was NOT Changed
sw-block/.private/phase/phase-13-log.md
+8 -64
View File
@@ -128,70 +128,14 @@ Reject if:
###### Task 1: Baseline Inventory Freeze
Collect the existing test inventory and classify each test. The inventory below is the frozen starting point — `sw` validates it against current code, fixes classification errors, and adds missing entries only.
**Category 1: Address Truth**
| Test | File | Status | Classification |
|------|------|--------|----------------|
| `TestCanonicalizeAddr_WildcardIPv4_UsesAdvertised` | `net_util_test.go` | PASS | existing, reusable |
| `TestCanonicalizeAddr_WildcardIPv6_UsesAdvertised` | `net_util_test.go` | PASS | existing, reusable |
| `TestCanonicalizeAddr_NilIP_UsesAdvertised` | `net_util_test.go` | PASS | existing, reusable |
| `TestCanonicalizeAddr_AlreadyCanonical_Unchanged` | `net_util_test.go` | PASS | existing, reusable |
| `TestCanonicalizeAddr_Loopback_Unchanged` | `net_util_test.go` | PASS | existing, reusable |
| `TestCanonicalizeAddr_NoAdvertised_FallsBackToOutbound` | `net_util_test.go` | PASS | existing, reusable |
| `TestBug3_ReplicaAddr_MustBeIPPort_WildcardBind` | `sync_all_bug_test.go` | PASS* | documents gap: ReplicaReceiver may return `:port` not `ip:port` |
**Category 2: Durable Progress Truth**
| Test | File | Status | Classification |
|------|------|--------|----------------|
| `TestReplicaProgress_BarrierUsesFlushedLSN` | `sync_all_protocol_test.go` | FAIL expected | gap: barrier doesn't gate on replicaFlushedLSN |
| `TestReplicaProgress_FlushedLSNMonotonicWithinEpoch` | `sync_all_protocol_test.go` | FAIL expected | gap: replicaFlushedLSN API missing |
| `TestBarrier_EpochMismatchRejected` | `sync_all_protocol_test.go` | FAIL expected | gap: barrier doesn't check epoch on replica |
| `TestBarrier_ReplicaSlowFsync_Timeout` | `sync_all_protocol_test.go` | FAIL expected | gap: barrier timeout is hardcoded |
| `TestBarrier_RejectsReplicaNotInSync` | `sync_all_protocol_test.go` | verify | existing, needs verification |
| `TestBarrierResp_FlushedLSN_Roundtrip` | `sync_all_protocol_test.go` | verify | existing, needs verification |
| `TestBarrierResp_BackwardCompat_1Byte` | `sync_all_protocol_test.go` | verify | existing, needs verification |
| `TestReplica_FlushedLSN_OnlyAfterSync` | `sync_all_protocol_test.go` | verify | existing, needs verification |
| `TestReplica_FlushedLSN_NotOnReceive` | `sync_all_protocol_test.go` | verify | existing, needs verification |
| `TestShipper_ReplicaFlushedLSN_UpdatedOnBarrier` | `sync_all_protocol_test.go` | verify | existing, needs verification |
| `TestShipper_ReplicaFlushedLSN_Monotonic` | `sync_all_protocol_test.go` | verify | existing, needs verification |
| `TestShipperGroup_MinReplicaFlushedLSN` | `sync_all_protocol_test.go` | verify | existing, needs verification |
| `TestDistSync_SyncAll_NilGroup_Succeeds` | `dist_group_commit_test.go` | PASS | existing, reusable |
| `TestDistSync_SyncAll_AllDegraded_Fails` | `dist_group_commit_test.go` | PASS | existing, reusable |
| `TestBug2_SyncAll_SyncCache_AfterDegradedShipperRecovers` | `sync_all_bug_test.go` | FAIL expected | gap: catch-up not implemented, barrier hangs after recovery |
**Category 3: Reconnect / Catch-up**
| Test | File | Status | Classification |
|------|------|--------|----------------|
| `TestReconnect_CatchupFromRetainedWal` | `sync_all_protocol_test.go` | FAIL expected | gap: no reconnect handshake or WAL catch-up |
| `TestReconnect_GapBeyondRetainedWal_NeedsRebuild` | `sync_all_protocol_test.go` | FAIL expected | gap: no retention tracking, no NeedsRebuild transition |
| `TestReconnect_EpochChangeDuringCatchup_Aborts` | `sync_all_protocol_test.go` | FAIL expected | gap: no CatchingUp state, no epoch-aware abort |
| `TestReconnect_CatchupTimeout_TransitionsDegraded` | `sync_all_protocol_test.go` | FAIL expected | gap: no catch-up timeout |
| `TestBarrier_DuringCatchup_Rejected` | `sync_all_protocol_test.go` | FAIL expected | gap: no CatchingUp state |
| `TestAdversarial_FreshShipperUsesBootstrapNotReconnect` | `sync_all_adversarial_test.go` | verify | existing, needs verification |
| `TestAdversarial_ReconnectUsesHandshakeNotBootstrap` | `sync_all_adversarial_test.go` | FAIL expected | gap: handshake protocol missing |
| `TestAdversarial_ReplicaRejectsDuplicateLSN` | `sync_all_adversarial_test.go` | verify | existing, needs verification |
| `TestAdversarial_ReplicaRejectsGapLSN` | `sync_all_adversarial_test.go` | verify | existing, needs verification |
| `TestAdversarial_CatchupMultipleDisconnects` | `sync_all_adversarial_test.go` | FAIL expected | gap: no catch-up protocol |
| `TestAdversarial_ConcurrentBarrierDoesNotCorruptCatchupFailures` | `sync_all_adversarial_test.go` | verify | existing, needs verification |
**Category 4: Retention / Rebuild Boundary**
| Test | File | Status | Classification |
|------|------|--------|----------------|
| `TestWalRetention_RequiredReplicaBlocksReclaim` | `sync_all_protocol_test.go` | FAIL expected | gap: WAL reclaim not replica-aware |
| `TestWalRetention_TimeoutTriggersNeedsRebuild` | `sync_all_protocol_test.go` | FAIL expected | gap: no retention timeout |
| `TestWalRetention_MaxBytesTriggersNeedsRebuild` | `sync_all_protocol_test.go` | FAIL expected | gap: no max-bytes retention |
| `TestAdversarial_NeedsRebuildBlocksAllPaths` | `sync_all_adversarial_test.go` | FAIL expected | gap: NeedsRebuild state incomplete |
| `TestAdversarial_CatchupDoesNotOverwriteNewerData` | `sync_all_adversarial_test.go` | verify | existing, needs verification |
| `TestHeartbeat_ReportsPerReplicaState` | `rebuild_v1_test.go` | verify | existing, needs verification |
| `TestHeartbeat_ReportsNeedsRebuild` | `rebuild_v1_test.go` | verify | existing, needs verification |
| `TestReplicaState_RebuildComplete_ReentersInSync` | `rebuild_v1_test.go` | verify | existing, needs verification |
| `TestRebuild_AbortOnEpochChange` | `rebuild_v1_test.go` | verify | existing, needs verification |
| `TestRebuild_PostRebuild_FlushedLSN_IsCheckpoint` | `rebuild_v1_test.go` | verify | existing, needs verification |
> **Note:** This section was the initial expected inventory written before the baseline run.
> It has been **superseded** by the actual frozen baseline in `phase-13-cp1-baseline.md`.
> See that file for the real PASS / FAIL / PASS* results from running on current code.
>
> Key corrections from the actual run:
> - Many tests labeled `FAIL expected` here actually **PASS** on current code (CP13-3/4/5/6 behavior already implemented in earlier phases)
> - Many tests labeled `verify` turned out to be **PASS** (real proof, not just witness)
> - Only **4 tests actually FAIL** and **3 are PASS*** — see `phase-13-cp1-baseline.md` for the authoritative list
###### Task 2: Runnable Baseline Harness