Use less memory for multichain indexers - #583
Conversation
WalkthroughThe changes update how the target buffer size is determined for indexing. The environment variable for buffer size is now optional, and if unset, a new logic computes the buffer size based on the number of chain fetchers and batch size. Metrics and state now use this computed value. Changes
Sequence Diagram(s)sequenceDiagram
participant App
participant Env
participant GlobalState
App->>Env: Get ENVIO_INDEXING_MAX_BUFFER_SIZE (optional)
Env-->>App: Return Some(size) or None
App->>GlobalState: Call make()
GlobalState->>Env: Access targetBufferSize, maxProcessBatchSize
GlobalState->>GlobalState: Compute targetBufferSize:
alt ENVIO_INDEXING_MAX_BUFFER_SIZE is set
GlobalState->>GlobalState: Use env value
else Not set
GlobalState->>GlobalState: Compute from batch size and fetchers
end
GlobalState->>Prometheus: Update IndexingTargetBufferSize metric
GlobalState-->>App: Return state with computed buffer size
Poem
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
codegenerator/cli/templates/static/codegen/src/globalState/GlobalState.res (1)
73-77: Well-implemented adaptive buffer sizing logic.The implementation correctly handles the optional environment variable and provides intelligent fallback logic that reduces memory usage for multichain indexers by using a smaller buffer multiplier when there are many chains.
Consider extracting the multiplier calculation for improved readability:
let targetBatchesInBuffer = 3 + let bufferMultiplier = chainManager.chainFetchers->ChainMap.size > targetBatchesInBuffer ? 1 : targetBatchesInBuffer let targetBufferSize = switch Env.targetBufferSize { | Some(size) => size - | None => Env.maxProcessBatchSize * (chainManager.chainFetchers->ChainMap.size > targetBatchesInBuffer ? 1 : targetBatchesInBuffer) + | None => Env.maxProcessBatchSize * bufferMultiplier }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
scenarios/test_codegen/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (2)
codegenerator/cli/templates/static/codegen/src/Env.res(1 hunks)codegenerator/cli/templates/static/codegen/src/globalState/GlobalState.res(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build_and_test
🔇 Additional comments (3)
codegenerator/cli/templates/static/codegen/src/Env.res (1)
16-16: LGTM: Environment variable now properly optional.The change from a mandatory integer with fallback to an optional integer enables dynamic buffer size calculation in GlobalState.res, which aligns with the PR objective of optimizing memory usage for multichain indexers.
codegenerator/cli/templates/static/codegen/src/globalState/GlobalState.res (2)
79-79: Correct Prometheus metric update.The metric now properly reflects the computed buffer size rather than the raw environment variable, ensuring accurate monitoring of the actual buffer size being used.
85-85: State record properly updated with computed value.The targetBufferSize field correctly uses the computed value, maintaining consistency between the environment configuration, Prometheus metrics, and internal state.
| let targetBufferSize = switch Env.targetBufferSize { | ||
| | Some(size) => size | ||
| | None => Env.maxProcessBatchSize * (chainManager.chainFetchers->ChainMap.size > targetBatchesInBuffer ? 1 : targetBatchesInBuffer) | ||
| } |
There was a problem hiding this comment.
We don't need multiple batches in the buffer, when there are multiple chains.
There was a problem hiding this comment.
Ideally to calculate it dynamically, so it ignores fully synced chains.
There was a problem hiding this comment.
For better memory, it'd be possible to make it less than Env.maxProcessBatchSize per chain. However, I don't want to do it to keep the single-chain batch optimisation working.
There was a problem hiding this comment.
Nice, do you want to add these notes in a comment for future implementation?
There was a problem hiding this comment.
I'll add them in another PR
2 participants
Summary by CodeRabbit