-
Notifications
You must be signed in to change notification settings - Fork 25k
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
Implement socket and server ChannelContexts #28275
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I really just left one question
private ChannelContext context; | ||
private BiConsumer<NioSocketChannel, Exception> exceptionContext; | ||
private final AtomicBoolean contextSet = new AtomicBoolean(false); | ||
private SocketChannelContext context; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why do we have a context here again? the superclass has it aready? Can't we use a getter that has the right type or a generic?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there any change we can untangle the cyclic dependencies here and pass the context in as a ctor argument? or can we pass it as method arguments instead and don't have it as a member?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I removed the duplicate contexts. Now each stances (NioServerSocketChannel
and NioSocketChannel
) has their own. I could store it in the super class parameterized. But AbstractNioChannel
is already parameterized once and it would get pretty big.
Probably not possible to change the cyclic relationship right now. Context needs the channel. So it cannot be a ctor argument without allowing this
to escape. Channel needs context as a field, as it is the hook into lower level selector operations. But going forward I have some ideas about how to continue to reduce this relationship.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cool lets move on with this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
* es/master: (38 commits) Build: Add pom generation to meta plugins (#28321) Add 6.3 version constant to master Minor improvements to translog docs (#28237) [Docs] Remove typo in painless-getting-started.asciidoc Build: Fix meta plugin usage in integ test clusters (#28307) Painless: Add spi jar that will be published for extending whitelists (#28302) mistyping in one of the highlighting examples comment -> content (#28139) Documents applicability of term query to range type (#28166) Build: Omit dependency licenses check for elasticsearch deps (#28304) Clean up commits when global checkpoint advanced (#28140) Implement socket and server ChannelContexts (#28275) Plugins: Fix meta plugins to install bundled plugins with their real name (#28285) Build: Fix meta plugin integ test installation (#28286) Modify Abstract transport tests to use impls (#28270) Fork Groovy compiler onto compile Java home [Docs] Update tophits-aggregation.asciidoc (#28273) Docs: match between snippet to its description (#28296) [TEST] fix RequestTests#testSearch in case search source is not set REST high-level client: remove index suffix from indices client method names (#28263) Fix simple_query_string on invalid input (#28219) ...
This commit is related to #27260. Currently have a channel context that
implements reading and writing logic for socket channels. Additionally,
we have exception contexts to handle exceptions. And accepting contexts
to handle accepted channels. This PR introduces a ChannelContext that
handles close and exception handling for all channel types.
Additionally, it has implementers that provide specific functionality
for socket channels (read and writing). And specific functionality for
server channels (accepting).