Skip to content

Conversation

@flemzord
Copy link
Member

@flemzord flemzord commented Feb 6, 2026

Summary

  • Replace the per-store isAloneInBucket bool (from the subquery-based approach on v2.2) with a shared *atomic.Bool per bucket, managed by the DefaultFactory
  • When a second ledger is created in the same bucket, SetAloneInBucket(false) now propagates immediately to all existing stores in that bucket
  • Add CountLedgersInBucket to the system store interface so CreateLedger and OpenLedger can seed the flag correctly

Root cause

The previous approach (subquery in newScopedSelect on v2.2, or cached bool per store on main) could miss bucket membership changes: when multiple ledgers share a bucket, a store created before the second ledger still believed it was alone, skipping the WHERE ledger = ? predicate and leaking rows from other ledgers (e.g. TestAccountsList failures).

Changes

  • internal/storage/ledger/store.go: aloneInBucket *atomic.Bool field + SetAloneInBucket / newScopedSelect using atomic load
  • internal/storage/ledger/factory.go: bucketFlags map[string]*atomic.Bool shared across all stores of the same bucket
  • internal/storage/driver/driver.go: CreateLedger and OpenLedger call CountLedgersInBucket and SetAloneInBucket
  • internal/storage/system/store.go: new CountLedgersInBucket method on interface + implementation
  • internal/storage/driver/system_generated_test.go: mock updated for new method

Test plan

  • earthly -P +tests passes (all unit, integration, e2e, stress tests)
  • TestAccountsList no longer flaky with multiple ledgers in the same bucket

…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 flemzord requested a review from a team as a code owner February 6, 2026 12:45
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 6, 2026

Walkthrough

Changes introduce a per-bucket optimization flag (AloneInBucket) to track whether a ledger is the sole ledger in its bucket. The flag is managed through the factory with synchronized state, set during ledger creation and opening via counting operations, and affects query scoping logic for performance optimization.

Changes

Cohort / File(s) Summary
Driver Ledger Operations
internal/storage/driver/driver.go
Modified CreateLedger and OpenLedger to count ledgers in the target bucket and set the AloneInBucket flag based on count equality to 1. Includes error handling for count failures.
Ledger Store and Query Logic
internal/storage/ledger/store.go
Added aloneInBucket atomic flag field to Store struct, introduced SetAloneInBucket() setter method, and updated newScopedSelect to conditionally apply ledger filters based on the flag state.
Factory Per-Bucket State Management
internal/storage/ledger/factory.go
Added mutex and bucketFlags map to DefaultFactory for thread-safe per-bucket atomic flag management. Create() now retrieves or initializes per-bucket flags and assigns them to produced Store instances.
System Store Ledger Counting
internal/storage/system/store.go
Added CountLedgersInBucket() method to Store interface and DefaultStore implementation to count ledger records matching a specified bucket using SQL Count.
Mock Support
internal/storage/driver/system_generated_test.go
Added CountLedgersInBucket() mock methods to SystemStore and SystemStoreMockRecorder for test support.

Sequence Diagram(s)

sequenceDiagram
    participant Driver
    participant SystemStore
    participant Factory
    participant Store
    
    Driver->>SystemStore: CountLedgersInBucket(ctx, bucket)
    SystemStore-->>Driver: count
    
    alt count == 1
        Driver->>Store: SetAloneInBucket(true)
        Store->>Factory: (via aloneInBucket flag)
        Factory->>Store: propagates flag to bucket stores
    end
    
    Store->>Store: newScopedSelect() uses flag
    Store-->>Driver: optimized query (conditional filter)
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐰 A flag now marks each bucket's claim,
When one ledger stands alone—no shame!
The factory guards with locks so tight,
Queries skip filters, swift and light.
Per-bucket wisdom, shared just so,
Efficiency blooms as buckets grow! 🌱

🚥 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 and specifically describes the primary change: introducing a shared atomic.Bool flag per bucket to replace per-store state management.
Description check ✅ Passed The description is directly related to the changeset, providing clear context about the root cause, changes made across multiple files, and expected test outcomes.

✏️ 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 fix/v2.2/shared-alone-in-bucket-flag

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

@flemzord flemzord merged commit 7be5a96 into release/v2.2 Feb 6, 2026
7 checks passed
@flemzord flemzord deleted the fix/v2.2/shared-alone-in-bucket-flag branch February 6, 2026 13:04
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.

2 participants