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

JAMES-3715 Small cleanups following Netty 4 migration #899

Closed
wants to merge 6 commits into from

Conversation

chibenwa
Copy link
Contributor

Cc @ff-wl I'm interested by your feedback...

Playing around with the code resulting from the Netty 4 migration ( #886 ) I realized:

  • That the threading model was further simplified than I thought (which is good!) and that we could get rid of a few configuration options and of a few extra classes (yeah!)
  • That some ssl related options were leftover
  • And that the ioWorker option was no longer applied while we easily could.

@ff-wl feel free to cherry-pick the commits.

@ff-wl
Copy link
Contributor

ff-wl commented Mar 1, 2022

Cc @ff-wl I'm interested by your feedback...

Playing around with the code resulting from the Netty 4 migration ( #886 ) I realized:

  • That the threading model was further simplified than I thought (which is good!) and that we could get rid of a few configuration options and of a few extra classes (yeah!)
  • That some ssl related options were leftover
  • And that the ioWorker option was no longer applied while we easily could.

@ff-wl feel free to cherry-pick the commits.

looks good for me. I've cherry-picked all commits and will push them to the #886 branch.

@chibenwa
Copy link
Contributor Author

chibenwa commented Mar 1, 2022

My bad f7f0c18 should be removed from here: it is a mistake... Sorry...

chibenwa added 5 commits March 1, 2022 18:15
Before this parameter was ignored, we can use it to size Netty server
child executor instead.
Netty 4 no longer allows setting internal threads and instead relies
on event loop groups.

Removing unused methods...
This is dead code since the Netty 4 migration. Remove.

We now have easier to exploit metrics (JMX, HTTP endpoint, logs, ES reportings)
that expose more interesting applicative level metrics.
…ExecutorThreads

Computation are now conducted on the child threads of the Netty4 Server bootstraps.

On Netty 3 those computations were conducted on a thread pool yet separated
from the Netty 'workers' which could be seen as unnecessary context switches -
Netty worker threads were merely decoding and encoding the traffic.
@chibenwa
Copy link
Contributor Author

chibenwa commented Mar 1, 2022

Integrated in #886

@chibenwa chibenwa closed this Mar 1, 2022
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.

2 participants