/db/history: update the claimable balance SQL query #4398
Conversation
…IT depending on whether paging cursor parameters are present or not
There was a problem hiding this comment.
After performing several more benchmarking experiments, I think we actually need to place the limit clause based on whether or not the claimants filter is present.
Regardless of pagination, if the user is filtering on claimants, the limit clause should be outside the subselect query:
") select " + claimableBalancesSelectStatement + " from cb LIMIT ?"
If the claimants filter is not present, the LIMIT clause should always reside in the subselect:
"LIMIT ?) select " + claimableBalancesSelectStatement + " from cb"
I tested with the following combinations:
- filtering only
asset - filtering
assetwith pagination - filtering only
claimants - filtering
claimantswith pagination - filtering only
sponsor - only pagination
- filtering
assetandclaimantswith pagination
the assets filter sees great performance improvement by always having the LIMIT clause within the subselect. Other combinations only had negligible improvement. Conversely, the claimants filter requires the LIMIT clause to always exist outside the subselect for optimal performance.
|
@2opremio , I can adjust this to assemble the sql query based on @sydneynotthecity 's profiling, should I proceed on that or hold off? |
|
Oh crap, I deleted the branch by mistake. Let me recreate it. |
|
Done! No, we shouldn't hold off! |
…ce query by claimant id per profiling
|
@sydneynotthecity @2opremio , I reworked that claimant balance query for LIMIT placement per the profiling results and recommendations. If the query has asset and claimant parameters, then i chose to let that follow the LIMIT placement per the claimant, i.e. outside query. |
| // we need to use WITH syntax and correct LIMIT placement to force the query planner to use the right | ||
| // indexes, otherwise when the limit is small, it will use an index scan | ||
| // which will be very slow once we have millions of records | ||
| limitClausePlacement := "LIMIT ?) select " + claimableBalancesSelectStatement + " from cb" |
There was a problem hiding this comment.
Do we need to worry about adding a space before LIMIT? Or does the original statement insert the necessary space? Like " LIMIT ?) select " instead
There was a problem hiding this comment.
Hello @sydneynotthecity , yes, looks like the squirrel Suffix takes care of spacing on concatenation, i dumped the final emitted sql under both conditions and looks ok with spacing:
# limit on the outer
WITH cb AS ( SELECT cb.id, cb.claimants, cb.asset, cb.amount, cb.sponsor, cb.last_modified_ledger, cb.flags FROM claimable_balances cb WHERE (cb.last_modified_ledger, cb.id) > (?, ?) AND cb.claimants @> '[{"destination": "GC3C4AKRBQLHOJ45U4XG35ESVWRDECWO5XLDGYADO6DPR3L7KIDVUMML"}]' ORDER BY cb.last_modified_ledger asc, cb.id asc ) select cb.id, cb.claimants, cb.asset, cb.amount, cb.sponsor, cb.last_modified_ledger, cb.flags from cb LIMIT ?
# limit on the inner
WITH cb AS ( SELECT cb.id, cb.claimants, cb.asset, cb.amount, cb.sponsor, cb.last_modified_ledger, cb.flags FROM claimable_balances cb ORDER BY cb.last_modified_ledger asc, cb.id asc LIMIT ?) select cb.id, cb.claimants, cb.asset, cb.amount, cb.sponsor, cb.last_modified_ledger, cb.flags from cb
thanks for double checking on that!
* #4382: refined the placement of LIMIT clause on claimant balance query by claimant id per profiling results.
* #4382: refined the placement of LIMIT clause on claimant balance query by claimant id per profiling results.
PR Checklist
PR Structure
otherwise).
services/friendbot, orallordocif the changes are broad or impact manypackages.
Thoroughness
.mdfiles, etc... affected by this change). Take a look in the
docsfolder for a given service,like this one.
Release planning
needed with deprecations, added features, breaking changes, and DB schema changes.
semver, or if it's mainly a patch change. The PR is targeted at the next
release branch if it's not a patch change.
What
Change the claimable balance SQL Query generation to position LIMIT clause dependent on presence of cursor parameters as that determines how many effective rows and the indexes are chosen by db query planner.
Why
the query planner isn't picking the right indexes to use on claimable balance query due to LIMIT clause
Known limitations