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

Invoke channelAcquired callback on first time channel acquire #9093

Merged
merged 1 commit into from Apr 29, 2019

Conversation

Projects
None yet
3 participants
@divijvaidya
Copy link
Contributor

commented Apr 26, 2019

Motivation:

SimpleChannelPool provides ability to provide custom callbacks/handlers
on major events such as "channel acquired", "channel created" and
"channel released". In the current implementation, when a request to
acquire a channel is made for the first time, the internal channel pool
creates the channel lazily. This triggers the "channel created" callback
but does not invoke the "channel acquired" callback. This is contrary to
caller expectations who assumes that "channel acquired" will be invoked
at the end of every successful acquire call. It also leads to an
inconsistent API experience where the acquired callback is sometimes
invoked and sometimes it isn't depending on whether the internal
mechanism is creating a new channel or re-using an existing one.

Modifications:

  • Invoke acquired callback consistently even when creating a new channel
    and modify the tests to support this behaviour.
  • Replace deprecated LocalEventLoopGroup with recommended DefaultEventLoopGroup in tests

Result:

Consistent experience for the caller of acquire API. Every time they
call the API, the acquired callback will be invoked.

Divij Vaidya
Invoke channelAcquired callback on first time channel acquire
Motivation:

SimpleChannelPool provides ability to provide custom callbacks/handlers
on major events such as "channel acquired", "channel created" and
"channel released". In the current implementation, when a request to
acquire a channel is made for the first time, the internal channel pool
creates the channel lazily. This triggers the "channel created" callback
but does not invoke the "channel acquired" callback. This is contrary to
caller expectations who assumes that "channel acquired" will be invoked
at the end of every successful acquire call. It also leads to an
inconsistent API experience where the acquired callback is sometimes
invoked and sometimes it isn't depending on wheather the internal
mechanism is creating a new channel or re-using an existing one.

Modifications:

Invoke acquired callback consistenly even when creating a new channel
and modify the tests to support this behaviour

Result:

Consistent experience for the caller of acquire API. Every time they
call the API, the acquired callback will be invoked.
@netty-bot

This comment has been minimized.

Copy link

commented Apr 26, 2019

Can one of the admins verify this patch?

@normanmaurer

This comment has been minimized.

Copy link
Member

commented Apr 29, 2019

@divijvaidya can you please sign our ICLA and let me know once done:

https://netty.io/s/icla

@normanmaurer

This comment has been minimized.

Copy link
Member

commented Apr 29, 2019

@netty-bot test this please

@normanmaurer normanmaurer added this to the 4.1.36.Final milestone Apr 29, 2019

@normanmaurer

This comment has been minimized.

Copy link
Member

commented Apr 29, 2019

@divijvaidya good catch!

@divijvaidya

This comment has been minimized.

Copy link
Contributor Author

commented Apr 29, 2019

Thank you for the review. I had already signed the ICLA prior to submitting this PR. Nevertheless, I signed it again in case there was some error the last time.

@normanmaurer normanmaurer merged commit b9c4e17 into netty:4.1 Apr 29, 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

This comment has been minimized.

Copy link
Member

commented Apr 29, 2019

@divijvaidya thanks a lot!

@normanmaurer normanmaurer self-assigned this Apr 29, 2019

@normanmaurer normanmaurer added the defect label Apr 29, 2019

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.