Skip to content
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

Merged
merged 5 commits into from Mar 26, 2024

Conversation

untitaker
Copy link
Member

@untitaker untitaker commented Mar 25, 2024

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.

@untitaker untitaker requested review from a team as code owners March 25, 2024 11:28
@untitaker untitaker changed the title ref/clickhouse streaming write ref: Initial refactor to facilitate streaming writes to clickhouse Mar 25, 2024
Copy link

codecov bot commented Mar 25, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 89.92%. Comparing base (4ff3424) to head (2fe578a).
Report is 28 commits behind head on master.

✅ 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.
📢 Have feedback on the report? Share it here.

@@ -141,90 +137,3 @@ impl ProcessingStrategy<BytesInsertBatch> for ClickhouseWriterStep {
self.inner.join(timeout)
}
}

#[derive(Clone)]
Copy link
Member

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?

Copy link
Member Author

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

@untitaker untitaker merged commit 28323fc into master Mar 26, 2024
32 checks passed
@untitaker untitaker deleted the ref/clickhouse-streaming-write branch March 26, 2024 15:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants