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
Enabled ClientStateListener to be used on FailoverClient #19090
Enabled ClientStateListener to be used on FailoverClient #19090
Conversation
Due to nature of how failover client works, we don't allow different listeners to be registered at different client configs. Since ClientStateListener registers itself to the ClientConfig, this prevents it to be used via FailoverClient. The reasoning behind constructor register itself is to force the user to use this via config rather than registering it after client starts. So, sticking with the same decision we introduce another constructor which accepts ClientFailoverConfig and registers same listener(itself) to all the client configs. Also on the ClientStateListener I have ignored `CLIENT_CHANGED_CLUSTER`. This is rather a temporal event. We fire CLIENT_CONNECTED than CLIENT_CHANGED_CLUSTER, in those cases we want the current state to remain CLIENT_CONNECTED. fixes hazelcast#18351
public ClientStateListener(@Nonnull ClientFailoverConfig clientFailoverConfig) { | ||
List<ClientConfig> clientConfigs = clientFailoverConfig.getClientConfigs(); | ||
for (ClientConfig clientConfig : clientConfigs) { | ||
clientConfig.addListenerConfig(new ListenerConfig(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.
asking to be sure, I think the problem is resolved since the listeners registered to each config is the same object here, isn't it?
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.
Yes.
for (ClientConfig clientConfig : clientConfigs) { | ||
clientConfig.addListenerConfig(new ListenerConfig(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.
nit: you might delete this line
Due to nature of how failover client works, we don't allow different
listeners to be registered at different client configs.
Since ClientStateListener registers itself to the ClientConfig,
this prevents it to be used via FailoverClient.
The reasoning behind constructor register itself is to force the
user to use this via config rather than registering it after client
starts.
So, sticking with the same decision we introduce another constructor
which accepts ClientFailoverConfig and registers same listener(itself)
to all the client configs.
Also on the ClientStateListener I have ignored
CLIENT_CHANGED_CLUSTER
.This is rather a temporal event. We fire CLIENT_CONNECTED than
CLIENT_CHANGED_CLUSTER, in those cases we want the current state to
remain CLIENT_CONNECTED.
Fixes #18351
EE PR: https://github.com/hazelcast/hazelcast-enterprise/pull/4142
Checklist:
Team:
,Type:
,Source:
,Module:
) and Milestone setAdd to Release Notes
label if changes should be mentioned in release notes orNot Release Notes content
if changes are not relevant for release notes@Nonnull/@Nullable
annotations@since
tags in Javadoc