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 to specify a EventLoopTaskQueueFactory for various EventLoopGro… #9247

Merged
merged 1 commit into from Jun 21, 2019

Conversation

Projects
None yet
5 participants
@normanmaurer
Copy link
Member

commented Jun 17, 2019

…up implementations

Motivation:

Sometimes it is desirable to be able to use a different Queue implementation for the EventLoop of a Channel. This is currently not possible without resort to reflection.

Modifications:

  • Add a new constructor to Nio|Epoll|KQueueEventLoopGroup which allows to specify a factory which is used to create the task queue. This was the user can override the default implementation.
  • Add test

Result:

Be able to change Queue that is used for the EventLoop.

@normanmaurer

This comment has been minimized.

Copy link
Member Author

commented Jun 17, 2019

/cc @iamaleksey and @belliottsmith as this is something that Cassandra needs.

@normanmaurer

This comment has been minimized.

Copy link
Member Author

commented Jun 17, 2019

Also please keep in mind that this is only needed for 4.1 as master already provides way to override this.

@normanmaurer normanmaurer added this to the 4.1.37.Final milestone Jun 17, 2019

@njhill

This comment has been minimized.

Copy link
Member

commented Jun 17, 2019

@normanmaurer this looks fine to me but I think it would be similarly possible just by making Nio|Epoll|KQueueEventLoops non-final/public? Presumably this approach is to avoid having to do that?

@normanmaurer

This comment has been minimized.

Copy link
Member Author

commented Jun 17, 2019

@njhill yes...

@carl-mastrangelo
Copy link
Member

left a comment

LGTM

@njhill

njhill approved these changes Jun 17, 2019

@normanmaurer

This comment has been minimized.

Copy link
Member Author

commented Jun 17, 2019

@iamaleksey can you please verify this gives you what you need and let me know ?

@iamaleksey

This comment has been minimized.

Copy link
Contributor

commented Jun 17, 2019

@normanmaurer Thanks, this is perfect 👍 Will replace our reflection work around with queue factory constructors as soon as 4.1.37 is out.

Allow to specify a EventLoopTaskQueueFactory for various EventLoopGro…
…up implementations

Motivation:

Sometimes it is desirable to be able to use a different Queue implementation for the EventLoop of a Channel. This is currently not possible without resort to reflection.

Modifications:

- Add a new constructor to Nio|Epoll|KQueueEventLoopGroup which allows to specify a factory which is used to create the task queue. This was the user can override the default implementation.
- Add test

Result:

Be able to change Queue that is used for the EventLoop.

@normanmaurer normanmaurer force-pushed the pluggable_queue branch from 2688d36 to a316235 Jun 19, 2019

@normanmaurer

This comment has been minimized.

Copy link
Member Author

commented Jun 20, 2019

@netty-bot test this please

@normanmaurer normanmaurer merged commit c9aaa93 into 4.1 Jun 21, 2019

3 checks passed

pull request validation (centos6-java11) Build finished.
Details
pull request validation (centos6-java12) Build finished.
Details
pull request validation (centos6-java8) Build finished.
Details

@normanmaurer normanmaurer deleted the pluggable_queue branch Jun 21, 2019

@normanmaurer

This comment has been minimized.

Copy link
Member Author

commented Jun 21, 2019

@iamaleksey will be included in the next release 🎉

@iamaleksey

This comment has been minimized.

Copy link
Contributor

commented Jun 21, 2019

@normanmaurer thanks, man. Appreciated.

@njhill

This comment has been minimized.

Copy link
Member

commented Jun 21, 2019

@normanmaurer sorry for the belated comment (just noticed) ... curious why EventLoopTaskQueueFactory was added in MultithreadEventLoopGroup which seems unrelated. Why not just top-level interface in the same package?

@normanmaurer

This comment has been minimized.

Copy link
Member Author

commented Jun 22, 2019

@njhill sure why not... Let me move it.

@normanmaurer

This comment has been minimized.

Copy link
Member Author

commented Jun 22, 2019

@njhill 517a93d

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.