Skip to content

test(collections): target the actual WAL format in split recovery - #5391

Merged
snissn merged 3 commits into
mainfrom
repair/split-recovery-actual-wal-main-01
Nov 4, 2026
Merged

snissn merged 3 commits into
mainfrom
repair/split-recovery-actual-wal-main-01

Conversation

@snissn

@snissn snissn commented Nov 4, 2026 •

Copy link
Copy Markdown
Owner

The split-insert recovery fixture assumes its mutated WAL is fixed v3. Seeding a closed fixed predecessor does not ensure that: Open advances to a new writer, whose container follows the existing medium policy. This patch locates the actual closed acknowledged segment and injects corruption or a torn tail in its real fixed/stream layout.

Reuse the strict V2 reader, offsets and terminal-tail inspector; retain fixed extent/witness geometry and truncate genuine stream bytes after complete frame identity. Remove ineffective pre-open seed plumbing. No product format, medium policy, durability, journal or recovery implementation changes.

Validation:

  • Exact current head eacbfd1/tree6b0fc6bd9f6a22621c85526342727509fc3bcafe received independent complete source review plus review of its four-space/inventory correction. Inventory refresh/check passes at17workflows/63members, with only discovery binding changed. Exact-head Codex review completed with no findings.
  • Current TreeDB41773223457 Linux normal and race raw artifacts each prove3tops/19namedoutcomes/16leaves run/PASS and collections packagePASS, bound to actual synthetic840559dbdaa1a0862e2499eee6f42d67fef031e2 whose tree equals the candidate. Existing accepted-prefix, no false row/LSN advance, retry identity and reopen assertions remain unchanged. Focused selector: ^Test(SplitVectorInsertTerminalTailRecoveryV1|SplitCollectionRecoveryActualContainerFaultsV1|SplitCollectionRecoveryFaultRefusalsV1)$.
  • macOS and Windows each prove16 direct control outcomes PASS; the existing Linux-only namespace guard skips the original recovery parent, so its two children do not enter on those platforms. macOS real fixed/stream controls enter; Windows stream controls are positive, while fixed cases assert unsupported-factory refusal, not positive fixed-format recovery.
  • Corrected-head hosted Go1.26.10 Format41773218504 passed. All8 current-head Actions workflows passed, including all38 TreeDB jobs and required app15368 gate126396180941. All54 check contexts passed, including the completed exact-head CodeRabbit review with no actionable comments. Its generic docstring-coverage warning is advisory; no repository configuration applied and the check concluded SUCCESS. The existing branch-policy and issue gates remain unchanged. All local Go checks remain UNRUN; no local host runtime is admitted. Previous head10861 Format failure is retained.
  • Original ci: expose macOS vet progress without changing its gate #5385 race8 failure and previous passes remain retained. Actual failing filesystem/header was not recorded; this is no claim of reproduced hosted cause or product recovery defect.
  • Fixture cost adds a bounded image (max64MiB), strict scan and control oracle copies; fixed writes retain reservation. Sparse capacity refusal is counted in logical output. Existing resource/deadline controls stay unchanged. No public performance claim.

Semantic source review SHA256 ef4bf0ff13f1b52c53e53f53a5bea9cf34f1fd00b45035b8ca52e07b846381f6; exact corrected-head review efa531fcb2af486e755aa4f29ec3735d7554d7492ed1c1b75c92e8f3edf7ab44; named/platform entry audit b3a5afd4da52a056cba24c59c0d46035730631645f6e9bb280c974aaa8adc5a4. Current required CI, independent source/entry review and exact-head Codex and CodeRabbit reviews are complete. Final live policy/base/head/threads/merge-tree readiness checks govern integration. #5386's distinct Windows startup timeout and broader performance acceptance stay open.

Fixes #5389. Parent #5225.

Summary by CodeRabbit

  • Tests
    • Expanded recovery test coverage across supported storage formats, including corrupted or incomplete data scenarios.
    • Added checks for invalid and unsupported inputs, including cases where recovery operations must leave data unchanged.
    • Updated test fixtures for vector collections and partition scenarios.

@coderabbitai

coderabbitai Bot commented Nov 4, 2026 •

Copy link
Copy Markdown

Explain Change →

No actionable comments were generated in the recent review.

Walkthrough

The recovery tests now use production-selected WAL containers and mutate frames identified in the actual WAL image. They cover stream and fixed-container layouts, verify recovery-reader outcomes, and reject unsupported or invalid inputs without modifying the image.

Changes: Split-insert recovery tests

Layer / File(s) Summary
Use standard production fixtures
TreeDB/collections/column_vector_graph_typed_column_test.go, TreeDB/collections/vector_index_partition_live_v1_test.go, TreeDB/collections/vector_partition_split_insert_recovery_v1_test.go
The fixture helpers no longer pass pre-open setup callbacks. The terminal-tail recovery test uses the standard production fixture.
Locate and damage terminal frames
TreeDB/collections/vector_partition_split_insert_recovery_v1_test.go
Frame selection checks the durable command identity and terminal position. Shared helpers locate frames and apply corruption or relaxed-tail damage to stream and fixed-container images.
Exercise container formats and refusal cases
TreeDB/collections/vector_partition_split_insert_recovery_v1_test.go, .github/ci/ci_impact.json
Tests create WALs with maintained writers and check mutation outcomes and refusal cases. The CI impact inventory hash also changes.

Estimated code review effort: 4 (Complex) | ~45 minutes

Change: Other

Possibly related PRs

  • snissn/gomap#5224: Adds fixed-WAL reader and frame-inspection support used by this PR’s split-insert recovery tests.
Merge Risk: Minimal · up to eacbf

The test-only changes have no established merge-blocking defect. Complete the required exact-head CI before merging.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 44.44% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documenta… Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title "test(collections): target the actual WAL format in split recovery" matches the main change. The changeset rewrites split-insert recovery tests to locate and mutate the real WAL container (f…
Linked Issues check Passed Issue #5389 requires test-fixture-only fault placement in the actual closed, acknowledged WAL segment. The recovery diff removes pre-open fixed-WAL seeding and locates the acknowledged frame with `New…
Out of Scope Changes check Passed The changes remain within issue #5389. The main change is in vector_partition_split_insert_recovery_v1_test.go. The two helper-file changes remove the now-unused pre-open setup callback that enabled…
Full details: Title check

Explanation

The title "test(collections): target the actual WAL format in split recovery" matches the main change. The changeset rewrites split-insert recovery tests to locate and mutate the real WAL container (fixed or stream layout) instead of pre-creating fixed WAL segments, and removes the ineffective pre-open setup plumbing. The title is concise, specific, and follows a conventional commit style. It does not list every detail, which is expected.

Full details: Linked Issues check

Explanation

Issue #5389 requires test-fixture-only fault placement in the actual closed, acknowledged WAL segment. The recovery diff removes pre-open fixed-WAL seeding and locates the acknowledged frame with NewReaderWithOptions, ReadCommandFrameV2, offsets, and terminal-tail inspection. It validates frame identity, bounds, fixed extent geometry, stream layout, durability, and refusal cases. It preserves the existing recovery, retry, lifecycle, and no-false-advance assertions. The added controls exercise fixed and stream writers, corruption, incomplete frames, and unsupported fixed creation. The reported focused normal/race tests and hosted validation passed. No product implementation changed.

Full details: Out of Scope Changes check

Explanation

The changes remain within issue #5389. The main change is in vector_partition_split_insert_recovery_v1_test.go. The two helper-file changes remove the now-unused pre-open setup callback that enabled the ineffective seed path. The CI inventory change updates discovery metadata. The diff contains no product format, medium-policy, durability, journal, recovery, workload, or unrelated #5386/#5373 changes.

Full details: Docstring Coverage

Explanation

Docstring coverage is 44.44% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 9 functions across 2 files. (1 skipped: 1 unsupported.)

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

Finishing Touches
Generate docstrings
  • Commit to this branch
  • Create a new PR
Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share
Recent review info
Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: b4303fd1-055d-4a20-a5a1-2ffc559af846
Commits

Reviewing files that changed from the base of the PR and between b2f9c7f and eacbfd1.

Files selected for processing: 4
  • .github/ci/ci_impact.json
  • TreeDB/collections/column_vector_graph_typed_column_test.go
  • TreeDB/collections/vector_index_partition_live_v1_test.go
  • TreeDB/collections/vector_partition_split_insert_recovery_v1_test.go

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

snissn commented Nov 4, 2026

Copy link
Copy Markdown
Owner Author

@codex review

Please review exact current head eacbfd1 (tree6b0fc6bd9f6a22621c85526342727509fc3bcafe). Focus on actual closed acknowledged WAL target identity, fixed/stream physical offsets and witness geometry, fail-closed refusal controls, preserved recovery/retry assertions and bounded fixture cost. This is test-only; no product medium-policy change. The preceding Format failure is retained; corrected-head hosted Go1.26.10 Format passed. Full current-head normal/race/affected-scope CI remains running and independently blocking.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Nov 4, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-11-04T04:56:49.935772Z eacbfd1 Draft marked ready
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Swish!

Reviewed commit: eacbfd10ec

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@snissn
snissn marked this pull request as ready for review November 4, 2026 04:54
Copilot AI balanced review requested due to automatic review settings November 4, 2026 04:54

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 insufficient remaining quota. Try requesting a review at a lower effort level.

@snissn
snissn merged commit 8e11330 into main Nov 4, 2026
54 checks passed
@snissn
snissn deleted the repair/split-recovery-actual-wal-main-01 branch November 4, 2026 05:07

snissn commented Nov 4, 2026

Copy link
Copy Markdown
Owner Author

Coordinator merge verified on GitHub: 8e1133085b4052e344a73063f28c2ab142485c73, tree 6b0fc6bd9f6a22621c85526342727509fc3bcafe, parents actual main b2f9 + qualified eac. The actual merge tree equals the qualified candidate/synthetic tree. All54 exact-head checks passed (all38 TreeDB jobs), Codex and CodeRabbit completed with no actionable findings, threads empty; Copilot quota exhaustion was not counted as review. Live rules/head/base were rechecked before expected-head merge.

Automatic actual-main qualification is now running: TreeDB41777285003 plus the six other push workflows. Dependent publications wait for those gates. Original failure/format history and platform-entry limitations remain retained; no public performance claim.

Final readiness SHA256 7b5203ede99563e7d23666efe7d74de42f2d1751a4c065d68cfee0e6384e4615; actual merge receipt 626d4e04d4043c6a7e9aea67b34ec82761ffc32ed3770d29f447bf620ecb99b1.

snissn commented Nov 4, 2026

Copy link
Copy Markdown
Owner Author

Actual-main qualification is BLOCKED: windows-nativewire126400474982 failed on actual8e113308/Go1.26.10 Windows. The only functional failing child is TestFixedPeerTCPNativeCollectionCatalogGuardInterpositionV1/bson: fixed_peer_native_collections_v1_test.go:395, submit guard-create returns raftcluster: commit ambiguous joined with HTTP body/context deadline. Child4.46s,parent5.51s; JSON childPASS1.06s; package411.982s. No WARNING: DATA RACE.

This is distinct from #5386's earlier sparse-catalog30s setup timeout and #5373's separately owned Raft failure. Cause remains INCONCLUSIVE. Original artifact13298166791 ZIP SHA256 34710e3fe772926a5aaa5b92f8869481ba0de7d6ec5897d52025e0755c4ac512, rawJSON e336838497e3a76dea13e5698abe63aa604a068c182a1b4d848e451ff3b4667c retained. Existing source owner has one bounded actual-input/call-path diagnosis; no unchanged rerun, timeout/skip/gate change or product repair selected. Other post-main jobs are still running. Dependent publications stay held.

snissn commented Nov 4, 2026

Copy link
Copy Markdown
Owner Author

Actual-main post-merge qualification is terminal: run41777285003 has38 jobs,36PASS; windows-nativewire126400474982 and aggregate126409827568 FAIL. All six other automatic push workflows PASS. The sole functional failure is BSON guard-create/body-read deadline; cause/server outcome remain INCONCLUSIVE, distinct from #5386/#5373. No unchanged rerun.

The owning recovery race shard126400474922 did enter and pass all3 parent tests/16leaves/19outcomes on actual8e. Selected-event audit SHA d494cb2035e4c8146141922650928d9d6757ac20c7569ab508eda87f08068a06. This preserves #5391's merged and exact-head/owning evidence in its actual scope; it does not make default-main green. Final post-main receipt SHA624acd7327d315c0220b5db4a9cad2fa7617cdeb8ae4ffd7fa6d659d89067234.

One bounded, independently reviewed client-trace diagnostic is now #5392/#5393 at f696b71/tree91239e, with one fresh exact-head hosted qualification block. It changes no product code, RPC/deadline/assertion or transport/body and is not a claimed repair. A green diagnostic run does not automatically waive the original blocker. #5385/#5390/#5386 remain held.

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.

TreeDB: place split-insert recovery faults in the actual WAL container

2 participants