ticket: entry-hash-abolish 完了
This commit is contained in:
parent
55c5ac4942
commit
5362e5858c
1
TODO.md
1
TODO.md
|
|
@ -8,7 +8,6 @@
|
|||
- Pod: 任意ターンからの Fork（複数ターン巻き戻しを汎用化） → [tickets/pod-session-fork.md](tickets/pod-session-fork.md)
|
||||
- Pod: Inbound PodEvent ハンドリングの重複を統合 → [tickets/pod-inbound-pod-event-dedup.md](tickets/pod-inbound-pod-event-dedup.md)
|
||||
- 永続化層整理 (Storage)
|
||||
- Entry hash chain 廃止 → [tickets/entry-hash-abolish.md](tickets/entry-hash-abolish.md)
|
||||
- SessionId → SegmentId リネーム → [tickets/segment-rename.md](tickets/segment-rename.md)
|
||||
- Session (Segment 群の grouping) 導入 → [tickets/session-grouping-introduce.md](tickets/session-grouping-introduce.md)
|
||||
- live auto-fork の marker 形式確定 → [tickets/live-fork-marker.md](tickets/live-fork-marker.md)
|
||||
|
|
|
|||
|
|
@ -1,48 +0,0 @@
|
|||
# session-store: Entry hash chain の廃止
|
||||
|
||||
## 背景
|
||||
|
||||
session-store の各 entry は SHA-256 hash chain (`prev_hash` → `hash`) で連結されており、`HashedEntry` として JSONL に 1 行ずつ書かれる。実際に効いている用途は以下の 2 つだけ:
|
||||
|
||||
1. `ensure_head_or_fork` (`crates/pod/src/pod.rs:1591`) — store 末尾と Pod 保持の `head_hash` を比較して auto-fork。**末尾識別子があれば良い**。
|
||||
2. `fork_at(source_id, at_hash)` (`crates/session-store/src/session.rs:425`) — 過去 entry からの fork。`pod-session-fork` の入口仕様は turn 番号であり、entry hash は内部 pointer に過ぎず turn boundary (TurnEnd entry の index) で代替可能。
|
||||
|
||||
宣伝されている改竄検知 (tamper-evident chain) は walk して verify するルートがコード上に存在せず、削除しても regression にならない。
|
||||
|
||||
write 経路は既に sync 化済み (`6e5b148`)。前提足場は揃っている。
|
||||
|
||||
## 要件
|
||||
|
||||
- `HashedEntry` 廃止、JSONL は 1 行 1 `LogEntry`。
|
||||
- `compute_hash` / `build_chain` / `EntryHash` の撤去（外部に公開している場合は呼び出し元を含めて）。
|
||||
- `SessionOrigin.at_hash` → `at_turn_index` (TurnEnd entry 由来の turn 番号) に置換。
|
||||
- `ensure_head_or_fork` の検知ロジックを末尾 seq 比較ベースに置換。形式は実装時に決める（terminal marker entry / 末尾 seq の何れか）。
|
||||
- **`session_head` mutex の撤去**。hash chain が無くなる結果として "head_hash を直前 entry から取得して次へ渡す" という serialize 必須の依存が消える。1 行 < `PIPE_BUF` (Linux 4KB) の `O_APPEND` write は kernel が atomic に直列化するため user space で mutex 不要。
|
||||
- 既存 JSONL の読み込み互換は不要（プロジェクト方針として後方互換性は作らない）。
|
||||
|
||||
## 完了条件
|
||||
|
||||
- `HashedEntry` / `prev_hash` / `compute_hash` / `build_chain` / `EntryHash` がコードベースから消えている。
|
||||
- `SessionOrigin` が `at_turn_index` を保持し、`fork_at` も同 API になっている。
|
||||
- `session_head` mutex への参照が無く、`SessionLogWriter` 系は writer ハンドルを `Arc` で持つだけになっている。
|
||||
- `cargo check --workspace` および `cargo test -p session-store -p pod` が通る。
|
||||
- 既存 session を新規再生成して JSONL が 1 行 1 `LogEntry` になっていることを目視確認できる。
|
||||
|
||||
## 範囲外
|
||||
|
||||
- `SessionId` → `SegmentId` のリネーム（別チケット `segment-rename`）。
|
||||
- 新 `SessionId` (Segment 群の grouping) 導入（別チケット `session-grouping-introduce`）。
|
||||
- live auto-fork の marker 形式の最終決定（別チケット `live-fork-marker`、ここでは末尾 seq 比較相当の最小実装で繋ぐ）。
|
||||
|
||||
## 関連
|
||||
|
||||
- `crates/session-store/src/session_log.rs`
|
||||
- `crates/session-store/src/session.rs`
|
||||
- `crates/pod/src/pod.rs:1591` `ensure_head_or_fork` 経路
|
||||
- `tickets/segment-rename.md` (後続)
|
||||
|
||||
## Review
|
||||
|
||||
- 状態: Approve
|
||||
- レビュー詳細: [./entry-hash-abolish.review.md](./entry-hash-abolish.review.md)
|
||||
- 日付: 2026-05-20
|
||||
|
|
@ -1,56 +0,0 @@
|
|||
# Review: session-store: Entry hash chain の廃止
|
||||
|
||||
対象コミット: `d5dff6d update: entry hash chain と session_head mutex を撤廃`
|
||||
ベース: `develop`
|
||||
|
||||
## 前提・要件の確認
|
||||
|
||||
- `HashedEntry` 廃止 / JSONL = 1 行 1 `LogEntry`: 達成。
|
||||
- `crates/session-store/src/session_log.rs` から型・関連 impl が削除。
|
||||
- `FsStore::append` / `read_all` / `create_session` 全部 `&LogEntry` / `Vec<LogEntry>` に書き換え (`crates/session-store/src/fs_store.rs:68-110`)。
|
||||
- `compute_hash` / `build_chain` / `EntryHash` 撤去: 達成。`grep` でコードベース上に残存なし。`crates/session-store/src/lib.rs:48` の re-export からも消えている。
|
||||
- `SessionOrigin.at_hash → at_turn_index`: 達成 (`crates/session-store/src/session_log.rs:167-177`)。doc コメントで「`TurnEnd` の `turn_count` 由来。`0` は SessionStart 直後」と意味も明文化済み。
|
||||
- `ensure_head_or_fork` を末尾比較ベースに置換: 達成。entry count 比較に統一 (`crates/session-store/src/session.rs:99-118`、Pod 側は `crates/pod/src/pod.rs:1619-1666`)。`Store::read_head_hash` も `read_entry_count` に改名されインタフェースとして整合 (`crates/session-store/src/store.rs:58-64`)。
|
||||
- `session_head` mutex 撤去 / writer ハンドル = `Arc`: 達成。`Arc<SyncMutex<SessionHead>>` を `Arc<SessionState>` (`ArcSwap<SessionId>` + `AtomicUsize`) に置換し (`crates/pod/src/pod.rs:45-86`)、`parking_lot` 依存も削除 (`crates/pod/Cargo.toml:33`)。`LogWriterHandle` は store + state + sink の薄い 3 タプル。
|
||||
- 既存 JSONL の読み込み互換不要: 仕様通り。新フォーマットは「素の `LogEntry` (`tag = "kind"`)」で、旧 `HashedEntry` ラッパー行は deserialize できないが、ticket の明示的な範囲外。
|
||||
|
||||
## 完了条件の対応
|
||||
|
||||
- `HashedEntry / prev_hash / compute_hash / build_chain / EntryHash` 残存無し: 確認 (`grep -r` で hit 無し、コメント/テスト中の文字列残存のみ — 後述 nit)。
|
||||
- `SessionOrigin.at_turn_index` & `fork_at(source_id, at_turn_index)`: 確認 (`session.rs:373-409`)。
|
||||
- `session_head` mutex 参照消失 / Writer は `Arc` のみ: 確認 (`grep -rn parking_lot|SyncMutex|SessionHead crates/pod/src/ crates/session-store/src/` で 0 件)。
|
||||
- `cargo check --workspace` / `cargo test -p session-store -p pod`: 通る。
|
||||
- `pod` 側で `double_run_returns_error` がたまに失敗するが、`KNOWN_ISSUES.md` に既知 flakiness として登録済み (本チケット起因ではない、再実行で pass)。
|
||||
- 既存 session を新規再生成して 1 行 1 `LogEntry`: 形式上は `FsStore::append` が `serde_json::to_string(&LogEntry)` を 1 行で書き出すだけになっており、目視確認できる構造である。
|
||||
|
||||
## アーキテクチャ・スコープ
|
||||
|
||||
- スコープ厳守: 範囲外と宣言した「`SessionId → SegmentId` rename / Session grouping / live-fork marker 形式の最終決定」のいずれにも踏み込んでいない。`fork_at` の API は `at_turn_index: usize` という最小限の置換で止まっており、後続チケットの設計余地を温存している。
|
||||
- レイヤ境界: `session-store` は依然として低レベル persistence、Pod が状態保持と fork 判定を行うという責務分割が維持されている。`LogWriterHandle` / `SessionState` は pod クレート内で完結し、外部に新しい抽象を生やしていない。
|
||||
- 不要な後方互換無し: 旧 JSONL の読み出しを試みる shim も無く、設計方針 (CLAUDE.md「変更量を最小にするために設計を歪めたり、設計問題に対して不必要な後方互換性を作らない」) に合致。
|
||||
- 依存追加 `arc-swap = "1.9.1"`: `cargo add` で入った形跡 (pinned patch version)。`manifest` クレートで既に同 crate を使っており、新規導入ではない。プレフィックス無し短い名前ポリシーと無関係 (third-party)。問題なし。
|
||||
|
||||
## 指摘事項
|
||||
|
||||
### Blocking
|
||||
|
||||
なし。
|
||||
|
||||
### Non-blocking / Follow-up
|
||||
|
||||
- **mirror と disk の順序乖離リスク** — `crates/pod/src/pod.rs:622-636` (`commit_entry`) と `crates/pod/src/pod.rs:108-117` (`LogWriterHandle::append_entry`) は、`store.append` (kernel が直列化) と `sink.publish` (mirror mutex 内で直列化) を別ロックで行う。現状の Pod runtime では実質的に append は単一タスク (worker.run 駆動経路) に閉じているため race は発生しない。ただし将来「Pod の独立タスクから `LogWriterHandle::append_entry` を発火する」変更が入ると、kernel 順序と mirror 順序が一致しない可能性がある (disk に B が後着でも mirror の `subscribe_with_snapshot` プレフィックスでは A の前に B が並ぶ等)。ticket は「`O_APPEND` は kernel が atomic に直列化」までしか論証しておらず、mirror 側の不変条件は明示されていない。後続 `live-fork-marker` で marker entry 形式を確定する際に併せて「mirror の順序保証は append 経路の単一性に依存する」旨を pod 側 doc に書き残しておくと事故防止になる。
|
||||
- **`FsStore::read_entry_count` の O(n) 読み込み** — `fs_store.rs:121-122` は `fs::read_to_string` でファイル全体を読んで行数を数える。ターンの度に `ensure_head_or_fork` の中で呼ばれるため、長期セッション (数千 entry) では毎ターン無駄が出る。代替: ファイルサイズ + 末尾シーク + line counter、もしくは entry 数が膨らんできた段階で別途 metadata ファイル化。`live-fork-marker` チケットで marker entry を導入する際の見直しポイント。
|
||||
- **stale doc / test コメント** — 以下に旧用語 (`head_hash` / `SessionHead`) が残っており、ticket 完了後にも誤読を招く:
|
||||
- `crates/session-store/src/lib.rs:22` doc example の `let (session_id, head_hash) = create_session(...)` (API は既に `SessionId` 単独を返す)。
|
||||
- `crates/pod/tests/restore_test.rs:66` のコメント「`collect_state` returns `head_hash = None`」。
|
||||
- `crates/pod/tests/compact_events_test.rs:523-598` の `SessionHead` / `head_hash` を前提とした説明文。テスト自体は session_id 比較で正しく動くが、説明が現実装と乖離している。
|
||||
本チケット範囲を厳密に解釈すれば追従不要だが、近くを触る次のチケット (`segment-rename` 等) で同時に潰しておくのが望ましい。
|
||||
|
||||
### Nits
|
||||
|
||||
- `crates/pod/Cargo.toml:33` は `arc-swap = "1.9.1"` と patch まで pin されている。`manifest` 側は `"1"`。整合の必要はないが、workspace.dependencies に上げる選択肢もある (将来 1.x bump 時に一箇所で動く)。
|
||||
- `pod.rs:2148-2165` 圧縮経路で `let old_session_id = self.session_state.session_id();` の直後に `let w = self.worker.as_ref().unwrap();` を 2 回行っており (1 度目は turn_count 用、2 度目は entry 構築用)、まとめて 1 つの `let w = ...` で済む。可読性のみの話。
|
||||
|
||||
## 判断
|
||||
|
||||
**Approve** — チケットの要件・完了条件は全て充足され、削除と置換が clean に行われている。`session_state` の lock-free 化はチケットの論拠 (single-appender + `O_APPEND` 原子性) の範囲内では正しく、`ensure_head_or_fork` の entry-count 比較は hash 比較と検出力上等価 (writer の自前 tally と on-disk 行数の乖離 = 他プロセス侵入)。指摘した non-blocking 項目はいずれも本チケットのスコープ外、または後続 `live-fork-marker` / `segment-rename` のタイミングで吸収できる内容。
|
||||
Loading…
Reference in New Issue
Block a user