d393cad0 - Aggregate the finance change log in SQL instead of loading the month's entities - #4535
Draft
TaprootFreak wants to merge 1 commit into
Draft
d393cad0 - Aggregate the finance change log in SQL instead of loading the month's entities#4535TaprootFreak wants to merge 1 commit into
TaprootFreak wants to merge 1 commit into
Conversation
…s entities getChangeLog runs every minute and loaded every buy-crypto, buy-fiat, exchange and payout row of the current month as fully hydrated entities, only to reduce them to a few fee sums. Eager relations expand a single buy-crypto row to 481 columns across 15 LEFT JOINs: 3225 rows amount to 9 MB and ~1.55 M field values per run. The database answers that in 58 ms; deserialising it in the pg driver took up to 7926 ms and kept the event loop saturated. Replace the four loaders with SQL aggregates that return only the totals. Semantics are preserved: cryptoInput stays LEFT-joined so rows without one keep counting as regular fees, the payout fee COALESCEs both columns the way the entity getter relied on JS null coercion, and an empty period maps NULL to 0. Cover the aggregates with pg-mem suites, since a mocked repository executes no SQL.
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
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.
Problem
LogJobService.saveTradingLog()runs every minute. Inside it,getChangeLog()loaded everybuy-crypto, buy-fiat, exchange and payout row of the current month as fully hydrated entities —
only to reduce them to a handful of fee sums.
Because of TypeORM eager relations, one buy-crypto row expands to 481 columns across 15 LEFT
JOINs. Measured against production data on the 30th of the month:
The gap is not query time — it is the Node process deserialising rows. A CPU profile of the running
API attributed 36.1 % of wall time to the garbage collector and 17.9 % to
parseRowin thepgdriver, with application code below 1 %. Event-loop utilisation sat at a median of 96 % with ap90 delay above 1 s, which is what made unrelated endpoints (asset and fiat lists,
buy/paymentInfos)take hundreds of milliseconds.
Change
The four sources now aggregate in SQL and return only the totals:
BuyCryptoService.getBuyCrypto(from, relations)getBuyCryptoFee(from)→{ regular, paymentLink }BuyFiatService.getBuyFiat(from, relations)getBuyFiatFee(from)→{ regular, paymentLink }ExchangeTxService.getExchangeTx(from)getExchangeTxFee(from)→ one row per exchange/typePayoutService.getPayoutOrders(from)getPayoutOrderFee(from)→ one row per contextThe removed methods had no other callers.
getFeeAmount()inLogJobServicebecomes obsolete withthem. The shape of the
FinancialChangesLogentry is unchanged.Semantics were preserved deliberately, not approximated:
cryptoInputis LEFT-joined, so buy-crypto rows without a crypto input (bank purchases — themajority) still count as
regular, exactly as the previous!p.cryptoInput?.paymentLinkPaymentfilter did.
COALESCE(prep, 0) + COALESCE(payout, 0), mirroring the entity getter, where aNULL column contributed 0 via JS
null + x === x. Two such rows exist in the current month; a plainprep + payoutwould have turned them into NULL and swallowed the fee that is set.SUMover an empty set is NULL and is mapped to 0 — no rows in the period means no fees.Verification
Beyond the unit tests, the SQL that TypeORM generates for all four aggregates was run read-only
against the production database and compared to the
FinancialChangesLogentry the currentimplementation had just written:
The remaining differences are float summation order, in the 11th significant digit.
The same four statements execute in 15–33 ms each.
Tests
*.pg.spec.tssuites run the aggregates against Postgres semantics viapg-mem, followingthe existing
trading-rule.service.pg.spec.tspattern — a mocked repository executes no SQL, so awrong CASE, a missing GROUP BY, a dropped COALESCE or a flipped date comparison would pass unnoticed.
Covered: the regular/payment-link split, rows without a crypto input, negative rebates netting
within a group, NULL fees, the date boundary and the empty period.
log-job.service.spec.tskeeps covering howgetChangeLogassembles the totals, now against theaggregate shapes, plus two new cases: the buy-fee split across both sources and the separation of
the ref payout context from all others.