ethdb, triedb: tune pebble write path and add safe pathdb state carry-over#2170
ethdb, triedb: tune pebble write path and add safe pathdb state carry-over#2170marcello33 merged 4 commits intodevelopfrom
Conversation
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.
Tip: disable this comment in your organization's Code Review settings.
|
@claude review |
|
Claude finished @marcello33's task in 1m 58s —— View job Code Review: PebbleDB Optimizations & Pathdb State Carry-Over
Overall this is a well-structured PR with solid test coverage for the new carry-over logic. The copy-on-write approach is correctly implemented and the async flush isolation is properly handled. Below are my findings. Pebble Tuning (
|
| Area | Verdict |
|---|---|
| Pebble tuning | Good — reasonable, well-commented changes |
| Buffer split logic | Good — correct arithmetic, clean API |
| COW state carry-over | Good — correctly implemented with proper isolation |
| Async flush safety | Good — snapshot capture + COW prevents races |
| Revert path | Good — carried states correctly cleared |
| Test coverage | Excellent — covers all critical paths including async isolation |
| Config wiring | Good — StateReservation threaded through generate, journal, tests |
No CRITICAL or HIGH issues found. The PR is well-designed and thoroughly tested. The COW carry-over is the most complex part and it's correctly implemented — the flush() snapshot capture pattern combined with copy() ensures the frozen buffer remains immutable while the live buffer evolves independently.
|
There was a problem hiding this comment.
Pull request overview
This PR tunes the PebbleDB write path and refactors PathDB’s write buffer to reduce write stalls while safely reusing dirty flat-state across flushes (via copy-on-write state snapshots).
Changes:
- Pebble: adjust sync/flush/compaction knobs (
BytesPerSync,MemTableStopWritesThreshold, adaptive compaction concurrency settings). - PathDB: split write-buffer budgeting between trie nodes and flat state, flush on node-pressure or hard total limit, and carry over state only when safe/within budget.
- Add extensive unit tests covering buffer budgeting, carry-over semantics, and copy-on-write isolation.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
ethdb/pebble/pebble.go |
Pebble option tuning for sync cadence, write-stall reduction, and adaptive compaction. |
triedb/pathdb/config.go |
Introduces StateReservation config and raises maximum allowed buffer size. |
triedb/pathdb/buffer.go |
Adds node/state budgeting, new flush trigger logic, and carry-over eligibility helper. |
triedb/pathdb/disklayer.go |
Implements safe state carry-over on node-pressure flushes; clears carried state on revert with no transitions. |
triedb/pathdb/states.go |
Adds copy-on-write stateSet.copy() and detaching logic for shared per-account storage submaps. |
triedb/pathdb/buffer_test.go |
New tests for buffer split, flush triggers, carry-over behavior, and async-flush isolation. |
triedb/pathdb/states_test.go |
New tests validating copy-on-write behavior for storage submaps and revert detaching. |
triedb/pathdb/journal.go |
Wires StateReservation into buffer construction during journal loading. |
triedb/pathdb/generate.go |
Wires StateReservation into buffer construction during snapshot generation. |
triedb/pathdb/difflayer_test.go |
Updates buffer construction to new signature. |
triedb/pathdb/layertree_test.go |
Updates buffer construction to new signature. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report❌ Patch coverage is
❌ Your patch status has failed because the patch coverage (85.84%) is below the target coverage (90.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## develop #2170 +/- ##
===========================================
+ Coverage 51.88% 51.92% +0.03%
===========================================
Files 884 884
Lines 155324 155412 +88
===========================================
+ Hits 80593 80693 +100
+ Misses 69522 69512 -10
+ Partials 5209 5207 -2
... and 20 files with indirect coverage changes
🚀 New features to boost your workflow:
|



Description
This PR implements some PebbleDB optimizations.
Others are on a config level, and will be managed in pos-ops.
The suggested configs are (tested on a 128GB RAM machine)
The PR combines write-path tuning with pathdb buffer changes aimed at reducing write stalls and improving dirty-state reuse across flushes.
On the Pebble side, it tunes sync and compaction behavior. On the pathdb side, it splits the write buffer into node/state budgets, allows state carry-over when flushes are caused by trie-node pressure, and keeps that carry-over safe under async flushing via copy-on-write state snapshots. The copy-on-write carry-over means:
This keeps the frozen flush snapshot immutable while still avoiding eager duplication of all storage submaps.
Pebble tuning:
BytesPerSyncfrom the implicit default to1 MiBMemTableStopWritesThresholdto reduce write stalls during flushesL0CompactionConcurrency = 1CompactionDebtConcurrency = 256 MiBPathdb buffer changes:
maxBufferSizeto2 GiBStateReservationto split the write buffer between:Changes
Checklist
Testing