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

Async IPC Reader/Writer #1207

Closed
tustvold opened this issue Jan 19, 2022 · 4 comments · Fixed by #5531
Closed

Async IPC Reader/Writer #1207

tustvold opened this issue Jan 19, 2022 · 4 comments · Fixed by #5531
Labels
enhancement Any new improvement worthy of a entry in the changelog

Comments

@tustvold
Copy link
Contributor

Is your feature request related to a problem or challenge? Please describe what you are trying to do.

The current IPCReader and IPCWriter require a sync Reader/Writer complicating their use in async systems such as DataFusion. Once such use case can be found in apache/datafusion#1596 where it is used for spilling buffers

Describe the solution you'd like

The encoding/decoding logic to/from Flatbuffers is already largely broken out, it should be a relatively straightforward task of writing an async muxer for this. Ultimately I'm not aware of a way to avoid reading/writing the flatbuffer to an intermediate buffer first, and so I don't think it is possible to do better than this - e.g. by having the codecs read/write directly to/from the file.

@tustvold tustvold added the enhancement Any new improvement worthy of a entry in the changelog label Jan 19, 2022
@trueleo
Copy link
Contributor

trueleo commented Jul 13, 2023

@tustvold I was going to create a similar issue but found this. Is something related to flatbuffer serialization blocking IPC streamwriter and filewriter from being async.

Looking at the code it feels like operations can be made async by switching out std::io with tokio::io. If it is actually that trivial to make the change then probably the next question is about how or if arrow-ipc crate should provides both async and sync readers/writers.

I will create a seperate issue if this issue is not related to arrow-ipc filewriter/streamwriter

@tustvold
Copy link
Contributor Author

Looking at the code it feels like operations can be made async by switching out std::io with tokio::io

For writing it should simply be a case of taking the output of IpcDataGenerator::encoded_batch and feeding the result to the appropriate async IO primitive.

The read side will be slightly more complicated, but as you say is large similar to the current sync approach. I think the hard part will be avoiding large amounts of code duplication, which will likely require splitting out various utility functions to contain the common logic

@AdamGS
Copy link
Contributor

AdamGS commented Mar 1, 2024

I might pick this work up because I want to play around with IPC files a bit and most of the surrounding is already async. Any thoughts about module structure? In my mind there are two alternatives:

  1. Adding a new sync module for the existing reader/writer and a feature-flagged async module, I think that's more elegant with shorter use statements but its a breaking change.
  2. using the similar structure to parquet where the async reader/writer each have their own module and both are behind the async feature flag.

@tustvold
Copy link
Contributor Author

tustvold commented Mar 1, 2024

I think #5249 is largely all that is needed for the read side, I think all that remains is showing how it can be hooked to to object_store or similar.

Similarly the write side is much the same, we already provide free functions for encoding batches

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Any new improvement worthy of a entry in the changelog
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants