Join GitHub today
GitHub is home to over 20 million developers working together to host and review code, manage projects, and build software together.
=rem #23023 log reasons for disassociations with debug level #23116
Conversation
akka-ci
added validating needs-attention and removed validating
labels
Jun 7, 2017
|
Test FAILed. |
| // Expected handshake to be finished, dropping connection | ||
| + if (log.isDebugEnabled) | ||
| + log.debug("Sending disassociate to {} because unexpected message was received during handshake {}", wrappedHandle, msg) |
patriknw
Jun 7, 2017
Owner
I think we should avoid full toString of messages, also on debug level, to not risk bad performance (or even OOME). Use the class name.
| @@ -261,7 +263,16 @@ trait AssociationHandle { | ||
| * could be called arbitrarily many times. | ||
| * | ||
| */ | ||
| + @deprecated |
patriknw
Jun 7, 2017
Owner
It's fine to deprecate it, but include version and comment as in other places
|
Adressed review feedback. |
jrudolph
added the
to-be-backported
label
Jun 14, 2017
akka-ci
added validating needs-attention and removed needs-attention validating
labels
Jun 14, 2017
|
Test FAILed. |
akka-ci
added validating needs-attention and removed needs-attention validating
labels
Jun 14, 2017
|
Test FAILed. |
|
I added a mima filter. Notably, custom transport implementations compiled against old versions of akka-remote will fail to run with the new version. I don't think that's a likely scenario. Do we know about such a custom implementation for which that could be a problem? |
akka-ci
added validating tested and removed needs-attention validating
labels
Jun 14, 2017
|
Test PASSed. |
|
custom transports are not encouraged, so I have no problem with breaking that :) |
jrudolph commentedJun 7, 2017
•
edited
I deprecated the old
disassociatemethod for now but that might not be reasonable given that remoting classes are public API / SPI.Refs #23023