feat(storage): add config to disable alone-in-bucket query optimization#1406
feat(storage): add config to disable alone-in-bucket query optimization#1406sylr wants to merge 1 commit into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (3)
WalkthroughAdds a configurable kill switch that forces emitting ChangesLedger scoped select optimization kill switch
Sequence Diagram(s)sequenceDiagram
participant ServeCommand
participant storage.NewFXModule
participant driver.NewFXModule
participant LedgerStore
ServeCommand->>storage.NewFXModule: call NewFXModule(ModuleConfig{DisableScopedSelectOptimization: bool})
storage.NewFXModule->>driver.NewFXModule: forward driver.ModuleConfig{DisableScopedSelectOptimization: bool}
driver.NewFXModule->>LedgerStore: construct ledger store with ledgerstore.WithDisableScopedSelectOptimization(bool)
LedgerStore->>LedgerStore: newScopedSelect() uses disableScopedSelectOptimization to include/exclude "ledger = ?" predicate
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.12.2)level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies" 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. Comment |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1406 +/- ##
==========================================
- Coverage 80.55% 80.01% -0.55%
==========================================
Files 206 206
Lines 11293 11306 +13
==========================================
- Hits 9097 9046 -51
- Misses 1646 1659 +13
- Partials 550 601 +51 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
newScopedSelect skips the `ledger = ?` predicate when a ledger is the only one in its bucket, to avoid a degraded ~100%-selectivity seq scan. Add a --disable-ledger-scope-optimization serve flag that forces the predicate to always be emitted, as a performance/safety escape hatch. The config threads from the serve flag through storage.ModuleConfig and driver.ModuleConfig down to a new ledgerstore store option. Default keeps current behavior (optimization enabled). Constraint: disabling must only ever force the always-correct scoped query, never remove the predicate Rejected: gate on aloneInBucket hint alone | operators need a deterministic override independent of bucket population Confidence: high Scope-risk: narrow Directive: optimization is wired only into `serve`; worker/buckets-upgrade keep it on (correctness-neutral) Not-tested: end-to-end fx wiring of the flag (covered by unit-level SQL rendering + build)
edb0dbf to
1789924
Compare
Forward-port of
81a2cc6(originally landed onhotfix/v2.3/disable-ledger-predicate-optimization) tomain.What
newScopedSelectskips theledger = ?predicate when a ledger is the only one in its bucket, to avoid a degraded ~100%-selectivity seq scan. This adds a--disable-ledger-scope-optimizationserve flag that forces the predicate to always be emitted, as a performance/safety escape hatch. The config threads from the serve flag throughstorage.ModuleConfiganddriver.ModuleConfigdown to a newledgerstorestore option. Default keeps current behavior (optimization enabled).Forward-port notes
cmd/serve.goandinternal/storage/module.gohad conflicts: keptmain's newer content (audit flags,servicefx.ProvideHealthCheck) and layered the new flag/config on top.store_scoped_select_test.go: adapted to bunv1.2.18(main) —(*bun.DB).Formatter()was removed; switched the unit test to(*bun.DB).QueryGen(), which matchesSelectQuery.AppendQuery's newschema.QueryGensignature.Verification
go build ./cmd/... ./internal/storage/...✅go test ./internal/storage/ledger/ -run 'TestNewScopedSelect|TestWithDisableScopedSelectOptimization'✅