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

Allow specifying the desired IO selector backend for nio4r #2522

Merged
merged 9 commits into from
Jan 12, 2021

Conversation

jcmfernandes
Copy link
Contributor

@jcmfernandes jcmfernandes commented Jan 5, 2021

Description

nio4r will soon have experimental support for io_uring via libev. The brave might want to try it out :) regardless of that, allowing users to explicitly set the IO selector backend seems like a good idea to me.

Your checklist for this pull request

  • I have reviewed the guidelines for contributing to this repository.
  • I have added an entry to History.md if this PR fixes a bug or adds a feature. If it doesn't need an entry to HISTORY.md, I have added [changelog skip] or [ci skip] to the pull request title.
  • I have added appropriate tests if this PR fixes a bug or adds a feature.
  • My pull request is 100 lines added/removed or less so that it can be easily reviewed.
  • If this PR doesn't need tests (docs change), I added [ci skip] to the title of the PR.
  • If this closes any issues, I have added "Closes #issue" to the PR description or my commit messages.
  • I have updated the documentation accordingly.
  • All new and existing tests passed, including Rubocop.

nio4r will soon have experimental support for io_uring via libev. The brave
might want to try it out :) regardless of that, allowing users to explicitly
set the IO selector backend seems like a good idea to me.
@nateberkopec nateberkopec added feature waiting-for-changes Waiting on changes from the requestor labels Jan 6, 2021
@nateberkopec
Copy link
Member

Thanks! Left some comments, lmk what you think.

@jcmfernandes
Copy link
Contributor Author

Thanks! Left some comments, lmk what you think.

Thank you! I updated the docs.

Copy link
Member

@dentarg dentarg left a comment

Choose a reason for hiding this comment

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

Should we rescue the ArgumentError coming from nio4r and raise a better one?

It is not that clear that it is coming from nio4r:

$ grep io_selector_backend test/config/app.rb
io_selector_backend :ruby

Puma starting in single mode...
* Puma version: 5.1.1 (ruby 2.7.1-p83) ("At Your Service")
*  Min threads: 0
*  Max threads: 5
*  Environment: development
*          PID: 15846
* Listening on http://0.0.0.0:57461
bundler: failed to load command: bin/puma (bin/puma)
Traceback (most recent call last):
  23: from /Users/dentarg/.gem/ruby/2.7.1/bin/bundle:23:in `<main>'
  22: from /Users/dentarg/.gem/ruby/2.7.1/bin/bundle:23:in `load'
  21: from /Users/dentarg/.rubies/ruby-2.7.1/lib/ruby/gems/2.7.0/gems/bundler-2.2.4/exe/bundle:37:in `<top (required)>'
  20: from /Users/dentarg/.rubies/ruby-2.7.1/lib/ruby/site_ruby/2.7.0/bundler/friendly_errors.rb:130:in `with_friendly_errors'
  19: from /Users/dentarg/.rubies/ruby-2.7.1/lib/ruby/gems/2.7.0/gems/bundler-2.2.4/exe/bundle:49:in `block in <top (required)>'
  18: from /Users/dentarg/.rubies/ruby-2.7.1/lib/ruby/site_ruby/2.7.0/bundler/cli.rb:24:in `start'
  17: from /Users/dentarg/.rubies/ruby-2.7.1/lib/ruby/site_ruby/2.7.0/bundler/vendor/thor/lib/thor/base.rb:485:in `start'
  16: from /Users/dentarg/.rubies/ruby-2.7.1/lib/ruby/site_ruby/2.7.0/bundler/cli.rb:30:in `dispatch'
  15: from /Users/dentarg/.rubies/ruby-2.7.1/lib/ruby/site_ruby/2.7.0/bundler/vendor/thor/lib/thor.rb:392:in `dispatch'
  14: from /Users/dentarg/.rubies/ruby-2.7.1/lib/ruby/site_ruby/2.7.0/bundler/vendor/thor/lib/thor/invocation.rb:127:in `invoke_command'
  13: from /Users/dentarg/.rubies/ruby-2.7.1/lib/ruby/site_ruby/2.7.0/bundler/vendor/thor/lib/thor/command.rb:27:in `run'
  12: from /Users/dentarg/.rubies/ruby-2.7.1/lib/ruby/site_ruby/2.7.0/bundler/cli.rb:494:in `exec'
  11: from /Users/dentarg/.rubies/ruby-2.7.1/lib/ruby/site_ruby/2.7.0/bundler/cli/exec.rb:28:in `run'
  10: from /Users/dentarg/.rubies/ruby-2.7.1/lib/ruby/site_ruby/2.7.0/bundler/cli/exec.rb:63:in `kernel_load'
   9: from /Users/dentarg/.rubies/ruby-2.7.1/lib/ruby/site_ruby/2.7.0/bundler/cli/exec.rb:63:in `load'
   8: from bin/puma:10:in `<top (required)>'
   7: from /Users/dentarg/src/puma/lib/puma/cli.rb:80:in `run'
   6: from /Users/dentarg/src/puma/lib/puma/launcher.rb:181:in `run'
   5: from /Users/dentarg/src/puma/lib/puma/single.rb:53:in `run'
   4: from /Users/dentarg/src/puma/lib/puma/server.rb:241:in `run'
   3: from /Users/dentarg/src/puma/lib/puma/server.rb:241:in `new'
   2: from /Users/dentarg/src/puma/lib/puma/reactor.rb:23:in `initialize'
   1: from /Users/dentarg/src/puma/lib/puma/reactor.rb:23:in `new'
/Users/dentarg/src/puma/lib/puma/reactor.rb:23:in `initialize': unsupported backend: :ruby (ArgumentError)

lib/puma/dsl.rb Show resolved Hide resolved
@jcmfernandes
Copy link
Contributor Author

I hope I addressed all feedback, thank you! Let me know what you think.

@nateberkopec
Copy link
Member

Great work from everyone - @jcmfernandes and reviewers 👍

@nateberkopec nateberkopec merged commit 503b1cd into puma:master Jan 12, 2021
@jcmfernandes
Copy link
Contributor Author

Thank you @nateberkopec and all the reviewers! 🙌

JuanitoFatas pushed a commit to JuanitoFatas/puma that referenced this pull request Sep 9, 2022
* Allow specifying the desired IO selector backend

nio4r will soon have experimental support for io_uring via libev. The brave
might want to try it out :) regardless of that, allowing users to explicitly
set the IO selector backend seems like a good idea to me.

* Change History.md

* Add PR number

* Fix History.md

Oops!

* Update docs based on PR feedback

* Add test serving requests using a custom IO selector

* Document the NIO4R_PURE environment variable

* Raise clearer error when the requested backend is unsupported

* Simply boot the server and check the backend
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature waiting-for-changes Waiting on changes from the requestor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants