Repository navigation
filesystem: trace journal accounting transitions - #1344
Merged
Merged
Conversation
Signed-off-by: bootjp <contact@bootjp.me>
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 38 minutes. View limit details
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. Comment |
This was referenced Oct 28, 2026
bootjp
marked this pull request as ready for review
October 29, 2026 06:11
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
bootjp
added a commit
that referenced
this pull request
Oct 29, 2026
Design: docs/design/2026_10_28_proposed_validation_lint_baseline.md ## Purpose Restore the **unfiltered** repository lint baseline required by the test-cleanup design, and make one native-snapshot acceptance wait honor its existing phase deadline. This is a separate validation prerequisite, not a claim that #1330 or all unfinished designs are complete. The design was committed before implementation and independently reviewed. It covers all eight findings on main `64fc9e62`: four complexity reports, two duplicate bodies, one unused-parameter report, and one nested-test complexity report. Removing the first unused argument revealed a second always-nil wrapper argument; all five callers were audited before removing both. General routed publication still accepts real effect/read-key arguments. ## Changes - Preserve main's migration of the memory dispatcher to `kvtest.LocalCoordinator`; keep the added timestamp-boundary, copy, unsupported-operation and OCC regressions on that shared path. The obsolete local helpers are not restored. - Extract journaled FUSE writes and share unlink/rmdir lifecycle handling. Operation-specific capabilities remain independent; session/capability checks precede operation-ID allocation. Admission, refresh, empty writes, receipts, unknown-outcome classification, and logging retain their order and errno behavior. - Share the existing zero-or-mismatched attempt predicate in journal planning/publication/rejection. Publication and duplicate-resolution ordering is unchanged. - Split the raw verdict predicate into protocol categories with the exact prior sentinel set/evaluation order and readiness exclusion priority. - Preserve all 24 migration-control protection cases and every assertion while extracting their case body. - Replace the separate five-second snapshot wait with a poll bounded by the **existing 60-second write/snapshot phase**. All actual members, including the learner, still need persisted native snapshots covering the ACKed write before restart; restored bytes and PREPARE evidence assertions are unchanged. No production wire/on-disk format, timestamp issuance, Raft settings, authorization, retry policy, or default flags change. No suppression, new exclusion, shorter workload, or test skip is added. PR #1344's accounting trace and workflow changes are not duplicated or incorporated; its changes affect different journal bodies from this refactor. ## Verification - **Unfiltered golangci-lint v2.15.0: PASS, 0 issues** with `GOMAXPROCS=4 golangci-lint run ./... --timeout=10m`. No `--new-from-rev` filter. The same command failed with the eight findings before implementation. - Focused new parity tests passed before and after their refactors: FUSE partial capabilities/admission/session/unknown-outcome behavior; dispatcher copy/boundary/OCC behavior; attempt identity; raw verdict allowlist/exclusion. The extracted migration case set retains all 24 names/assertions. - `GOMAXPROCS=4 go test -p 2 -race ./adapter -run '^TestQuorumAcceptanceSnapshotWait|^TestMigrationQuorumPartitionAcceptance$' -count=2 -timeout=5m`: PASS (74.843s), including actual native snapshot, restart, restored bytes, and resolution-evidence requirements. - Full filesystem/KV/root race: PASS (exit 0), `GOMAXPROCS=4 go test -p 2 -race ./internal/filesystem/... ./kv/... . -count=1 -timeout=20m`. Filesystem 103.569s, fstest 1.822s, fuseadapter 11.005s, KV 69.721s, kvtest 1.781s, root 104.345s. No short/skip flags are used. Kernel FUSE checks require Linux CI rather than macOS local execution. - An earlier worker full filesystem attempt failed in `TestHandleContentionIsTerminalAndReplayable` with deadline/unknown-outcome errors. Its cause was not established; a later parent full filesystem run passed. The earlier failure is not counted as passing evidence. - `git diff --check`: PASS. ### Snapshot Regression Evidence PR #1346's Go CI failed `TestMigrationQuorumPartitionAcceptance` at the five-second wait for all four persisted native snapshots. Three isolated two-CPU race repetitions of that unchanged test passed (90.317s). That does not establish a production regression or identify the physical cause of the CI delay. The new focused regression makes the final member's snapshot coverage arrive after 5.25 seconds, within its phase context. With the old five-second sub-budget it **failed at 5.00s**, showing target 22 and snapshots `[22 22 22 21]`. It passes when the helper uses the caller's existing phase deadline. Separate cases reject partial/empty coverage, canceled phases, and deadline expiration. This changes neither the real acceptance phase's 60-second budget nor its payload, member count, snapshot threshold, or restored-data assertions. The helper reports target and last observed snapshot indexes on failure. The test remains red if actual native snapshots do not cover the write within the phase. It does not substitute applied indexes or a stub for the real four-member acceptance case. ## Conflict Resolution Against Current Main Merged main `75daeeb800ee9b661e5dfd3d9b91fb656928c145` without rewriting existing history. Final HEAD: `3d2d0b436c8d953031713460bf2d1e229c149eae` (bootjp author/committer, `contact@bootjp.me`, signature Verified and DCO sign-off). - Resolve four conflicted files: preserve main's shared coordinator migration and exact-size bootstrap fixture, retain this PR's explicit cap-error and durable peer-metadata assertions, and keep exactly one compile-time applied-index interface assertion. - Adapt the new memory-dispatcher regressions to main's existing invalid-request sentinel, prefix validation order, nested error wrapping and empty-slice copy semantics. No production decision/error contract is changed by this resolution; all dispatcher/service callers were checked. - Include only mechanical lint fixes for shared-helper result declarations and import grouping exposed by the main merge. No workload, timeout, assertion or exclusion was removed. - Independent read-only conflict/FUSE integration review: no findings. - Targeted conflict-area race tests: PASS (fstest, kvtest, fsmwire, live Raft config persistence). - Full filesystem/KV/Raft/fsmwire/root race command: PASS, exit 0 (`GOMAXPROCS=4 go test -p 2 -race ./internal/filesystem/... ./kv/... ./internal/raftengine/... ./internal/encryption/fsmwire . -count=1 -timeout=20m`). Filesystem 101.625s, fuseadapter 10.540s, KV 72.379s, etcd 54.961s, root 108.109s. No explicit short/skip filters; kernel-FUSE checks require Linux CI. - Affected DynamoDB/S3 helper-binding tests plus the real four-member snapshot/restart acceptance and snapshot-wait regressions: race PASS, exit 0 (34.184s). - Initial unfiltered lint on the main merge found four new reports; mechanical fixes were applied. Final unfiltered lint, without auto-fix or a differential filter: PASS, 0 issues (`GOMAXPROCS=4 golangci-lint run ./... --timeout=10m`, exit 0). A subsequent commit hook also passed with no remaining edits. - `git diff --check`: PASS. GitHub reports MERGEABLE. Fresh post-push review and CI are pending; old-head reviews/CI are historical only. ## Risk and Self-review 1. Data loss: journal effect/OCC ordering, receipt handling, and raw error classification are unchanged; native snapshot and restored-byte checks are retained. 2. Concurrency / distributed failures: dispatcher serialization and FUSE mutation-ticket lifetime are preserved. The snapshot poll uses the original phase cancellation/deadline and verifies every member. Full race results and their limits are above. 3. Performance: helpers share existing work rather than add network/consensus operations or new per-key scans. The polling allocation is test-only. 4. Data consistency: all prior verdict sentinels and exclusion precedence, attempt identity checks, mutation copies, timestamp patch bounds, and all migration-control combinations are explicitly pinned. 5. Test coverage: behavior parity was exercised before and after refactoring; the wait regression was observed red before the fix. Unfiltered full lint is distinguished from differential CI. Linux kernel acceptance and remote CI are not claimed complete yet. Related: #1330, #1345, #1346. Earlier PR heads do not retroactively gain this clean baseline; they need to incorporate it and re-run latest-HEAD review/CI. Author: bootjp <!-- This is an auto-generated comment: release notes by coderabbit.ai --> <!-- coderabbit-ai-generated:v1 --> ## Summary by CodeRabbit * **不具合修正** * 分散環境でのスナップショット確認を、書き込み内容が全メンバーに反映されるまで待つよう変更しました。待機中に期限切れやキャンセルが発生した場合は、状況を示すエラーを返します。 * ファイル操作やデータ移行時の検証を強化し、未対応の操作や不正な入力による予期しない適用を防ぎます。 * **ドキュメント** * 検証基準と受け入れ条件に関する提案書を追加しました。 <!-- end of auto-generated comment: release notes by coderabbit.ai -->
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.