[c++] Add bounded RecordBatch log reader APIs - #3988
Conversation
|
@fresh-borzoni @charlesdong1991 @leekeiabstraction Appreciate a review here when you have time 🙏 |
leekeiabstraction
left a comment
There was a problem hiding this comment.
TY for the PR, added a couple of comments. PTAL.
| auto ffi_result = reader_->record_batch_log_reader_next_batch(timeout_ms); | ||
| auto result = utils::from_ffi_result(ffi_result.result); | ||
| if (!result.Ok()) { | ||
| return result; |
There was a problem hiding this comment.
When FFI call fails, we return with out.status still set to TimedOut, this will cause caller following documentation to retry forever on unretriable errors.
There was a problem hiding this comment.
Fixed. If the FFI call fails, NextBatch() now leaves out as Finished with no batch instead of TimedOut. This prevents callers from mistaking an actual error for a timeout and retrying forever. Callers should still check the returned Result before using out.
| /// Drains all remaining batches until every stopping offset is reached. | ||
| Result CollectAllBatches(ArrowRecordBatches& out); |
There was a problem hiding this comment.
This doesn't have a timeout arg, what happens if it is an error such as a tablet server outage? Will this hang caller forever?
There was a problem hiding this comment.
Fixed. CollectAllBatches(timeout_ms, out) now uses timeout_ms as the time limit for the entire collection. If the timeout expires before all stopping offsets are reached, it stops and returns REQUEST_TIME_OUT; any complete batches already collected remain in out. Only Ok() means the result is complete. The documentation, examples, and tests have been updated accordingly.
|
Thanks @leekeiabstraction for the thorough review! Both comments have been addressed and the updates have been pushed. PTAL when you have a chance. |
Purpose
Linked issue: close #3987
Expose bounded
RecordBatchLogReaderAPIs through the C++ binding.API example
The API also supports:
TimedOutallows callers to check cancellation or deadlines before retrying.Finishedmeans all assigned buckets have reached their stopping offsets.Tests
cargo test -p fluss-rs --lib client::table::readercargo test -p fluss-cpp --libcargo clippy -p fluss-rs -p fluss-cpp --lib -- -D warningscargo fmt --checktimestamp ranges, and partitioned tables
API and Format
This PR adds backward-compatible public C++ APIs.
Documentation
Updated the C++ README, API reference, log table guide, and example program.