refactor(fibre): replace go-ds-pebble wrapper with direct Pebble API#7062
Merged
refactor(fibre): replace go-ds-pebble wrapper with direct Pebble API#7062
Conversation
CPU profiling showed the go-ds-pebble wrapper adds ~45% overhead to every store write due to interface indirection through ds.Batching, key conversion from ds.Key to bytes, and extra allocations. The store only uses basic operations (Put, Get, prefix scan, batch delete) that map directly to Pebble's native API. - Store struct: ds.Batching -> *pebbledb.DB - NewMemoryStore: pebbledb.Open with vfs.NewMem() - NewBadgerStore: delegates to NewPebbleStore (kept for test compat) - NewPebbleStore: pebbledb.Open directly (all tuning options preserved) - Put: db.NewBatch + batch.Set + batch.Commit - Get: prefix scan via pebbledb.NewIter with LowerBound/UpperBound - GetPaymentPromise: db.Get with closer pattern - PruneBefore: iterator + batch delete - Key functions return []byte instead of ds.Key - Added prefixUpperBound helper for prefix scans - Removed imports: go-datastore, go-datastore/query, go-datastore/sync, go-ds-badger4, go-ds-pebble Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
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.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Member
To confirm, the impact was measured without value separation? |
evan-forbes
previously approved these changes
Apr 13, 2026
Member
evan-forbes
left a comment
evan-forbes
left a comment
There was a problem hiding this comment.
besides the linter failures seems gud
Comment on lines
-238
to
+216
| func (s *Store) PruneBefore(ctx context.Context, before time.Time) (int, error) { | ||
| results, err := s.ds.Query(ctx, query.Query{ | ||
| Prefix: "/prune/", | ||
| KeysOnly: true, | ||
| Orders: []query.Order{query.OrderByKey{}}, | ||
| func (s *Store) PruneBefore(_ context.Context, before time.Time) (int, error) { |
Member
There was a problem hiding this comment.
I don't have a strong opinion here but do we want to permanently remove the context?
Wondertan
reviewed
Apr 14, 2026
Add missing iter.Error() checks after Pebble iterator loops in Get and PruneBefore to avoid silently converting storage errors into ErrStoreNotFound or partial prunes. Replace []byte(fmt.Sprintf(...)) with fmt.Appendf(nil, ...) to satisfy the modernize linter. Remove stale NewBadgerStore wrapper and unused ipfs dependencies. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Wondertan
approved these changes
Apr 14, 2026
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
3 participants
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.
Summary
Replaces the
go-ds-pebble/go-datastoreabstraction layer with direct Pebble v2 API calls.Motivation
The
go-datastoreinterface adds overhead through interface indirection, key conversion (ds.Key→ bytes), and extra allocations. Local benchmarks comparing wrapper vs direct Pebble on the same workload:Read path (prefix scan for shard retrieval):
go-datastoreQuery interface builds result channels and copies values into generic result structsWrite path (batch write of promise + shard + prune key):
The store only uses basic operations (batch write, point lookup, prefix scan, range delete) that map directly to Pebble's native API. The abstraction layer provides no benefit since there is no backend swapping.
Changes
go-datastore,go-ds-pebble,go-ds-badger4ds.Batching→*pebbledb.DBBatch.Set,DB.Get,DB.NewIter, etc.)NewBadgerStoreremoved (was unused alias)NewMemoryStoreuses in-memory Pebble viavfs.NewMem()iter.Error()checks after iterator loopsTest plan
./fibre/...test suite passesCloses https://linear.app/celestia/issue/PROTOCO-1495
🤖 Generated with Claude Code