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.

Sign up for GitHub

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

Jump to bottom

ISPN-12882 Test for disabled ciphersuites #9176

Merged
merged 1 commit into from Mar 31, 2021
Merged

ISPN-12882 Test for disabled ciphersuites #9176

merged 1 commit into from Mar 31, 2021

Conversation

tristantarrant
Copy link
Member

@tristantarrant
Copy link
Member Author

Some related failures

@tristantarrant
Copy link
Member Author

Fixed

@tristantarrant tristantarrant force-pushed the ISPN-12882/ssl_engine_config branch 2 times, most recently from 5f898dd to 9a7384a Compare March 30, 2021 06:47
tristantarrant
tristantarrant commented Mar 30, 2021
View reviewed changes
...d-client/src/main/java/org/infinispan/client/hotrod/impl/transport/netty/ChannelFactory.java
@@ -282,6 +282,7 @@ private SocketAddress getNextServer(Set<SocketAddress> failedServers, byte[] cac
if (failedServers != null && failedServers.containsAll(getServers(cacheName))) {
log.debug("All the servers are marked as failed. Cluster might have completely shut down, try reverting to the initial server list.");
reset(cacheName);
closeChannelPools(failedServers);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@danberindei I've added this line, which ensures that the pool map is properly cleaned up

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tristantarrant reset calls updateServers, which calls updateTopologyInfo, which already does this (that's where the original code of closeChannelPools is from). So I'm not sure how this extra call changes anything.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I stand corrected @tristantarrant, I didn't realize failedServers means something else in updateTopologyInfo

@tristantarrant tristantarrant force-pushed the ISPN-12882/ssl_engine_config branch 2 times, most recently from 14a62b7 to f740ac2 Compare March 30, 2021 13:49
@tristantarrant
Copy link
Member Author

@danberindei @wburns let's fix the double decrement in https://issues.redhat.com/browse/ISPN-12902
In the meantime the changes I've introduced here resolve the issue and don't cause regressions.

@oraNod oraNod self-requested a review March 31, 2021 14:52
oraNod
oraNod approved these changes Mar 31, 2021
View reviewed changes
Copy link
Contributor

@oraNod oraNod left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 😎

@danberindei danberindei merged commit dc2403a into infinispan:master Mar 31, 2021
@danberindei
Copy link
Member

Merged, thanks Tristan!

@tristantarrant tristantarrant deleted the ISPN-12882/ssl_engine_config branch April 1, 2021 12:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@danberindei danberindei danberindei approved these changes

@oraNod oraNod oraNod approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
3 participants
@tristantarrant @danberindei @oraNod