ClickHouse / ClickHouse/clickhouse-rs
RFC: Breaking `Inserter` API Changes
- Dominant language
- Rust
- Stars
- 559
- Forks
- 172
- Avg merge
- 1d 19h
- Merged PRs (30d)
- 3
Description
### Use case
#### Writing to `Inserter` from multiple threads
Applications that are trying to maximize client-side batching might want to buffer data from multiple threads simultaneously. As of now, they'd have to wrap `Inserter` in a mutex which would force all writes to be serial.
#### Async flushes
When the `period` elapses, `Inserter` has no way to flush the batch asynchronously, i.e. without the user calling `.write()` or `.commit()`. This is because the timeout checks and flushing are done directly in these methods.
Additionally, because these methods have to "block" on finishing the current `INSERT` statement and receiving a response, it's not possible to begin the next `INSERT` statement until we get a response for the previous one, resulting in a stall in ingestion rate (#41).
#### `.commit()` method naming is misleading
ClickHouse doesn't have transactions (not technically; there's a preview feature for transactions in Cloud but it's not generally available or recommended yet), and data is generally inserted as soon as it's received server-side. `.commit()` should perhaps more accurately be called `.flush()` because all it does is ensure the data in the buffer is sent to the server and finishes the current `INSERT` statement and underlying HTTP request.
#### Enabling retries
Currently, when an `INSERT` statement fails in the middle of writing data, it's impossible to tell how much data was successfully flushed to the server and the client-side buffer is completely discarded when it could be buffered and sent with the next request instead (#212).
### Describe the solution you'd like
Move the `Inserter` state machine to a background task and send data chunks via a channel. (Alternatively, some sort of ring buffer? I don't remember there being a good high-quality crate for ringbufs though.)
Since channels are generally MPSC, `.write()` can change to take `&self`.
Deprecate or just delete `.commit()` and `.force_commit()`. Add `.flush()` as a replacement for `.force_commit()`. Threshold-based committing/flushing can be done asynchronously on the background task without an explicit call.
_Maybe_ add a `.compress_buffer()` call (#233)?
### Describe the alternatives you've considered
### Additional context
Contributor guide
Research direction
Start by tracing Inserter and its write(), commit(), and force_commit() methods; the issue does not name files or tests. Review the existing state machine, timeout and flush behavior, and referenced issues #41, #212, and #233. Done would require an agreed API and background-task design, so this RFC needs maintainer direction before implementation.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- backend, databases
- Issue type
- Feature
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Quiet
- Clarity
- Needs clarification
- Newbie friendliness
- 25/100