Skip to content

Add autosplit committed-window reader#1152

Merged
bootjp merged 2 commits into
mainfrom
feature/hotspot-m3-observe-reader
Jul 23, 2026
Merged

Add autosplit committed-window reader#1152
bootjp merged 2 commits into
mainfrom
feature/hotspot-m3-observe-reader

Conversation

@bootjp

@bootjp bootjp commented Jul 22, 2026

Copy link
Copy Markdown
Owner

Summary

  • add committed window boundaries to keyviz matrix columns
  • add an autosplit snapshot reader that converts committed keyviz columns into detector windows without calling SplitRange
  • document the M3-PR2b-a slice and leave Top-K, leadership watermarks, and scheduler wiring as remaining M3 work

Evidence

  • go test ./keyviz ./distribution/autosplit
  • go test ./adapter ./internal/admin -run 'KeyViz|Subrange|TestCommittedWindows|TestObserveSnapshot|TestFlushStampsWindowStart'\n- git diff --check\n\nAuthor: bootjp

Summary by CodeRabbit

  • 新機能
    • スナップショットから確定済みウィンドウを読み取り、観測専用の自動分割判定を実行できるようになりました。
    • ウィンドウ開始時刻を優先して処理し、不正なデータをスキップできるようになりました。
  • 改善
    • サンプラーがウィンドウの開始・終了時刻を正確に保持するようになりました。
    • スナップショットのコピー時にもウィンドウ境界情報が維持されます。
  • ドキュメント
    • 自動化マイルストーンの進捗と関連作業の整理を更新しました。
  • テスト
    • ウィンドウ境界、読み取り範囲、データ欠落時のリセット動作を検証するテストを追加しました。

@gemini-code-assist

Copy link
Copy Markdown
Contributor

Caution

The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased.

@github-actions

Copy link
Copy Markdown
Contributor

TLA+ spec divergence review (auto-triggered)

This PR touches files that the TLA+ safety spec has an anchor on (per
docs/design/2026_05_28_implemented_tla_safety_spec.md §3),
so an AI review is requested below to verify the implementation has not drifted
from the model.

Anchored files changed in this PR head (63ea6a7):

  • distribution/autosplit/detector.go
  • distribution/autosplit/sampler_reader.go
  • distribution/autosplit/sampler_reader_test.go

What to check, by subsystem:

  • kv/hlc*.goNext() must respect the HLC-4 preconditions (i)/(ii)/(iii) from the design doc: bounded skew, logical-counter handoff on leader change (strategy (c) Observe(MaxAppliedHLC)), and the commit-time ceiling fence (fail-closed when wall_now >= physicalCeiling). Any change to the bit layout (48/16), the CAS loop, or the ceiling getter/setter is in scope.
  • kv/coordinator.go, kv/sharded_coordinator.goRunHLCLeaseRenewal, hlcRenewalInterval, hlcPhysicalWindowMs constants, and the new-term detection that calls Observe(fsm.MaxAppliedHLC()) (strategy (c)). Any change to renewal cadence, group selection, or fail-closed behaviour is in scope.
  • kv/transaction.go, kv/lock_resolver.go — OCC commit-ts assignment, lock-map encoding (key, lock_ts) -> start_ts, and the LockResolver action OCC-3 depends on. (M2 spec will land OCC-1..OCC-5; until then the spec doc §5.2 is the contract.)
  • kv/fsm.go — FSM apply of HLC lease entries (SetPhysicalCeiling), and any future MaxAppliedHLC() accessor that strategy (c) needs.
  • store/mvcc_store.go — version visibility, snapshot install, and the MVCC-1..MVCC-4 invariants (M3 scope).
  • distribution/** — route catalog versioning, SplitRange atomicity, and CatalogWatcher async fan-out (M4 scope).

If the change is correct but requires a spec update, edit tla/hlc/HLC.tla (or the corresponding M2..M5 module once landed) and the design doc in the same PR. The tla-check workflow runs the TLC model check on the same paths.


@claude review please verify TLA+ spec divergence per the checklist above.

@codex review please verify TLA+ spec divergence per the checklist above.

@chatgpt-codex-connector

Copy link
Copy Markdown

To use Codex here, create a Codex account and connect to github.

@coderabbitai

coderabbitai Bot commented Jul 22, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@bootjp, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 11 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 47269cc2-1e3b-4c41-b370-9c3a30bc8232

📥 Commits

Reviewing files that changed from the base of the PR and between 63ea6a7 and c1eb6bb.

📒 Files selected for processing (4)
  • distribution/autosplit/sampler_reader.go
  • distribution/autosplit/sampler_reader_test.go
  • docs/design/2026_06_11_partial_hotspot_split_milestone3_automation.md
  • keyviz/sampler.go
📝 Walkthrough

Walkthrough

MatrixColumn にコミット区間の開始時刻を追加し、deep copy で保持するよう変更しました。スナップショットを ColumnWindow に変換して autosplit detector で評価する observe-only 読み取り経路と、その境界処理・リセット処理のテストを追加しました。

Changes

コミット済みウィンドウと observe-only 統合

Layer / File(s) Summary
MatrixColumn の境界保持と deep copy
keyviz/sampler.go, keyviz/ring_buffer.go, keyviz/sampler_test.go
MemSampler.FlushWindowStart を設定し、Snapshot の deep copy が境界情報を保持するよう変更しました。
スナップショットから detector への変換
distribution/autosplit/detector.go, distribution/autosplit/sampler_reader.go
スナップショット読み取り設定と結果型を追加し、WindowStart を優先した ColumnWindow 変換結果を Evaluate へ渡す経路を追加しました。
読み取り経路の検証と設計記録
distribution/autosplit/sampler_reader_test.go, docs/design/...
境界フォールバック、取得範囲、無効列のスキップ、サイレントギャップのリセット処理をテストし、Milestone 3 の PR 分割記録を更新しました。

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant ObserveSnapshot
  participant ReadCommittedWindows
  participant SnapshotSource
  participant CommittedWindowsFromColumns
  participant Evaluate
  ObserveSnapshot->>ReadCommittedWindows: SnapshotReadConfig を渡す
  ReadCommittedWindows->>SnapshotSource: Snapshot(from, now)
  SnapshotSource-->>ReadCommittedWindows: MatrixColumn スナップショット
  ReadCommittedWindows->>CommittedWindowsFromColumns: 列を ColumnWindow へ変換
  CommittedWindowsFromColumns-->>ReadCommittedWindows: Windows と newest を返す
  ObserveSnapshot->>Evaluate: Windows と Now を渡す
  Evaluate-->>ObserveSnapshot: Result を返す
Loading

Possibly related PRs

  • bootjp/elastickv#639: cloneColumnMatrixColumn.WindowStart を保持する変更に関連します。
  • bootjp/elastickv#1097: committed MatrixColumnColumnWindow に変換して detector の Evaluate へ渡す経路に関連します。
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed タイトルは主要変更であるautosplitのcommitted-window reader追加を簡潔に表しています。
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

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

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@docs/design/2026_06_11_partial_hotspot_split_milestone3_automation.md`:
- Line 12: Update the “Updated” date in the milestone 3 automation design
document to the actual modification date, replacing the future/review-following
date while preserving the document’s date format.

In `@keyviz/sampler.go`:
- Around line 1239-1243: Flush 全体を専用 mutex で直列化し、At の取得から history への push、および
lastFlushAt の更新までを同じロックで保護してください。対象は Flush の処理フローと、WindowStart を補正して col
を追加する箇所であり、並行実行時も列順序と実際の window 境界が一致する状態を維持してください。
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: fe18b72f-4498-4d5e-ace9-832fcc18f14d

📥 Commits

Reviewing files that changed from the base of the PR and between 5d4c5ca and 63ea6a7.

📒 Files selected for processing (7)
  • distribution/autosplit/detector.go
  • distribution/autosplit/sampler_reader.go
  • distribution/autosplit/sampler_reader_test.go
  • docs/design/2026_06_11_partial_hotspot_split_milestone3_automation.md
  • keyviz/ring_buffer.go
  • keyviz/sampler.go
  • keyviz/sampler_test.go

Comment thread docs/design/2026_06_11_partial_hotspot_split_milestone3_automation.md Outdated
Comment thread keyviz/sampler.go

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 63ea6a7f62

ℹ️ 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".

Comment thread distribution/autosplit/sampler_reader.go
Comment thread distribution/autosplit/sampler_reader.go
@bootjp

bootjp commented Jul 23, 2026

Copy link
Copy Markdown
Owner Author

@codex review

@github-actions

Copy link
Copy Markdown
Contributor

TLA+ spec divergence review (auto-triggered)

This PR touches files that the TLA+ safety spec has an anchor on (per
docs/design/2026_05_28_implemented_tla_safety_spec.md §3),
so an AI review is requested below to verify the implementation has not drifted
from the model.

Anchored files changed in this PR head (c1eb6bb):

  • distribution/autosplit/detector.go
  • distribution/autosplit/sampler_reader.go
  • distribution/autosplit/sampler_reader_test.go

What to check, by subsystem:

  • kv/hlc*.goNext() must respect the HLC-4 preconditions (i)/(ii)/(iii) from the design doc: bounded skew, logical-counter handoff on leader change (strategy (c) Observe(MaxAppliedHLC)), and the commit-time ceiling fence (fail-closed when wall_now >= physicalCeiling). Any change to the bit layout (48/16), the CAS loop, or the ceiling getter/setter is in scope.
  • kv/coordinator.go, kv/sharded_coordinator.goRunHLCLeaseRenewal, hlcRenewalInterval, hlcPhysicalWindowMs constants, and the new-term detection that calls Observe(fsm.MaxAppliedHLC()) (strategy (c)). Any change to renewal cadence, group selection, or fail-closed behaviour is in scope.
  • kv/transaction.go, kv/lock_resolver.go — OCC commit-ts assignment, lock-map encoding (key, lock_ts) -> start_ts, and the LockResolver action OCC-3 depends on. (M2 spec will land OCC-1..OCC-5; until then the spec doc §5.2 is the contract.)
  • kv/fsm.go — FSM apply of HLC lease entries (SetPhysicalCeiling), and any future MaxAppliedHLC() accessor that strategy (c) needs.
  • store/mvcc_store.go — version visibility, snapshot install, and the MVCC-1..MVCC-4 invariants (M3 scope).
  • distribution/** — route catalog versioning, SplitRange atomicity, and CatalogWatcher async fan-out (M4 scope).

If the change is correct but requires a spec update, edit tla/hlc/HLC.tla (or the corresponding M2..M5 module once landed) and the design doc in the same PR. The tla-check workflow runs the TLC model check on the same paths.


@claude review please verify TLA+ spec divergence per the checklist above.

@codex review please verify TLA+ spec divergence per the checklist above.

@chatgpt-codex-connector

Copy link
Copy Markdown

To use Codex here, create a Codex account and connect to github.

@chatgpt-codex-connector

Copy link
Copy Markdown

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

Reviewed commit: c1eb6bb506

ℹ️ 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".

@bootjp
bootjp merged commit afec059 into main Jul 23, 2026
11 checks passed
@bootjp
bootjp deleted the feature/hotspot-m3-observe-reader branch July 23, 2026 05:42
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.

1 participant