Skip to content

Conversation

@flemzord
Copy link
Member

@flemzord flemzord commented Feb 6, 2026

Summary

  • scope ledger reads with a cached isAloneInBucket hint set at store open/create time
  • add CountLedgersInBucket to system store and wire it in driver create/open flows
  • document the cache invalidation constraint to prevent cross-ledger data exposure when caching stores

Validation

  • go test ./internal/storage/ledger ./internal/storage/driver

@flemzord flemzord requested a review from a team as a code owner February 6, 2026 10:30
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 6, 2026

Caution

Review failed

The pull request is closed.

Walkthrough

Driver now queries the system store for the number of ledgers in a bucket when creating or opening a ledger and sets a shared per-bucket AloneInBucket flag on the ledger store; ledger query logic uses this flag to conditionally skip ledger-name filtering.

Changes

Cohort / File(s) Summary
Driver changes
internal/storage/driver/driver.go
Calls CountLedgersInBucket when creating/opening ledgers, wraps counting errors, and sets the store's AloneInBucket flag based on whether the bucket count equals 1.
System store API & impl
internal/storage/system/store.go
Added CountLedgersInBucket(ctx context.Context, bucket string) (int, error) to the Store interface and implemented it in DefaultStore (SELECT COUNT on _system.ledgers).
Mocks / tests
internal/storage/driver/system_generated_test.go
Extended mock SystemStore with CountLedgersInBucket and its recorder to support expectations in tests.
Ledger store optimization
internal/storage/ledger/store.go
Added per-bucket shared aloneInBucket *atomic.Bool field and SetAloneInBucket(bool); modified scoped select logic to skip ledger-name filtering when the flag is true.
Factory bucket flags
internal/storage/ledger/factory.go
Added mu sync.Mutex and bucketFlags map[string]*atomic.Bool to DefaultFactory; allocates/assigns per-bucket atomic flags to stores so flag is shared across a bucket.

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant Driver
    participant SystemStore
    participant LedgerStore

    Client->>Driver: CreateLedger/OpenLedger(bucket, name)
    Driver->>SystemStore: CountLedgersInBucket(ctx, bucket)
    SystemStore->>SystemStore: Execute COUNT query on _system.ledgers
    SystemStore-->>Driver: return count (n) or error
    Driver->>LedgerStore: SetAloneInBucket(n == 1)
    LedgerStore->>LedgerStore: update shared per-bucket atomic flag
    Driver-->>Client: return ledger store handle
    Note over LedgerStore: Subsequent queries use the flag to skip<br/>ledger-name filtering when true
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐰 I counted buckets in the night,
One ledger stood alone in light.
A shared small flag I set with care,
Now queries breeze through thinner air.
Hop, skip, optimize — a rabbit's cheer! 🥕✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: adding a bucket hint to scope ledger queries, which is the core optimization introduced across the modified files.
Description check ✅ Passed The pull request description clearly relates to the changeset, explaining the addition of a cached isAloneInBucket hint and CountLedgersInBucket method across multiple files.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch flemzord/fresh-start

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

flemzord added a commit that referenced this pull request Feb 6, 2026
…coping

The per-query sub-select on _system.ledgers to determine if a ledger is
alone in its bucket is replaced by a boolean flag set at store open/create
time. This avoids repeated cross-schema queries and prepares the ground
for safe store caching (the flag must be invalidated when bucket
composition changes).

Backport of #1250.
flemzord added a commit that referenced this pull request Feb 6, 2026
Replace the dynamic _system.ledgers subquery in newScopedSelect() with a
pre-computed isAloneInBucket boolean flag. The flag is set at store
creation/opening time via CountLedgersInBucket, avoiding degraded
PostgreSQL query plans when a ledger is alone in its bucket.

Backport of #1250.
…Factory

The previous per-store bool meant that when a second ledger was created
in the same bucket, existing stores kept isAloneInBucket=true and
skipped the WHERE ledger=? predicate, causing cross-ledger data leaks
in queries (e.g. TestAccountsList).

Replace the private bool with a *atomic.Bool shared across all stores
of the same bucket through the DefaultFactory. Any call to
SetAloneInBucket now immediately propagates to every store in that
bucket.
@flemzord
Copy link
Member Author

flemzord commented Feb 6, 2026

Superseded by a new PR targeting release/v2.2

@flemzord flemzord closed this Feb 6, 2026
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