upgrade clickhouse library to fix EOF errors #219
Conversation
WalkthroughThe changes update the Go module file to specify a more precise Go version and upgrade several dependencies to newer versions. Additionally, in the ClickHouse storage implementation, a deferred call to close batch resources is added after batch preparation in multiple batch-insertion methods to ensure proper resource management. Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant ClickHouseStorage
participant ClickHouseDB
Caller->>ClickHouseStorage: insertBlocks/insertTransactions/etc.
ClickHouseStorage->>ClickHouseDB: Prepare batch
Note right of ClickHouseStorage: defer batch.Close()
ClickHouseStorage->>ClickHouseDB: Insert data into batch
ClickHouseStorage->>ClickHouseDB: Send batch
ClickHouseDB-->>ClickHouseStorage: Acknowledge
ClickHouseStorage-->>Caller: Return result
Note right of ClickHouseStorage: batch.Close() is called automatically on function exit
✨ 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 (
|
This stack of pull requests is managed by Graphite. Learn more about stacking. |
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
iuwqyir
force-pushed
the
06-04-log_metrics_around_processing_durations
branch
2 times, most recently
from
a4ab66f to
65f8943
Compare
iuwqyir
force-pushed
the
06-04-upgrade_clickhouse_library_to_fix_eof_errors
branch
from
b4cffd0 to
db1561e
Compare
iuwqyir
changed the base branch from
06-04-log_metrics_around_processing_durations
to
graphite-base/219
iuwqyir
force-pushed
the
06-04-upgrade_clickhouse_library_to_fix_eof_errors
branch
from
db1561e to
b2efc84
Compare
iuwqyir
force-pushed
the
06-04-upgrade_clickhouse_library_to_fix_eof_errors
branch
from
b2efc84 to
7a61163
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: 1
🧹 Nitpick comments (2)
go.mod (2)
3-3: Pin specific Go version: Specifyinggo 1.23.0helps maintain consistency across toolchains. Ensure your CI and development environments support Go 1.23.0.
107-113: Auditgolang.org/xupgrades: Multiplegolang.org/x/*modules were updated. Rungo mod tidy, recompile, and execute full test suite to detect any compatibility issues or deprecations.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (2)
go.mod(4 hunks)internal/storage/clickhouse.go(9 hunks)
🔇 Additional comments (4)
go.mod (4)
6-6: Verify ClickHouse client upgrade: Bumpinggithub.com/ClickHouse/clickhouse-go/v2to v2.36.0 may include breaking changes or new defaults. Review the release notes and run integration tests to catch any surfacing issues.
22-22: Validatech-gobump: Upgradinggithub.com/ClickHouse/ch-goto v0.66.0 should be vetted against your existing code paths. Confirm no deprecated APIs or altered behaviors.
64-67: Review auxiliary dependencies: Indirect bumps forklauspost/compress(v1.18.0) andmagiconair/properties(v1.8.10) may affect compression or configuration parsing. Verify no regressions in performance or behavior.
102-104: Ensure OpenTelemetry alignment: Upgradinggo.opentelemetry.io/oteland itstracepackage to v1.36.0 could introduce API changes. Re-run all tracing pipelines and validate telemetry emission.
| if err != nil { | ||
| return err | ||
| } | ||
| defer batch.Close() |
There was a problem hiding this comment.
Avoid deferring inside loops: Deferring batch.Close() in each iteration delays resource release until the function returns, risking high resource usage under load. Replace each defer batch.Close() with an explicit close immediately after batch.Send() and handle its error.
Also applies to: 216-216, 298-298, 362-362, 420-420, 883-883, 920-920, 985-985, 1189-1189
🤖 Prompt for AI Agents
In internal/storage/clickhouse.go at line 145, replace the deferred call to
batch.Close() inside the loop with an immediate call to batch.Close() right
after batch.Send(), and handle any error returned by Close(). Apply the same fix
to lines 216, 298, 362, 420, 883, 920, 985, and 1189 by removing defer
batch.Close() inside loops and closing batches explicitly after sending.
TL;DR
Updated dependencies in the Go module and added proper resource cleanup with
defer batch.Close()in ClickHouse operations.What changed?
Updated Go dependencies:
clickhouse-go/v2from v2.30.1 to v2.36.0 andch-gofrom v0.63.1 to v0.66.0klauspost/compress,magiconair/properties,opentelemetry, and multiple Golang packagesAdded proper resource cleanup in the ClickHouse connector:
defer batch.Close()after each batch creation in multiple methods to ensure proper cleanup of resourcesHow to test?
Why make this change?
defer batch.Close()prevents potential memory leaks and resource exhaustion during database operations, especially under high loadSummary by CodeRabbit
Chores
Bug Fixes