Proper shutdown + clickhouse timeouts cfg#174
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
This PR improves the shutdown process by implementing proper task cancellation with CancellationToken and waiting for critical tasks using JoinHandles. It also adds configurable timeout parameters for ClickHouse HTTP requests.
Changes:
- Removed the
runnermodule and moved its functionality directly intomain.rs - Added
CancellationTokento gracefully cancel infinite loops andJoinHandles to wait for critical tasks during shutdown - Added two new CLI parameters (
send-timeout-msandend-timeout-ms) to configure ClickHouse timeouts
Reviewed changes
Copilot reviewed 13 out of 14 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/runner/mod.rs | Removed entire runner module |
| src/main.rs | Moved runner functionality here and implemented improved shutdown with cancellation tokens and task waiting |
| src/lib.rs | Updated function signatures to accept TaskExecutor, IndexerHandle, and CancellationToken instead of CliContext |
| src/utils.rs | Added wait_for_critical_tasks helper function and SHUTDOWN_TIMEOUT constant |
| src/indexer/mod.rs | Modified to return JoinHandles for proper shutdown waiting |
| src/indexer/click/mod.rs | Updated to use new timeout parameters and return join handles |
| src/ingress/mod.rs | Added cancellation token support to listen method |
| src/cli.rs | Added send_timeout_ms and end_timeout_ms configuration parameters |
| tests/common/mod.rs | Updated test helper to accept TaskManager parameter |
| tests/ingress.rs | Updated test to create and pass TaskManager |
| tests/network.rs | Updated test to create and pass TaskManager, changed IPv6 addresses to IPv4 |
| src/indexer/click/models.rs | Removed unnecessary .clone() call |
| Cargo.toml | Updated rbuilder dependencies and added tokio-util |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
mempirate
approved these changes
Feb 5, 2026
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
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.
This PR tries to improve a little the shutdown by properly waiting critical tasks with JoinHandles and cancelling infinite loops (locking resourses) with a CancellationToken.
It also add 2 new parameters indexer.clickhouse.send-timeout-ms/indexer.clickhouse.end-timeout-ms to configure clickhouse timeouts.