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

tokio-epoll-uring: fallback to std-fs if not available & not explicitly requested #7120

Merged
merged 10 commits into from Mar 15, 2024

Conversation

problame
Copy link
Contributor

@problame problame commented Mar 14, 2024

fixes #7116

Changes:

  • refactor PageServerConfigBuilder: support not-set values
  • implement runtime feature test
  • use runtime feature test to determine virtual_file_io_engine if not explicitly configured in the config
  • log the effective engine at startup
  • drive-by: improve assertion messages in test_pageserver_init_node_id

This needed a tiny bit of tokio-epoll-uring work, hence bumping it.
Changelog:

    git log --no-decorate --oneline --reverse 868d2c42b5d54ca82fead6e8f2f233b69a540d3e..342ddd197a060a8354e8f11f4d12994419fff939
    c7a74c6 Bump mio from 0.8.8 to 0.8.11
    4df3466 Bump mio from 0.8.8 to 0.8.11 (#47)
    342ddd1 lifecycle: expose `LaunchResult` enum (#49)

Changelog

```
git log --no-decorate --oneline --reverse 868d2c42b5d54ca82fead6e8f2f233b69a540d3e..342ddd197a060a8354e8f11f4d12994419fff939
c7a74c6 Bump mio from 0.8.8 to 0.8.11
4df3466 Bump mio from 0.8.8 to 0.8.11 (#47)
342ddd1 lifecycle: expose `LaunchResult` enum (#49)
```
@problame problame requested a review from a team as a code owner March 14, 2024 09:24
@problame problame requested review from jcsp, skyzh and arpad-m and removed request for jcsp March 14, 2024 09:24
Copy link

github-actions bot commented Mar 14, 2024

2688 tests run: 2563 passed, 0 failed, 125 skipped (full report)


Code coverage* (full report)

  • functions: 28.4% (7069 of 24849 functions)
  • lines: 47.0% (43412 of 92417 lines)

* collected from Rust tests only


The comment gets automatically updated with the latest test results
3db15cd at 2024-03-15T17:51:16.155Z :recycle:

Copy link
Member

@arpad-m arpad-m left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok except typo

pageserver/src/virtual_file/io_engine.rs Outdated Show resolved Hide resolved
Co-authored-by: Arpad Müller <arpad-m@users.noreply.github.com>
@problame problame enabled auto-merge (squash) March 15, 2024 12:23
@arpad-m
Copy link
Member

arpad-m commented Mar 15, 2024

legit clippy failures:


  error: an implementation of `From` is preferred since it gives you `Into<_>` for free where the reverse isn't true
     --> pageserver/src/virtual_file/io_engine.rs:272:1
      |
  272 | impl Into<IoEngineKind> for FeatureTestResult {
      | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
      |
      = help: `impl From<Local> for Foreign` is allowed by the orphan rules, for more information see
              https://doc.rust-lang.org/reference/items/implementations.html#trait-implementation-coherence
      = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#from_over_into
      = note: `-D clippy::from-over-into` implied by `-D warnings`
      = help: to override `-D warnings` add `#[allow(clippy::from_over_into)]`
  help: replace the `Into` implementation with `From<virtual_file::io_engine::FeatureTestResult>`
      |
  272 ~ impl From<FeatureTestResult> for IoEngineKind {
  273 ~     fn from(val: FeatureTestResult) -> Self {
  274 ~         match val {
      |
  
  error: unneeded late initialization
     --> pageserver/src/virtual_file/io_engine.rs:305:17
      |
  305 |                 let remark;
      |                 ^^^^^^^^^^^
      |
      = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_late_init
      = note: `-D clippy::needless-late-init` implied by `-D warnings`
      = help: to override `-D warnings` add `#[allow(clippy::needless_late_init)]`
  help: declare `remark` here
      |
  306 |                 let remark = match e.raw_os_error() {
      |                 ++++++++++++
  help: remove the assignments from the `match` arms
      |
  308 ~                         "creating tokio-epoll-uring failed with non-OS error: {e}"
  309 |                     }
  310 |                     Some(nix::libc::EFAULT) => {
  311 ~                         "tokio-epoll-uring crate produces EFAULT?"
  312 |                     }
  313 |                     Some(nix::libc::EPERM) => {
  314 ~                         "creating io_uring fails with EPERM"
      |
  help: add a semicolon after the `match` expression
      |
  321 |                 };
      |                  +

…ability-detection' into problame/tokio-epoll-uring-availability-detection
…ability-detection' into problame/tokio-epoll-uring-availability-detection
@problame problame merged commit 60f3000 into main Mar 15, 2024
52 checks passed
@problame problame deleted the problame/tokio-epoll-uring-availability-detection branch March 15, 2024 17:46
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.

auto regress to std fs backend if io_uring is not usable on Linux
4 participants