[ENG-1509] Fix /v4/fills/parentSubaccount query performance#3279
Merged
davidli1997 merged 5 commits intomainfrom Dec 10, 2025
Merged
[ENG-1509] Fix /v4/fills/parentSubaccount query performance#3279davidli1997 merged 5 commits intomainfrom
davidli1997 merged 5 commits intomainfrom
Conversation
Contributor
WalkthroughReplaced a dynamic subquery-based parent-subaccount filter in the fills store with a pre-resolved array of subaccount IDs. Implemented Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (56)
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 |
Kefancao
reviewed
Dec 10, 2025
| subaccountNumber: number, | ||
| }): Promise<string[]> { | ||
| const result = await knexReadReplica.getConnection() | ||
| .select('id') |
Contributor
There was a problem hiding this comment.
There is a sub accounts table module that is able to help with this query, can you use that instead?
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
indexer/packages/postgres/src/stores/subaccount-table.ts (1)
205-235: Parent‑subaccount helper is correct but could push filtering into SQLThe helper’s semantics look right: you resolve all subaccounts for the address, then select those whose
subaccountNumberdiffers from the parent’s by a multiple ofMAX_PARENT_SUBACCOUNTS, and return their IDs. That should preserve existing parent/child behavior while letting callers avoid the problematicIN (subquery)pattern.Two follow‑ups to consider:
- You currently fetch all subaccounts for the address and filter in JS. If an address ever has many subaccounts, this adds unnecessary network and app‑side work. A more scalable pattern would be to push the modulo filter into the SQL query (either via a dedicated query here or by extending
findAll) so Postgres does the filtering directly.- For extra clarity and safety around JS’s
%operator, you could make the non‑negativity explicit when computing the offset, e.g.:This documents the intended domain and avoids any surprises if types ever loosen.const offset = subaccount.subaccountNumber - parentSubaccount.subaccountNumber; return offset >= 0 && offset % MAX_PARENT_SUBACCOUNTS === 0;Functionally this is good to ship; the above are performance/readability refinements you can address now or later.
.github/workflows/indexer-build-and-push-dev-staging.yml (1)
5-9: Consider whether the personal branch trigger should be temporaryAdding
davidli/fills_endpointto the push triggers is fine for developing this PR, but it will keep building and pushing images for any future pushes to a branch with this exact name. Once the feature is merged, consider removing this entry (or replacing with a more generic pattern) to avoid unexpected CI runs and image pushes.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/indexer-build-and-push-dev-staging.yml(1 hunks)indexer/packages/postgres/src/stores/subaccount-table.ts(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2024-11-15T16:00:11.304Z
Learnt from: hwray
Repo: dydxprotocol/v4-chain PR: 2551
File: protocol/x/subaccounts/keeper/subaccount.go:852-865
Timestamp: 2024-11-15T16:00:11.304Z
Learning: The function `GetCrossInsuranceFundBalance` in `protocol/x/subaccounts/keeper/subaccount.go` already existed and was just moved in this PR; changes to its error handling may be out of scope.
Applied to files:
indexer/packages/postgres/src/stores/subaccount-table.ts
🧬 Code graph analysis (1)
indexer/packages/postgres/src/stores/subaccount-table.ts (2)
indexer/packages/postgres/src/types/utility-types.ts (1)
Options(15-25)indexer/packages/postgres/src/constants.ts (1)
MAX_PARENT_SUBACCOUNTS(130-130)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (56)
- GitHub Check: test / run_command
- GitHub Check: (Public Testnet) Build and Push ECS Services / call-build-and-push-ecs-service-roundtable / (roundtable) Build and Push
- GitHub Check: (Public Testnet) Build and Push ECS Services / call-build-and-push-ecs-service-socks / (socks) Build and Push
- GitHub Check: (Public Testnet) Build and Push ECS Services / call-build-and-push-vulcan / (vulcan) Build and Push
- GitHub Check: (Public Testnet) Build and Push ECS Services / call-build-and-push-auxo-lambda / (auxo) Build and Push Lambda
- GitHub Check: (Public Testnet) Build and Push ECS Services / call-build-and-push-bazooka-lambda / (bazooka) Build and Push Lambda
- GitHub Check: (Public Testnet) Build and Push ECS Services / call-build-and-push-ecs-service-comlink / (comlink) Build and Push
- GitHub Check: (Public Testnet) Build and Push ECS Services / call-build-and-push-ecs-service-ender / (ender) Build and Push
- GitHub Check: call-build-ecs-service-ender / (ender) Check docker image build
- GitHub Check: call-build-ecs-service-roundtable / (roundtable) Check docker image build
- GitHub Check: call-build-ecs-service-vulcan / (vulcan) Check docker image build
- GitHub Check: call-build-ecs-service-socks / (socks) Check docker image build
- GitHub Check: call-build-ecs-service-comlink / (comlink) Check docker image build
- GitHub Check: (Mainnet) Build and Push ECS Services / call-build-and-push-bazooka-lambda / (bazooka) Build and Push Lambda
- GitHub Check: (Mainnet) Build and Push ECS Services / call-build-and-push-ecs-service-ender / (ender) Build and Push
- GitHub Check: check-build-auxo
- GitHub Check: (Mainnet) Build and Push ECS Services / call-build-and-push-auxo-lambda / (auxo) Build and Push Lambda
- GitHub Check: (Mainnet) Build and Push ECS Services / call-build-and-push-vulcan / (vulcan) Build and Push
- GitHub Check: (Mainnet) Build and Push ECS Services / call-build-and-push-ecs-service-comlink / (comlink) Build and Push
- GitHub Check: (Mainnet) Build and Push ECS Services / call-build-and-push-ecs-service-socks / (socks) Build and Push
- GitHub Check: check-build-bazooka
- GitHub Check: (Mainnet) Build and Push ECS Services / call-build-and-push-ecs-service-roundtable / (roundtable) Build and Push
- GitHub Check: lint
- GitHub Check: build-and-push-testnet
- GitHub Check: build-and-push-mainnet
- GitHub Check: run_command
- GitHub Check: Analyze (go)
- GitHub Check: (Dev4) Build and Push ECS Services / call-build-and-push-ecs-service-socks / (socks) Build and Push
- GitHub Check: (Dev2) Build and Push ECS Services / call-build-and-push-bazooka-lambda / (bazooka) Build and Push Lambda
- GitHub Check: (Dev4) Build and Push ECS Services / call-build-and-push-ecs-service-comlink / (comlink) Build and Push
- GitHub Check: (Dev4) Build and Push ECS Services / call-build-and-push-auxo-lambda / (auxo) Build and Push Lambda
- GitHub Check: (Dev4) Build and Push ECS Services / call-build-and-push-ecs-service-ender / (ender) Build and Push
- GitHub Check: (Dev) Build and Push ECS Services / call-build-and-push-auxo-lambda / (auxo) Build and Push Lambda
- GitHub Check: (Dev) Build and Push ECS Services / call-build-and-push-ecs-service-roundtable / (roundtable) Build and Push
- GitHub Check: (Dev) Build and Push ECS Services / call-build-and-push-ecs-service-socks / (socks) Build and Push
- GitHub Check: (Dev4) Build and Push ECS Services / call-build-and-push-bazooka-lambda / (bazooka) Build and Push Lambda
- GitHub Check: (Dev4) Build and Push ECS Services / call-build-and-push-vulcan / (vulcan) Build and Push
- GitHub Check: (Dev) Build and Push ECS Services / call-build-and-push-bazooka-lambda / (bazooka) Build and Push Lambda
- GitHub Check: (Dev) Build and Push ECS Services / call-build-and-push-vulcan / (vulcan) Build and Push
- GitHub Check: (Dev4) Build and Push ECS Services / call-build-and-push-ecs-service-roundtable / (roundtable) Build and Push
- GitHub Check: (Dev) Build and Push ECS Services / call-build-and-push-ecs-service-comlink / (comlink) Build and Push
- GitHub Check: (Staging) Build and Push ECS Services / call-build-and-push-auxo-lambda / (auxo) Build and Push Lambda
- GitHub Check: (Dev) Build and Push ECS Services / call-build-and-push-ecs-service-ender / (ender) Build and Push
- GitHub Check: (Dev2) Build and Push ECS Services / call-build-and-push-ecs-service-roundtable / (roundtable) Build and Push
- GitHub Check: (Dev2) Build and Push ECS Services / call-build-and-push-ecs-service-socks / (socks) Build and Push
- GitHub Check: (Dev2) Build and Push ECS Services / call-build-and-push-ecs-service-comlink / (comlink) Build and Push
- GitHub Check: (Staging) Build and Push ECS Services / call-build-and-push-bazooka-lambda / (bazooka) Build and Push Lambda
- GitHub Check: (Dev2) Build and Push ECS Services / call-build-and-push-auxo-lambda / (auxo) Build and Push Lambda
- GitHub Check: (Dev2) Build and Push ECS Services / call-build-and-push-ecs-service-ender / (ender) Build and Push
- GitHub Check: (Staging) Build and Push ECS Services / call-build-and-push-vulcan / (vulcan) Build and Push
- GitHub Check: (Staging) Build and Push ECS Services / call-build-and-push-ecs-service-roundtable / (roundtable) Build and Push
- GitHub Check: (Dev2) Build and Push ECS Services / call-build-and-push-vulcan / (vulcan) Build and Push
- GitHub Check: (Staging) Build and Push ECS Services / call-build-and-push-ecs-service-ender / (ender) Build and Push
- GitHub Check: (Staging) Build and Push ECS Services / call-build-and-push-ecs-service-comlink / (comlink) Build and Push
- GitHub Check: (Staging) Build and Push ECS Services / call-build-and-push-ecs-service-socks / (socks) Build and Push
- GitHub Check: Summary
🔇 Additional comments (1)
indexer/packages/postgres/src/stores/subaccount-table.ts (1)
5-5: Import ofMAX_PARENT_SUBACCOUNTSis appropriateThe new constant import is used below in the parent/child subaccount helper and matches existing constants usage patterns in this module.
Kefancao
requested changes
Dec 10, 2025
| - main | ||
| - 'release/indexer/v[0-9]+.[0-9]+.x' # e.g. release/indexer/v0.1.x | ||
| - 'release/indexer/v[0-9]+.x' # e.g. release/indexer/v1.x | ||
| - davidli/fills_endpoint |
Contributor
There was a problem hiding this comment.
probably take this out before u merge
Kefancao
approved these changes
Dec 10, 2025
Contributor
Author
|
https://github.com/Mergifyio backport release/indexer/v9.5.x |
Contributor
✅ Backports have been createdDetails
|
Contributor
Author
|
https://github.com/Mergifyio backport release/indexer/v9.x |
Contributor
✅ Backports have been createdDetails
|
6 tasks
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
2 participants
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
The
/v4/fills/parentSubaccountNumberendpoint was timing out for users with large fill histories. Root cause: usingIN (subquery)to find fills across parent + child subaccounts caused Postgres to misestimate row counts and choose a catastrophically inefficient query plan—scanning millions of rows instead of using optimal index scans.Why this happens:
When Postgres plans a query with
IN (SELECT ...), it must estimate the subquery's cardinality before execution. For our subaccount lookup, it estimated ~314 rows when the actual result was 5.3M fills (testing with the subaccount causing the issue). This caused Postgres to:With explicit UUIDs, Postgres can use "Index Scan Backward" per subaccount and stop early at the LIMIT.
Fix:
Resolve subaccount IDs to concrete UUIDs in a separate query before the main fills query, replacing the inline subquery.
Testing:
Tested on testnet indexer database (240M fills) against address
dydx1qzq0dx4xjj06d4nxfx7ls53d8wru625s445mhs— with 5.3M fills in subaccount 0.Query performance comparison using
EXPLAIN ANALYZE:With the
"subaccountId" IN (Explicit UUIDs)approach,Summary by CodeRabbit
Refactor
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.