New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ref: Initial refactor to facilitate streaming writes to clickhouse #5683
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
✅ All tests successful. No failed tests found Additional details and impacted files@@ Coverage Diff @@
## master #5683 +/- ##
==========================================
- Coverage 89.94% 89.92% -0.02%
==========================================
Files 895 898 +3
Lines 43591 43453 -138
Branches 299 299
==========================================
- Hits 39207 39077 -130
+ Misses 4342 4334 -8
Partials 42 42 ☔ View full report in Codecov by Sentry. |
@@ -141,90 +137,3 @@ impl ProcessingStrategy<BytesInsertBatch> for ClickhouseWriterStep { | |||
self.inner.join(timeout) | |||
} | |||
} | |||
|
|||
#[derive(Clone)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need a test that actually writes a batch to clickhouse?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
right now those tests get in the way IMO
We want to eventually stop buffering writes to clickhouse, and instead hold one long-running connection for the lifetime of a batch, streaming rows slowly in. This is already how Python works.
This initial refactor creates a new Batch type that holds onto the clickhouse client internally.