GH-43631: [C++] Add C++ implementation of Async C Data Interface #44495
Conversation
github-actions
bot
added
Component: C++
Component: Documentation
awaiting committer review
| }; | ||
|
|
||
| namespace internal { | ||
| class Executor; |
There was a problem hiding this comment.
This really needs to not be internal at some point
There was a problem hiding this comment.
Can we add a test that also propagates errors? (Both: before the schema, and after the schema)
There was a problem hiding this comment.
added tests to propagate errors
github-actions
bot
added
awaiting changes
| Status operator()(const std::shared_ptr<RecordBatch>& record) { | ||
| std::unique_lock<std::mutex> lock(state_->mutex_); | ||
| if (state_->pending_requests_ == 0) { | ||
| state_->cv_.wait(lock, [this]() -> bool { |
There was a problem hiding this comment.
Are we supposed to have a blocking call in consuming an AsyncGenerator?
There was a problem hiding this comment.
I honestly couldn't figure out a better way around this to manage the backpressure from the calls to request. I'm open to suggestions for a better route here other than simply saying this should not be run on the main thread shrug
There was a problem hiding this comment.
I suppose the way it "should" work is that we never call into the AsyncGenerator unless we have a pending request? But we'd then need to handle more of the control flow here ourselves
github-actions
bot
added
awaiting changes
github-actions
bot
added
awaiting change review
github-actions
bot
added
awaiting change review
Co-authored-by: David Li <[email protected]>
Co-authored-by: David Li <[email protected]>
zeroshade
force-pushed
the
async-interface-implementation
branch
from
2ebbbb0
to
85178f7
Compare
Rationale for this change
Building on #43632 which created the Async C Data Structures, this adds functions to
bridge.h/bridge.ccto implement helpers for managing the Async C Data interfacesWhat changes are included in this PR?
Two functions added to bridge.h:
CreateAsyncDeviceStreamHandlerpopulates aArrowAsyncDeviceStreamHandlerand anExecutorto provide a future that resolves to anAsyncRecordBatchGeneratorto produce record batches as they are pushed asynchronously. TheArrowAsyncDeviceStreamHandlercan then be passed to any asynchronous producer.ExportAsyncRecordBatchReadertakes a record batch generator and a schema, along with anArrowAsyncDeviceStreamHandlerto use for calling the callbacks to push data as it is available from the generator.Are these changes tested?
Unit tests are added (currently only one test, more tests to be added)
Are there any user-facing changes?
No