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
ISPN-7297 java.lang.IllegalStateException on TriangleAckInterceptor.java #4713
Conversation
//protect with the shared lock. | ||
//it was possible the topology change before the collector was created. the collector would be initializer | ||
//with an old members that would never send the ack back resulting in timeout. | ||
stateTransferLock.acquireSharedTopologyLock(); |
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 would prefer checking the topology again after the collector was created, and retry if it changed, to avoid the contention on the number of readers in the shared lock.
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.
on other hand, you are creating a Collector to be removed later. I'm not sure if it has any impact in performance
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.
so, should I change 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.
I'd change it, even though it probably doesn't matter much in the grand scheme of things.
@@ -777,7 +777,7 @@ private void sendToAll(ReplicableCommand rpcCommand, DeliverOrder deliverOrder) | |||
AnycastAddress anycastAddress = new AnycastAddress(); | |||
dispatcher.sendMessage(anycastAddress, buffer, options); | |||
} else { | |||
dispatcher.castMessage(null, buffer, options); | |||
dispatcher.castMessage(null, buffer, options.anycasting(false)); |
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.
Looks superfluous. the default is already false
. It actually made me think maybe we're setting it to true
earlier, and that we shouldn't do that :)
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 know. I just want to be sure in case if one of us in the future change the default for some reason.
Caused by a miss configuration in RequestOptions
@danberindei updated |
Integrated, thanks Pedro! |
Caused by a miss configuration in RequestOptions
https://issues.jboss.org/browse/ISPN-7297