Skip to content

Add Pebble SST ingest snapshot transfer - #1130

Merged
bootjp merged 3 commits into
mainfrom
feature/pebble-sst-ingest-snapshot
Jul 19, 2026
Merged

Add Pebble SST ingest snapshot transfer#1130
bootjp merged 3 commits into
mainfrom
feature/pebble-sst-ingest-snapshot

Conversation

@bootjp

Copy link
Copy Markdown
Owner

Summary

  • add an opt-in checkpoint-derived Pebble SST snapshot stream with a versioned integrity manifest
  • restore verified SST files through DB.Ingest into a temporary database with atomic swap, rollback, and crash recovery
  • retain legacy snapshot emission by default and fall back before streaming when SST preparation fails
  • cover KV FSM and disk-spooled gRPC snapshot transport integration
  • mark the focused design implemented and keep physical object offload out of scope

Safety and rollout

Set ELASTICKV_PEBBLE_SST_INGEST_SNAPSHOT=true only after every member runs a receiver-capable binary. Receivers always accept the new format. Invalid or unset values preserve legacy emission. Manifest, file bounds, exact lengths, SHA-256 digests, and EOF are verified before the live database is replaced.

The shared restore swap now retains the old database until the replacement opens and metadata verifies. Startup recovers a single interrupted rollback directory and preserves ambiguous backups instead of deleting them.

Tests

  • go test ./store ./kv ./internal/raftengine/etcd -count=1 -timeout=20m
  • go test -race ./store ./kv ./internal/raftengine/etcd -run 'TestPebbleStoreSST|TestSSTIngest|TestCountingWriter|TestSwapInTempDB|TestKVFSMSnapshotRoundTripsSSTIngestPayload|TestGRPCSnapshotTransportRoundTripsSSTIngestPayload|TestPebbleStore_SnapshotRestore|TestPebbleStore_Restore' -count=1 -timeout=15m\n- go test ./adapter -count=1 -timeout=20m\n- golangci-lint --config=.golangci.yaml run --fix

@coderabbitai

coderabbitaiBot commented Jul 19, 2026

Copy link
Copy Markdown

Warning

Review limit reached

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

Next review available in:34 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

Run ID: 9e97baa5-4303-496d-b18d-3f03a24b667f

📥 Commits

Reviewing files that changed from the base of the PR and between 915bc77 and 214a8cc.

📒 Files selected for processing (8)
  • docs/design/2026_06_12_proposed_scaling_roadmap.md
  • docs/design/2026_07_19_implemented_pebble_sst_ingest_snapshot_transfer.md
  • internal/raftengine/etcd/grpc_transport_sst_snapshot_test.go
  • kv/snapshot_sst_ingest_test.go
  • store/lsm_store.go
  • store/snapshot_pebble.go
  • store/snapshot_pebble_sst.go
  • store/snapshot_pebble_sst_test.go

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.

@bootjp

Copy link
Copy Markdown
OwnerAuthor

Implementation and caller audit are complete on current HEAD 10491972bf234ed2253e18fef78a05d4699e7cd0.

Local evidence:

  • go test ./store ./kv ./internal/raftengine/etcd -count=1 -timeout=20m
  • focused go test -race across the SST, rollback, KV FSM, and gRPC transport paths
  • go test ./adapter -count=1 -timeout=20m
  • repository commit hook: golangci-lint with 0 issues
  • old proposed design path references: none

Please review checkpoint consistency, manifest validation, atomic rollback/crash recovery, legacy fallback, and the audited native/MVCC/SST swap callers.

@codex review

@gemini-code-assistgemini-code-assistBot 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.

Code Review

This pull request implements Pebble SST Ingest Snapshot Transfer, introducing a new snapshot format (EKVSSTI1) that exports external SST files from a point-in-time checkpoint and ingests them atomically on the receiver side with rollback support. Feedback on these changes focuses on enhancing resource safety and robustness. Key recommendations include using defer (and wrapping loop bodies in anonymous functions) to prevent file descriptor leaks in the event of panics during file operations, avoiding global mutable state in tests to prevent data races, and replacing filepath.Glob with os.ReadDir to handle directory paths containing glob characters safely.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment threadstore/snapshot_pebble_sst.go Outdated
Comment threadstore/snapshot_pebble_sst.go
Comment threadstore/snapshot_pebble_sst.go
Comment threadstore/snapshot_pebble_sst.go Outdated
Comment threadstore/snapshot_pebble_sst.go
@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. You're on a roll.

Reviewed commit:10491972bf

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

Copy link
Copy Markdown
OwnerAuthor

Addressed and resolved all five current-head Gemini threads in signed commit 214a8ccf027a9190d7a5ec467f27a6ae3658c097:

  • deferred, error-preserving file cleanup for inspect, stream, and receive paths
  • instance-local SST target size with no mutable package test state
  • literal os.ReadDir artifact discovery with a glob-character path regression

Verification on the new HEAD:

  • go test ./store ./kv ./internal/raftengine/etcd -count=1 -timeout=20m
  • focused go test -race across SST, rollback, KV FSM, and transport paths
  • changed-package lint: 0 issues
  • repository commit-hook lint: 0 issues

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. 🚀

Reviewed commit:214a8ccf02

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

Copy link
Copy Markdown
OwnerAuthor

Review-loop stop recorded for current HEAD 214a8ccf027a9190d7a5ec467f27a6ae3658c097:

  • current-head review: no major issues
  • current-head inline roots: none (all earlier Gemini roots are stale by original_commit_id and resolved)
  • issue-style findings: none on the current head
  • CI: all completed checks successful or intentionally skipped
  • mergeability: mergeable

The PR remains open and was not merged.

@bootjp
bootjp merged commit 910a97e into mainJul 19, 2026
19 checks passed
@bootjp
bootjp deleted the feature/pebble-sst-ingest-snapshot branch July 19, 2026 13:22
bootjp added a commit that referenced this pull request Jul 19, 2026
## Summary
- add the focused physical snapshot object-offload design and link it
from the storage roadmap
- expose the newest WAL-valid persisted FSM snapshot as a single-use
stream paired with Raft index, term, ConfState, byte count, and CRC32C
- add complete-payload restore preparation that preserves the existing
FSM header and leaves store-format dispatch to the receiver
- preserve the existing logical external-restore header behavior through
focused regression coverage
## Dependency and scope
PR #1130 owns the Pebble SST ingest snapshot stream. This pull request
treats the complete FSM payload as opaque and does not parse or
duplicate that format.
Because #1130 is still in review, this pull request intentionally
contains only the independently reviewable export/restore substrate.
Object-store publication, runtime scheduling, retention/GC, and operator
restore wiring remain later milestones in the focused design.
## Safety
The exporter reads only a WAL-valid persisted Raft snapshot. Token
index, metadata index, footer CRC32C, and the CRC32C recomputed during
streaming must agree. The source file descriptor remains open for the
whole stream, so local retention cannot replace the exported inode
mid-upload.
Physical restore targets an absent data directory and copies a verified
regular file through the existing atomic restore preparation. The
existing logical restore API and its synthetic KV header are unchanged.
## Tests
- `go test ./internal/raftengine/etcd -count=1 -timeout=20m`
- `go test -race ./internal/raftengine/etcd -run
'TestPreparePhysicalSnapshotRestoreAndExportOpaquePayload|TestPersistedSnapshotExport|TestOpenPersistedSnapshotExport'
-count=1 -timeout=10m`
- `go test ./... -run '^$' -count=1 -timeout=20m`
- `golangci-lint run ./internal/raftengine/etcd/... --timeout=10m`
- commit hook: full `golangci-lint`, 0 issues
- `git diff --check`
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
* **新機能**
* PebbleのSST取り込み方式によるスナップショットの作成・転送・復元に対応しました。
* 復元前後の整合性検証を強化し、失敗時には既存データを保持します。
* 物理スナップショットのエクスポートと復元に対応しました。
* SST方式が利用できない場合は従来方式へ自動的に切り替えます。
* **ドキュメント**
* SSTスナップショット転送および外部オブジェクトストレージへのオフロード設計を追加しました。
* **バグ修正**
* 不完全な書き込みを正しく検出するよう改善しました。
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
bootjp added a commit that referenced this pull request Aug 23, 2026
This document declares itself the canonical ownership index, so rows that
call finished work unimplemented and unowned send contributors off to
write a competing design. Seven rows named work that is already an
ancestor of this commit:
- route catalog delta watch (#1117, 6c7a66e) -> implemented owner
- SST ingest snapshot transfer (#1130, 910a97e) -> implemented owner
- leader-proxy circuit breaker (#1132, 56e36e9) -> implemented owner
- physical snapshot offload (#1131, 764db2d) -> partial owner, which
records M0/M1 implemented and still owns M2/M3
- TSO ceiling state machine (#1150, 0e85c82) -> merged, separated from
the group-0 issuance work that remains open
- autosplit committed-window reader (#1152, afec059) -> M3 is partially
implemented, not absent
- S3 offload transport and backfill (77ea547, #1126/5eaaa05d) -> the
transport path is merged; the focused design names reference counting,
GC readiness, and legacy migration as the remaining blockers
Each cited commit was confirmed to be an ancestor of this revision, and
every referenced design document was confirmed to exist.
The completion rule also could not be satisfied by every row: it demanded
each implemented row's focused owner be promoted to `implemented`, but
the shared Pebble cache row names only PR #1082 as its canonical owner
and a pull request has no status to promote. That made the roadmap
permanently ineligible for promotion regardless of what shipped. Merged
implementation evidence now closes such a row; an open pull request still
does not.
Claude-Session: https://claude.ai/code/session_013rNHooj7NF3giihWVba8QE
Sign up for freeto 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

@bootjp