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

Rename IO backends to support Linux platforms without io_uring #628

Merged
merged 3 commits into from
Jan 7, 2025

Conversation

LtdJorge
Copy link
Contributor

@LtdJorge LtdJorge commented Jan 7, 2025

As was suggested in #502 I have renamed the IO backends from darwin to unix and from linux to io_uring. The unix backend is now available to Linux platforms without io_uring.

A few things to confirm

  • The cfg() blocks for target_os = "macos" now use target_family = "unix". If this is not wanted, it can be changed to target_os = "macos" and target_os = "linux" with an any()
  • These commits introduce a new feature io_uring but that is a choice that may not be liked.
  • The io_uring feature is on by default, which right now cannot be changed if building the root project. However, this matches the previous behavior of using io_uring on Linux. What is the path to let platforms without support disable the feature? Should a root level feature be defined?
  • I have run the tests with and without the feature and executed make successfully. However, I am on Linux and don't have access to a Mac, so someone would need to try that this doesn't break anything, although the changes are pretty simple. Oops, forgot about CI, that should take care of it.

On the topic of letting platforms disable io_uring support, the primary example is Google, which posted this blog in 2023 about disabling io_uring by default in most of their servers/platforms, including Android (prime candidate user for a SQLite rewrite).

Another way to implement compatibility for platforms with io_uring disabled, without using the feature flag, would be to make a Linux PlatformIO wrapper that implements IO and tries to build a UringIO. If that fails, it builds a UnixIO and returns that as the Arch<dyn IO>. This, however, would require changing

impl PlatformIO {
    pub fn new() -> Result<Self>{
    }
}

to

impl UringIO {
    pub fn new() -> Result<Box<dyn IO>> {
    }
}

and then

match limbo_core::PlatformIO::new() {
            Ok(io) => Arc::from(io),
}

Let me know what you think

@PThorpe92
Copy link
Contributor

👍 This was the right move for sure, considering the unfortunate situation with containers and io_uring

@penberg penberg merged commit 3a853ad into tursodatabase:main Jan 7, 2025
36 checks passed
@penberg
Copy link
Collaborator

penberg commented Jan 7, 2025

Hey @LtdJorge, thanks, I merged this because it's a nice improvement. I do think we should make the choice of I/O dynamically configurable on Linux where people can do limbo --io=uring and limbo --io=syscall, but also have the ability to disable io_uring altogether with a feature flag.

@LtdJorge
Copy link
Contributor Author

LtdJorge commented Jan 7, 2025

Hey @LtdJorge, thanks, I merged this because it's a nice improvement. I do think we should make the choice of I/O dynamically configurable on Linux where people can do limbo --io=uring and limbo --io=syscall, but also have the ability to disable io_uring altogether with a feature flag.

Great, what do you think about the wrapper idea I presented? Cause if it sounds good, I can start working on that. It would give the option of disabling io_uring at runtime, even if it was built with the feature, and we can pass --io=syscall argument directly to the wrapper to short circuit to the UnixIO backend.

@penberg
Copy link
Collaborator

penberg commented Jan 7, 2025

@LtdJorge The wrapper sounds reasonable!

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

Successfully merging this pull request may close these issues.

3 participants