Skip to content

[improve][broker] Allow to use io_uring instead of epoll - #18385

Merged
nodece merged 2 commits into
apache:masterfrom
coderzc:io_uring
Nov 10, 2022
Merged

[improve][broker] Allow to use io_uring instead of epoll#18385
nodece merged 2 commits into
apache:masterfrom
coderzc:io_uring

Conversation

@coderzc

@coderzc coderzc commented Nov 8, 2022

Copy link
Copy Markdown
Member

Motivation & Modifications

Allow users to enable IO_uring for BK network transport for broker/proxy/client, similar to apache/bookkeeper#3595

Documentation

  • doc
  • doc-required
  • doc-not-needed
  • doc-complete

Matching PR in forked repository

PR in forked repository: coderzc#28

@github-actions github-actions Bot added the doc-not-needed Your PR changes do not impact docs label Nov 8, 2022
@coderzc
coderzc force-pushed the io_uring branch 2 times, most recently from 8893ffa to c115cad Compare November 8, 2022 12:08
Comment thread pulsar-common/pom.xml
@codecov-commenter

codecov-commenter commented Nov 9, 2022

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 45.14%. Comparing base (27186a1) to head (775357f).
⚠️ Report is 3362 commits behind head on master.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #18385      +/-   ##
============================================
- Coverage     45.61%   45.14%   -0.48%     
+ Complexity    10728    10604     -124     
============================================
  Files           752      752              
  Lines         72521    72523       +2     
  Branches       7791     7791              
============================================
- Hits          33083    32738     -345     
- Misses        35769    36147     +378     
+ Partials       3669     3638      -31     
Flag Coverage Δ
unittests 45.14% <ø> (-0.48%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.
see 53 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@Slf4j
public class EventLoopUtil {

private static final String ENABLE_IO_URING = "enable.io_uring";

@tisonkun tisonkun Nov 9, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'm wondering whether we need a prefix here, or if it's the same as how netty does it.

That said, if it's a Pulsar specific property, name it as:

private static final String ENABLE_IO_URING_KEY = "pulsar.enableUring";

can avoid accidental conflicts.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Sorry for missing this comment. This is a good idea, so like: pulsar.enable.io_uring.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I learn more properties don't separate enable and the target with a .. But it can be a style preference :)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Good suggestion, I will push a new PR to improve it.

@nodece
nodece merged commit dd07241 into apache:master Nov 10, 2022
@coderzc

coderzc commented Nov 10, 2022

Copy link
Copy Markdown
Member Author

@nodece Please also help to add labels and milestone.

@nodece nodece added this to the 2.12.0 milestone Nov 10, 2022
@momo-jun momo-jun added doc-required Your PR changes impact docs and you will update later. and removed doc-not-needed Your PR changes do not impact docs labels Nov 16, 2022
@momo-jun

Copy link
Copy Markdown
Contributor

@coderzc Since 2.11 has been released, now it's good timing to start adding the docs for this feature to the next version of docs. Feel free to share your plans and anything I can help with. Thank you.

@momo-jun momo-jun added doc-complete Your PR changes impact docs and the related docs have been already added. and removed doc-required Your PR changes impact docs and you will update later. labels Feb 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/broker doc-complete Your PR changes impact docs and the related docs have been already added. ready-to-test

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants