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

Revisit IoDispatcher #1670

Open
a10y opened this issue Dec 12, 2024 · 2 comments
Open

Revisit IoDispatcher #1670

a10y opened this issue Dec 12, 2024 · 2 comments

Comments

@a10y
Copy link
Contributor

a10y commented Dec 12, 2024

Capturing convo with @AdamGS. We are forcing all users of the vortex-io crate to take the overhead of dispatching the IO requests, even if they are already running in a Tokio runtime context.

We could change the way we do IO to make it more ergonomic for users that are just using Tokio already, for example we could force all of our read futures to be Send and simply spawn tasks when a Tokio runtime is already installed. A potential strawman new trait might look like this:

pub trait IO {
  // We need to choose if all futures are Send or !Send, and the only way to make this work without
  // assuming always having an external dispatcher is to force them all to be Send
  fn read_bytes(&self, range: Range<usize>) -> impl Future<Output = Bytes> + Send + Sync + 'static;
}

// We have a non-dispatched impl for tokio files
struct LocalTokioFile(File);

impl IO for LocalTokioFile {
  async fn read_bytes(&self, range: Range<usize>) -> Bytes {
    ...
  }
}

struct TokioFileDispatched {
  // We submit a request (byte range) as well as a handle for how the remote thread
  // can send us back a result (the Bytes read)
  submitter: Sender<(Range<usize>, Sender<Bytes>)>,
}

impl IO for TokioFileDispatched {
  fn read_bytes(&self, range: Range<usize>) -> impl Future<Output = Bytes> {
    // Run on the remote thread.
    let (tx, rx) = oneshot::channel();
    submitter.submit((range, tx));
    return rx;
  }
}

You'd have separate IO implementations for a source that controls how the futures are launched, allowing you to specialize sending them to a remote runtime or launching them immediately.

@robert3005
Copy link
Member

Even if you already have a tokio runtime you likely don’t want to mix your cpu and io workloads

@AdamGS
Copy link
Contributor

AdamGS commented Dec 13, 2024

As you know, I believe that's true, but the current design doesn't allow you to to have fine grained control over that, you HAVE to spin up an additional runtime, even you already architected your system to use multiple runtimes.

Its also much harder to extend the Dispatcher trait than a more IO focused trait, forcing users into caring about a whole new problem space.

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

No branches or pull requests

3 participants