pass context to rpc and worker #205
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
iuwqyir
force-pushed
the
05-19-retry_fetching_block_data_with_decreasing_batch_size
branch
from
4f2ca9a to
c692c5e
Compare
iuwqyir
force-pushed
the
05-20-pass_context_to_rpc_and_worker
branch
from
205a0b8 to
5705df1
Compare
WalkthroughThis change propagates Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant Orchestrator
participant Worker
participant RPCClient
participant Network
Caller->>Orchestrator: Start(ctx)
Orchestrator->>RPCClient: GetLatestBlockNumber(ctx)
RPCClient->>Network: Fetch block number (with ctx)
Network-->>RPCClient: Block number
RPCClient-->>Orchestrator: Block number
Orchestrator->>Worker: Run(ctx, blockNumbers)
Worker->>RPCClient: GetFullBlocks(ctx, blockNumbers)
RPCClient->>Network: Fetch blocks (with ctx)
Network-->>RPCClient: Block data
RPCClient-->>Worker: Block data
Worker-->>Orchestrator: Results
Note over Orchestrator,Worker: All long-running and RPC calls now respect ctx for cancellation and deadlines
sequenceDiagram
participant Test
participant MockIRPCClient
Test->>MockIRPCClient: GetBlocks(ctx, blockNumbers)
MockIRPCClient-->>Test: Mocked block results
Test->>MockIRPCClient: GetFullBlocks(ctx, blockNumbers)
MockIRPCClient-->>Test: Mocked full block results
Note over Test,MockIRPCClient: All test invocations and mocks now require ctx as the first argument
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (12)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (11)
✨ Finishing Touches
🪧 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 (
|
iuwqyir
force-pushed
the
05-19-retry_fetching_block_data_with_decreasing_batch_size
branch
2 times, most recently
from
afbb7e0 to
78e2b76
Compare
iuwqyir
force-pushed
the
05-20-pass_context_to_rpc_and_worker
branch
from
5705df1 to
5f133bc
Compare
iuwqyir
changed the base branch from
05-19-retry_fetching_block_data_with_decreasing_batch_size
to
graphite-base/205
iuwqyir
force-pushed
the
05-20-pass_context_to_rpc_and_worker
branch
from
5f133bc to
9cb66bd
Compare
iuwqyir
force-pushed
the
05-20-pass_context_to_rpc_and_worker
branch
from
9cb66bd to
150d5b4
Compare
![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
🔭 Outside diff range comments (3)
internal/rpc/rpc.go (1)
301-307: 🛠️ Refactor suggestionHasCode method still using context.Background()
While other RPC client methods have been updated to accept and use context parameters, the
HasCodemethod still usescontext.Background()which is inconsistent with the context propagation pattern.-func (rpc *Client) HasCode(address string) (bool, error) { - code, err := rpc.EthClient.CodeAt(context.Background(), gethCommon.HexToAddress(address), nil) +func (rpc *Client) HasCode(ctx context.Context, address string) (bool, error) { + code, err := rpc.EthClient.CodeAt(ctx, gethCommon.HexToAddress(address), nil) if err != nil { return false, err } return len(code) > 0, nil }Don't forget to update the interface definition:
type IRPCClient interface { GetFullBlocks(ctx context.Context, blockNumbers []*big.Int) []GetFullBlockResult GetBlocks(ctx context.Context, blockNumbers []*big.Int) []GetBlocksResult GetTransactions(ctx context.Context, txHashes []string) []GetTransactionsResult GetLatestBlockNumber(ctx context.Context) (*big.Int, error) GetChainID() *big.Int GetURL() string GetBlocksPerRequest() BlocksPerRequestConfig IsWebsocket() bool SupportsTraceBlock() bool - HasCode(address string) (bool, error) + HasCode(ctx context.Context, address string) (bool, error) Close() }internal/orchestrator/committer.go (1)
164-180: 🛠️ Refactor suggestionConsider passing context to the asynchronous publisher operation
While the method now correctly accepts a context parameter, the asynchronous call to
c.publisher.PublishBlockDatain the goroutine doesn't receive the context. This means that if the context is canceled, the publishing operation might still continue.go func() { - if err := c.publisher.PublishBlockData(blockData); err != nil { + if err := c.publisher.PublishBlockData(ctx, blockData); err != nil { log.Error().Err(err).Msg("Failed to publish block data to kafka") } }()This would require updating the Publisher interface and implementation to accept and respect the context.
internal/orchestrator/reorg_handler.go (1)
267-298: 🛠️ Refactor suggestionConsider passing context to the asynchronous publisher operation
While the method now correctly accepts a context parameter and passes it to the worker's Run method, the asynchronous call to
rh.publisher.PublishReorgin the goroutine doesn't receive the context. This means that if the context is canceled, the publishing operation might still continue.go func() { - if err := rh.publisher.PublishReorg(deletedBlockData, data); err != nil { + if err := rh.publisher.PublishReorg(ctx, deletedBlockData, data); err != nil { log.Error().Err(err).Msg("Failed to publish reorg data to kafka") } }()This would require updating the Publisher interface and implementation to accept and respect the context.
🧹 Nitpick comments (5)
internal/worker/worker.go (1)
107-126: Consider adding context cancellation for result collectionAfter launching all goroutines, there's no context cancellation check when collecting and processing results. If the context is cancelled after goroutines are started but before they complete, the function will still wait for all results.
go func() { + done := make(chan struct{}) wg.Wait() - close(resultsCh) + done <- struct{}{} }() -results := make([]rpc.GetFullBlockResult, 0, blockCount) -for batchResults := range resultsCh { - results = append(results, batchResults...) -} +results := make([]rpc.GetFullBlockResult, 0, blockCount) +loop: +for { + select { + case <-ctx.Done(): + log.Debug().Msg("Context canceled while collecting results") + return nil + case <-done: + close(resultsCh) + // Drain the channel after closing + for batchResults := range resultsCh { + results = append(results, batchResults...) + } + break loop + case batchResults, ok := <-resultsCh: + if !ok { + break loop + } + results = append(results, batchResults...) + } +}internal/rpc/batcher.go (2)
19-51: Consider adding context cancellation check before launching goroutinesThe function launches goroutines without checking if the context is already cancelled, which could lead to unnecessary goroutine creation and work.
func RPCFetchInBatches[K any, T any](rpc *Client, ctx context.Context, keys []K, batchSize int, batchDelay int, method string, argsFunc func(K) []interface{}) []RPCFetchBatchResult[K, T] { + // Check if context is already done + select { + case <-ctx.Done(): + return []RPCFetchBatchResult[K, T]{} + default: + // continue processing + } + if len(keys) <= batchSize { return RPCFetchSingleBatch[K, T](rpc, ctx, keys, method, argsFunc) } chunks := common.SliceToChunks[K](keys, batchSize) log.Debug().Msgf("Fetching %s for %d blocks in %d chunks of max %d requests", method, len(keys), len(chunks), batchSize) var wg sync.WaitGroup resultsCh := make(chan []RPCFetchBatchResult[K, T], len(chunks)) for _, chunk := range chunks { + select { + case <-ctx.Done(): + log.Debug().Msg("Context canceled, stopping batch fetching") + // Skip launching more goroutines + goto WaitAndCollect + default: + // continue processing + } + wg.Add(1) go func(chunk []K) { defer wg.Done() resultsCh <- RPCFetchSingleBatch[K, T](rpc, ctx, chunk, method, argsFunc) if batchDelay > 0 { time.Sleep(time.Duration(batchDelay) * time.Millisecond) } }(chunk) } +WaitAndCollect: go func() { wg.Wait() close(resultsCh) }() results := make([]RPCFetchBatchResult[K, T], 0, len(keys)) for batchResults := range resultsCh { results = append(results, batchResults...) } return results }
53-83: Add context cancellation check in RPCFetchSingleBatchThe function should check if the context is cancelled before making the RPC call to avoid unnecessary network operations.
func RPCFetchSingleBatch[K any, T any](rpc *Client, ctx context.Context, keys []K, method string, argsFunc func(K) []interface{}) []RPCFetchBatchResult[K, T] { + // Check if context is already done + select { + case <-ctx.Done(): + // Return error results for all keys + results := make([]RPCFetchBatchResult[K, T], len(keys)) + for i, key := range keys { + results[i] = RPCFetchBatchResult[K, T]{Key: key, Error: ctx.Err()} + } + return results + default: + // continue processing + } + batch := make([]gethRpc.BatchElem, len(keys)) results := make([]RPCFetchBatchResult[K, T], len(keys)) for i, key := range keys { results[i] = RPCFetchBatchResult[K, T]{Key: key} batch[i] = gethRpc.BatchElem{ Method: method, Args: argsFunc(key), Result: new(T), } } err := rpc.RPCClient.BatchCallContext(ctx, batch) if err != nil { for i := range results { results[i].Error = err } return results } for i, elem := range batch { if elem.Error != nil { results[i].Error = elem.Error } else { results[i].Result = *elem.Result.(*T) } } return results }internal/rpc/rpc.go (1)
211-219: Consider updating setChainID to accept a context parameterThe
setChainIDmethod is an internal method that usescontext.Background(). For consistency with the context propagation pattern, consider accepting a context parameter.-func (rpc *Client) setChainID() error { - chainID, err := rpc.EthClient.ChainID(context.Background()) +func (rpc *Client) setChainID(ctx context.Context) error { + chainID, err := rpc.EthClient.ChainID(ctx) if err != nil { return fmt.Errorf("failed to get chain ID: %v", err) } rpc.chainID = chainID config.Cfg.RPC.ChainID = chainID.String() return nil }And update the call in the
Initializefunction:- chainIdErr := rpc.setChainID() + chainIdErr := rpc.setChainID(context.Background())internal/orchestrator/reorg_handler_test.go (1)
1-787: Add tests for context cancellation scenariosWhile all method calls have been updated to include context parameters, there are no tests that verify the behavior when a context is cancelled. Consider adding tests that explicitly test context cancellation scenarios.
Here's an example test that could be added:
func TestReorgHandlerRespectsContextCancellation(t *testing.T) { mockRPC := mocks.NewMockIRPCClient(t) mockMainStorage := mocks.NewMockIMainStorage(t) mockOrchestratorStorage := mocks.NewMockIOrchestratorStorage(t) mockStorage := storage.IStorage{ MainStorage: mockMainStorage, OrchestratorStorage: mockOrchestratorStorage, } mockRPC.EXPECT().GetChainID().Return(big.NewInt(1)) mockRPC.EXPECT().GetBlocksPerRequest().Return(rpc.BlocksPerRequestConfig{Blocks: 100}) mockOrchestratorStorage.EXPECT().GetLastReorgCheckedBlockNumber(big.NewInt(1)).Return(big.NewInt(100), nil) mockMainStorage.EXPECT().GetMaxBlockNumber(big.NewInt(1)).Return(big.NewInt(1000), nil) // Create a context that will be cancelled ctx, cancel := context.WithCancel(context.Background()) // Set up expectations for the first GetBlocks call mockRPC.EXPECT().GetBlocks(ctx, mock.Anything).Run(func(ctx context.Context, blockNumbers []*big.Int) { // Cancel the context before returning cancel() }).Return([]rpc.GetBlocksResult{}) handler := NewReorgHandler(mockRPC, mockStorage) // The method should return early due to context cancellation _, err := handler.RunFromBlock(ctx, big.NewInt(99)) assert.ErrorIs(t, err, context.Canceled) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
internal/orchestrator/chain_tracker.go(1 hunks)internal/orchestrator/committer.go(9 hunks)internal/orchestrator/committer_test.go(9 hunks)internal/orchestrator/failure_recoverer.go(1 hunks)internal/orchestrator/poller.go(5 hunks)internal/orchestrator/reorg_handler.go(8 hunks)internal/orchestrator/reorg_handler_test.go(24 hunks)internal/rpc/batcher.go(4 hunks)internal/rpc/rpc.go(5 hunks)internal/worker/worker.go(5 hunks)test/mocks/MockIRPCClient.go(11 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (6)
internal/orchestrator/committer_test.go (1)
internal/rpc/rpc.go (1)
GetFullBlockResult(18-22)
internal/worker/worker.go (2)
internal/rpc/rpc.go (1)
GetFullBlockResult(18-22)configs/config.go (1)
Cfg(170-170)
internal/orchestrator/reorg_handler_test.go (2)
internal/rpc/rpc.go (2)
GetBlocksResult(24-28)GetFullBlockResult(18-22)internal/common/block.go (1)
Block(8-33)
internal/rpc/batcher.go (1)
internal/rpc/rpc.go (1)
Client(56-65)
internal/orchestrator/committer.go (1)
internal/common/block.go (2)
BlockData(61-66)Block(8-33)
test/mocks/MockIRPCClient.go (1)
internal/rpc/rpc.go (3)
GetBlocksResult(24-28)GetFullBlockResult(18-22)GetTransactionsResult(30-33)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Graphite / mergeability_check
🔇 Additional comments (56)
internal/orchestrator/chain_tracker.go (1)
38-38: Good addition of context to RPC callThis change correctly passes the context to the
GetLatestBlockNumbermethod, enabling proper cancellation and timeout handling during RPC operations. This aligns with the PR's goal of propagating context throughout the codebase.internal/orchestrator/failure_recoverer.go (1)
78-78: Context properly propagated to workerThis change correctly passes the context to the worker's
Runmethod, ensuring that cancellation signals are properly propagated from the failure recoverer to the worker. This is essential for graceful shutdown and resource management.internal/orchestrator/committer_test.go (9)
257-257: Properly updated test with context parameterThis test now correctly includes the context parameter in the call to
getSequentialBlockDataToCommit, maintaining consistency with the updated method signature.
293-293: Properly updated test with context parameterThis test now correctly includes the context parameter in the call to
getSequentialBlockDataToCommit, maintaining consistency with the updated method signature.
323-323: Context properly passed to commit method in testThe test now correctly includes the context parameter in the
commitmethod call, aligning with the method's updated signature.
346-346: Context propagated to RPC mock expectationsThe mock expectation for
GetFullBlocksnow correctly includes the context parameter, ensuring that the test matches the updated interface contract.
355-355: Context properly passed to handleGap method in testThe test now correctly includes the context parameter in the
handleGapmethod call, aligning with the method's updated signature.
466-466: Context properly passed to mock RPC expectationsThe mock expectation for
GetFullBlocksnow correctly includes the context parameter, ensuring that the test matches the updated interface contract.
485-485: Properly updated test with context parameterThis test now correctly includes the context parameter in the call to
getSequentialBlockDataToCommit, maintaining consistency with the updated method signature.
512-512: Context properly passed to mock RPC expectationsThe mock expectation for
GetFullBlocksnow correctly includes the context parameter, ensuring that the test matches the updated interface contract.
529-529: Properly updated test with context parameterThis test now correctly includes the context parameter in the call to
getSequentialBlockDataToCommit, maintaining consistency with the updated method signature.internal/orchestrator/poller.go (6)
106-106: Context properly passed to getNextBlockRange methodThis change correctly propagates the polling context to the
getNextBlockRangemethod, ensuring that block range retrieval operations can be canceled when needed.
120-120: Context properly passed to Poll methodThis change correctly propagates the polling context to the
Pollmethod, ensuring that polling operations can be canceled when needed.
149-149: Method signature updated to accept contextThe
Pollmethod signature has been correctly updated to accept a context parameter, ensuring that context can be propagated through the polling flow.
164-164: Context properly passed to workerThis change correctly passes the context from the
Pollmethod to the worker'sRunmethod, ensuring that cancellation signals are properly propagated to the worker.
173-173: Method signature updated to accept contextThe
getNextBlockRangemethod signature has been correctly updated to accept a context parameter, allowing for context-aware block range retrieval.
174-174: Context properly passed to RPC callThis change correctly passes the context to the
GetLatestBlockNumbermethod, enabling proper cancellation and timeout handling during RPC operations.internal/worker/worker.go (8)
4-4: Add context package import to support cancellationThe addition of the context package import enables cooperative cancellation and deadline handling throughout the worker's operations.
27-32: Good implementation of context cancellation checkThe early cancellation check at the beginning of
processChunkWithRetryensures we don't waste resources if the context is already cancelled before processing begins.
39-39: Propagate context to RPC callsCorrectly passing the context to the RPC client's
GetFullBlocksmethod enables cancellation propagation down to the network layer.
71-71: Properly pass context to recursive callsThe context is correctly passed to recursive calls, ensuring cancellation can happen at any depth of retry attempts.
87-87: Context propagation in goroutinesContext is correctly passed to the parallel retry operations, allowing cancellation to be respected in both chunks when splitting failed blocks.
Also applies to: 92-92
98-98: Add context parameter to Run methodAdding the context parameter to the
Runmethod allows the caller to control cancellation of the entire block processing operation.
108-114: Add context cancellation check in the main processing loopThis check ensures the worker stops processing new chunks if the context is cancelled, preventing unnecessary work.
119-119: Propagate context to chunk processingCorrectly passes the context to the chunk processing function, ensuring cancellation is respected in each processing goroutine.
internal/rpc/batcher.go (5)
19-19: Add context parameter to RPCFetchInBatches functionAdding a context parameter allows callers to control the lifecycle of batch operations, enabling cancellation propagation throughout the RPC layer.
21-21: Propagate context to single batch fetchThe context is correctly passed to the
RPCFetchSingleBatchfunction when handling a small number of keys.
34-34: Pass context to batch operations in goroutinesThe context is properly propagated to each batch operation running in a separate goroutine.
53-53: Add context parameter to RPCFetchSingleBatch functionThis function now accepts a context parameter, allowing for cancellation control during the RPC batch call.
66-66: Use provided context for BatchCallContextThe function correctly uses the provided context when making the RPC batch call, allowing for cancellation during network operations.
internal/rpc/rpc.go (5)
42-53: Update IRPCClient interface to support context parametersThe interface has been properly updated to include context parameters in all relevant RPC methods, following Go best practices for context propagation.
221-261: Update GetFullBlocks method to accept and propagate contextThe method now accepts a context parameter and passes it to all RPC calls, including multiple parallel goroutines, ensuring cancellation can be handled at all levels.
263-276: Update GetBlocks method to accept and propagate contextThe method now accepts a context parameter and correctly passes it to the underlying RPC call.
278-291: Update GetTransactions method to accept and propagate contextThe method now accepts a context parameter and correctly passes it to the underlying RPC call.
293-299: Update GetLatestBlockNumber method to use passed contextThe method now accepts a context parameter and uses it when calling the Ethereum client's BlockNumber method.
internal/orchestrator/reorg_handler_test.go (6)
243-247: Update GetBlocks calls to include context parameterAll calls to the
GetBlocksmethod have been correctly updated to includecontext.Background()as the first parameter, ensuring the tests match the updated method signature.Also applies to: 277-281, 307-318, 357-362, 364-369, 410-420, 459-471, 600-607, 655-665
500-504: Update GetFullBlocks calls to include context parameterAll calls to the
GetFullBlocksmethod have been correctly updated to includecontext.Background()as the first parameter, ensuring the tests match the updated method signature.Also applies to: 609-611, 667-676, 731-737
250-250: Update findReorgedBlockNumbers calls to include context parameterAll calls to the
findReorgedBlockNumbersmethod have been correctly updated to includecontext.Background()as the first parameter, ensuring the tests match the updated method signature.Also applies to: 284-284, 333-333, 386-386, 435-435, 482-482
510-510: Update handleReorg call to include context parameterThe call to the
handleReorgmethod has been correctly updated to includecontext.Background()as the first parameter, ensuring the test matches the updated method signature.
563-563: Update RunFromBlock calls to include context parameterAll calls to the
RunFromBlockmethod have been correctly updated to includecontext.Background()as the first parameter, ensuring the tests match the updated method signature.Also applies to: 618-618, 681-681, 744-744, 781-781, 818-818
539-539: Update to use context in Start method callThe call to the
Startmethod has been updated to includecontext.Background(), ensuring proper context propagation for the reorg handler's continuous operation.internal/orchestrator/committer.go (5)
54-66: Context propagation looks good!The
Startmethod now properly handles context cancellation in its select loop, and correctly passes the context togetSequentialBlockDataToCommit.
75-75: Context propagation looks good!The context is correctly propagated to the
commitmethod.
111-162: Context parameter correctly added to getSequentialBlockDataToCommitThe method now accepts a context parameter and properly propagates it to child method calls.
202-225: Context parameter correctly added to handleGapThe method now accepts a context parameter and properly propagates it to the Poll method call.
227-247: Context parameter correctly added to handleMissingStagingDataThe method now accepts a context parameter and properly propagates it to the Poll method call.
internal/orchestrator/reorg_handler.go (4)
73-100: Context propagation looks good!The
Startmethod now properly handles context cancellation in its select loop, and correctly passes the context toRunFromBlock.🧰 Tools
🪛 golangci-lint (1.64.8)
96-96: Error return value of
rh.storage.OrchestratorStorage.SetLastReorgCheckedBlockNumberis not checked(errcheck)
102-148: Context parameter correctly added to RunFromBlockThe method now accepts a context parameter and properly propagates it to child method calls.
193-225: Context properly propagated in findReorgedBlockNumbersThe method correctly passes the context to
getNewBlocksByNumberand also to the recursive call to itself.
227-265: Context parameter correctly added to getNewBlocksByNumberThe method now accepts a context parameter and properly propagates it to the RPC GetBlocks call.
test/mocks/MockIRPCClient.go (6)
8-13: Context import added correctlyThe context package is now properly imported.
61-79: Context parameter correctly added to GetBlocks mockThe mock method now accepts a context parameter, matching the updated interface.
87-108: Mock expectation helpers updated correctlyThe helper methods for setting up expectations now handle the context parameter.
202-249: Context parameter correctly added to GetFullBlocks mockThe mock method and its helper methods now handle the context parameter.
251-307: Context parameter correctly added to GetLatestBlockNumber mockThe mock method and its helper methods now handle the context parameter.
309-356: Context parameter correctly added to GetTransactions mockThe mock method and its helper methods now handle the context parameter.
iuwqyir
force-pushed
the
05-20-pass_context_to_rpc_and_worker
branch
from
150d5b4 to
3b3058e
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
internal/handlers/search_handlers.go (1)
61-93: 🛠️ Refactor suggestionConsider propagating request context to downstream functions.
The
Searchfunction receives a Gin context but doesn't propagate the request context to downstream functions likeexecuteSearch. This prevents proper cancellation propagation if the client disconnects.func Search(c *gin.Context) { + ctx := c.Request.Context() chainId, err := api.GetChainId(c) if err != nil { api.BadRequestErrorHandler(c, err) return } searchInput := parseSearchInput(c.Param("input")) if searchInput.ErrorMessage != "" { api.BadRequestErrorHandler(c, fmt.Errorf(searchInput.ErrorMessage)) return } mainStorage, err := getMainStorage() if err != nil { log.Error().Err(err).Msg("Error getting main storage") api.InternalErrorHandler(c) return } - result, err := executeSearch(mainStorage, chainId, searchInput) + result, err := executeSearch(ctx, mainStorage, chainId, searchInput) if err != nil { log.Error().Err(err).Msg("Error executing search") api.InternalErrorHandler(c) return } sendJSONResponse(c, api.QueryResponse{ Meta: api.Meta{ ChainId: chainId.Uint64(), }, Data: result, }) }Update other functions in the call chain accordingly to accept and propagate the context.
🧹 Nitpick comments (1)
internal/handlers/search_handlers.go (1)
333-374: Consider adding timeout to prevent long-running searches.The
searchByAddressfunction makes multiple storage and RPC calls without any timeout constraints. In line with the PR's goal of improving resource management, consider adding a timeout context to prevent potentially long-running searches from consuming resources indefinitely.func searchByAddress(mainStorage storage.IMainStorage, chainId *big.Int, address string) (SearchResultModel, error) { + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() searchResult := SearchResultModel{Type: SearchResultTypeAddress} - contractCode, err := checkIfContractHasCode(chainId, address) + contractCode, err := checkIfContractHasCode(ctx, chainId, address) if err != nil { return searchResult, err } // ... rest of the function }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
internal/handlers/search_handlers.go(2 hunks)internal/orchestrator/chain_tracker.go(1 hunks)internal/orchestrator/committer.go(9 hunks)internal/orchestrator/committer_test.go(9 hunks)internal/orchestrator/failure_recoverer.go(1 hunks)internal/orchestrator/poller.go(5 hunks)internal/orchestrator/reorg_handler.go(8 hunks)internal/orchestrator/reorg_handler_test.go(24 hunks)internal/rpc/batcher.go(4 hunks)internal/rpc/rpc.go(7 hunks)internal/worker/worker.go(5 hunks)test/mocks/MockIRPCClient.go(14 hunks)
🚧 Files skipped from review as they are similar to previous changes (11)
- internal/orchestrator/committer_test.go
- internal/orchestrator/failure_recoverer.go
- internal/orchestrator/chain_tracker.go
- internal/orchestrator/poller.go
- internal/worker/worker.go
- internal/orchestrator/reorg_handler_test.go
- internal/rpc/batcher.go
- internal/orchestrator/reorg_handler.go
- internal/rpc/rpc.go
- internal/orchestrator/committer.go
- test/mocks/MockIRPCClient.go
🔇 Additional comments (1)
internal/handlers/search_handlers.go (1)
4-4: Import context package for RPC client context support.The addition of the context package is necessary to support the context parameters in RPC client method calls, which is part of the broader context propagation changes in this PR.
iuwqyir
force-pushed
the
05-20-pass_context_to_rpc_and_worker
branch
from
3b3058e to
dce6992
Compare
TL;DR
Added context propagation throughout the codebase to improve cancellation handling and resource management.
What changed?
This PR adds proper context propagation throughout the codebase by:
Adding context parameters to all RPC client interface methods:
GetFullBlocksGetBlocksGetTransactionsGetLatestBlockNumberUpdating all orchestrator components to pass context to RPC calls:
Propagating context through the worker package to ensure proper cancellation handling
Updating all batch RPC operations to respect context cancellation
How to test?
Why make this change?
This change improves resource management and application behavior by:
Summary by CodeRabbit
New Features
Bug Fixes
Tests