baa7748e - Add a hard-coded master switch to disable all ledger cron jobs - #4521
Merged
Conversation
…on jobs pg_stat_activity sampling on 2026-07-30 (~70ms interval, two independent runs over 180s/60s, own backend PID excluded) showed the ledger's asset- price projection alone accounting for 29.6% / 32.4% of all active production DB queries. Root cause: nine booking consumers run every minute and each preload a two-day price window by jsonb_each-ing a ~57 kB JSON blob out of a text column - roughly a third of total production DB time for a downstream process nobody currently reads from. Config.ledger.enabled is a hard-coded, default-off switch (deliberately not process.env/DB/setting-backed, and not an addition to the generic DISABLED_PROCESSES list) that every one of the 13 @DfxCron ledger entry points now respects before their first database read: - The ten booking consumers and the CoA-bootstrap cron (all in LedgerBookingJobService) and LedgerCutoverService.run() check it directly, first thing, no DB call before it. - LedgerMarkToMarketService.run() and LedgerReconciliationService.run() check it transitively: both already call jobService.isLedgerReady() as their very first statement, and that method now short-circuits on the switch before it ever reaches settingService.get. No line in either file mentions Config.ledger.enabled, which looks like a gap on a plain grep - it is the intended design, not an oversight. The cutover cron keeps its own already-cut-over check as a second, separately-meaning guard (a different question than "is the ledger enabled at all") rather than folding the two together. A new completeness test (ledger-master-switch.spec.ts) derives the set of ledger cron entry points from AccountingModule's own provider metadata plus each method's @DfxCron process flag, instead of listing the 13 methods by name - so it also catches the two transitively-gated services above, and will catch a future ledger cron job that forgets to check the switch at all.
… harden completeness guard Two integration suites drive the REAL LedgerCutoverService.run() (or a sibling that reaches it) to exercise cutover semantics, not the new master switch: staleness-cutover.integration.spec.ts's "Cutover idempotency" describe (no Config setup of its own, so it inherited whatever a preceding describe in the same file last left Config at) and crypto-input-cutover.integration.spec.ts (new ConfigService() with no override, so ledger.enabled defaulted to its new off-by-default value). With the switch off by default, run() now no-ops immediately, so both suites asserted on bookings that never happened. Both now explicitly set Config.ledger.enabled = true where they drive the real cutover, independent of whichever other describe/file ran before them. Also strengthens the ledger-master-switch completeness test: its core assertion only proved zero calls against mocked dependencies, which a future ledger cron WITHOUT any gate could satisfy by coincidence (e.g. an unrelated early return under createMock()'s defaults) and stay green without having checked anything. It now redefines Config.ledger.enabled as an accessor for the duration of each run and asserts it was actually read - proof the gate was consulted, whether directly (the cutover cron) or transitively (isLedgerReady(), which reads it internally) - rather than merely observing silence. The remaining blind spot (a ledger cron registered outside AccountingModule, or without a Process.LEDGER_* flag at all) is documented in a comment as an accepted limitation shared with the existing per-process kill-switches, which depend on the same flag.
The test replaces Config.ledger.enabled with a recording getter to prove the gate is actually read, but never put the original descriptor back. describe.each runs this thirteen times against the same shared Config.ledger instance, so each iteration started on state the previous one had prepared. Not exploitable as it stands - Jest gives each spec file its own module registry, so the getter cannot reach another file - which is why this is hygiene rather than a fix. try/finally instead of afterEach keeps the teardown next to the override, and it now also runs when an assertion fails or the cron method throws.
Collaborator
Author
|
Two completed review passes on this branch (correctness and conformance). Findings acted on:
The hard-coded switch was checked against A further pass over the final commit is still running. Anything it turns up will be handled in a |
TaprootFreak
marked this pull request as ready for review
July 30, 2026 19:59
This was referenced Jul 30, 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
1 participant
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.
The ledger jobs are the single largest consumer of production database time — for a secondary process that is not currently consumed downstream. This adds one switch to turn all of them off until that cost is understood.
Measured
Sampling of
pg_stat_activityin production, two independent runs (180 s and 60 s, own backend pid excluded):Attributing the consumer queries as well —
LiquidityOrder,BuyCrypto,PayoutOrder,TradingOrder,BuyFiat,CryptoInput, all tables the booking consumers read — puts the ledger at roughly two thirds of sampled database activity. That wider figure is an attribution by table name, not a proof; the sampling after this switch is deployed will settle it.Root cause: nine booking consumers run every minute, and each calls
LedgerMarkService.preload()for a two-day window. That projection takes theassetsblock (~57 kB) apart withjsonb_each— out of atextcolumn, so every row is parsed in full.What this adds
A hard-coded switch in
config.ts, default off. Deliberately not an environment variable, not a database setting, and not another entry in the genericDISABLED_PROCESSESlist — it is meant to be visible in the code, with the measurement and the reason next to it, so nobody flips it back on without re-checking.All 13 ledger cron entry points bail out before their first database query:
ledger-booking-job.service.ts, via the sharedisLedgerReady()gateledger-cutover.service.ts, checked directlyledger-mark-to-marketandledger-reconciliationcallisLedgerReady()as the first line of theirrun()methodThat last point is worth stating explicitly: grepping those two files for the switch finds nothing, which invites the false conclusion that they are uncovered. They are not.
The part that matters long-term
A test discovers every ledger cron entry from
AccountingModule's own provider metadata — every@DfxCronwhoseprocessstarts withLEDGER— rather than from a hand-written list. It asserts it found at least 13 (so an undersized discovery cannot pass vacuously) and that each one issues zero calls against any dependency while the switch is off.A ledger job added later is therefore covered automatically, or the test fails.
Turning it back on
Re-run the same
pg_stat_activitysampling afterwards and compare. The measurement is the point of the switch, not the switch itself.