Allow operator defined backfill block size #834
Conversation
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. |
WalkthroughThe changes introduce a configurable backfill block size for log streaming, replacing a fixed constant with per-instance settings. This includes new fields and options in configuration structs, updates to constructors to accept and propagate the new parameter, and modifications to test utilities to support the new configuration. Changes
Sequence Diagram(s)sequenceDiagram
participant Config as ContractsOptions
participant Indexer
participant AppChain
participant RpcLogStreamer
Config->>Indexer: Provides BackfillBlockSize
Indexer->>AppChain: Passes BackfillBlockSize to NewAppChain
AppChain->>RpcLogStreamer: Initializes with WithBackfillBlockSize
RpcLogStreamer->>RpcLogStreamer: Uses backfillBlockSize for log fetching
Suggested reviewers
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (1.64.8)Error: you are using a configuration file for golangci-lint v2 with golangci-lint v1: please use golangci-lint v2 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (5)
🔇 Additional comments (1)
✨ Finishing Touches
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: 3
🧹 Nitpick comments (1)
pkg/indexer/app_chain/app_chain.go (1)
44-44: Function signature correctly updated for configurable block size.The addition of the
blockSize uint64parameter properly enables configuration of the backfill block size, replacing the previous fixed constant approach.Consider adding validation for the
blockSizeparameter to ensure it's greater than zero:func NewAppChain( ctxwc context.Context, log *zap.Logger, cfg config.AppChainOptions, db *sql.DB, validationService mlsvalidate.MLSValidationService, blockSize uint64, ) (*AppChain, error) { + if blockSize == 0 { + return nil, fmt.Errorf("blockSize must be greater than 0") + } ctxwc, cancel := context.WithCancel(ctxwc)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
pkg/blockchain/rpcLogStreamer.go(3 hunks)pkg/blockchain/rpcLogStreamer_test.go(1 hunks)pkg/config/options.go(1 hunks)pkg/indexer/app_chain/app_chain.go(2 hunks)pkg/indexer/indexer.go(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
pkg/blockchain/rpcLogStreamer_test.go (1)
pkg/blockchain/rpcLogStreamer.go (1)
WithBackfillBlockSize(44-49)
pkg/indexer/app_chain/app_chain.go (1)
pkg/blockchain/rpcLogStreamer.go (1)
WithBackfillBlockSize(44-49)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: Code Review
- GitHub Check: Push Docker Images to GitHub Packages (xmtpd-cli)
- GitHub Check: Test (Node)
- GitHub Check: Push Docker Images to GitHub Packages (xmtpd)
- GitHub Check: Upgrade Tests
🔇 Additional comments (4)
pkg/blockchain/rpcLogStreamer_test.go (1)
57-57: Good test coverage for the new configuration option.The test correctly includes the new
WithBackfillBlockSize(500)option, ensuring the configurable backfill block size path is covered. The value 500 is consistent with the default configured inpkg/config/options.go.pkg/config/options.go (1)
29-29: Well-structured configuration field addition.The new
BackfillBlockSizefield is properly defined with appropriate CLI and environment variable tags, descriptive help text, and a reasonable default value. Theuint64type is consistent with its usage throughout the codebase.pkg/indexer/indexer.go (1)
39-39:Details
✅ Verification successful
Correct configuration propagation to app chain.
The
cfg.BackfillBlockSizeparameter is properly passed through to theNewAppChainconstructor, maintaining the configuration flow from the options to the blockchain components.Since the
NewAppChainfunction signature has changed, let's verify that all call sites have been updated:
🏁 Script executed:
#!/bin/bash # Description: Verify all call sites to NewAppChain have been updated to include the new blockSize parameter # Expected: Only find calls with the correct number of parameters # Search for NewAppChain function calls rg -A 10 "NewAppChain\("Length of output: 1131
All
NewAppChaincall sites correctly updated – approving.Verification confirmed only two occurrences of
NewAppChain(:
- Definition in
pkg/indexer/app_chain/app_chain.go- Invocation in
pkg/indexer/indexer.goalready includescfg.BackfillBlockSizeNo further updates required.
pkg/indexer/app_chain/app_chain.go (1)
117-117: Correct integration of configurable backfill block size.The
blockchain.WithBackfillBlockSize(blockSize)option is properly applied to the RpcLogStreamer, enabling dynamic configuration of the backfill block size per the PR objectives.
Replace hardcoded 1000-block backfill size with configurable block size parameter in
|
Replace hardcoded 1000-block backfill size with configurable block size parameter in
RpcLogStreamerIntroduces configurable block size for blockchain log backfilling through:
backfillBlockSizefield toRpcLogStreamerin rpcLogStreamer.go with newWithBackfillBlockSizeoption functionBackfillBlockSizeconfiguration option in options.go with CLI flag and environment variable supportNewAppChainin app_chain.go to accept block size parameterGetNextPagemethod to use configurable block size instead of hardcoded 1000-block constant📍Where to Start
Start with the
RpcLogStreamerstruct andWithBackfillBlockSizeoption function changes in rpcLogStreamer.go, which define the core functionality for configurable block sizes.Macroscope summarized db8f348.
Summary by CodeRabbit
New Features
Chores