Hi Roger,
Thank you for reviewing, I have updated as you suggested:
http://cr.openjdk.java.net/~mli/8232446/webrev.01/
Thank you
-Hamlin
On 2019/11/18 11:48 PM, Roger Riggs wrote:
Hi Hamlin,
TCPConnection.java:212:
Keep the "close connection" logging and add the socket to the same log
message:
If anyone is scraping the log, they won't loose this message.
TCPTransport.tcpLog.log(Log.BRIEF, "close connection, socket: " +
socket);
TCPTransport.java
277-278: combine the message to be one logging call.
server socket
289: use Log.BRIEF, avoid creating mixture of and too many log levels.
Reword the log messages so they each begin with "server socket...",
or "server socket close"...
it makes it easier to grep for and coorelate related messages
Thanks, Roger
On 11/6/19 7:02 AM, Hamlin Li wrote:
On 2019/11/6 5:36 PM, Peter Levart wrote:
Hi Hamlin,
in TCPTransport.decrementExportCount():
283 try {
284 if (tcpLog.isLoggable(Log.BRIEF)) {
285 tcpLog.log(Log.BRIEF, "close server socket
on " + ss);
286 }
287 ss.close();
288 } catch (IOException e) {
289 }
...you could add a log statement to the catch block too. Or even
better, rearrange for IOException to be thrown from that method and
deal with it in two places:
- in exportObject() - add it as suppressed exception to exception
thrown from super.exportObject().
- in targetUnexported() - log it or wrap it into
UncheckedIOException (depending on what are the callers of
targetUnexported())
What do you think?
Thanks Peter.
I agree. I adopt your first suggestion: add log statement to catch
block, as I think it's simple/straight and sufficient to help diagnose.
And I also add log for catch blocks in other close places.
The change is updated in place at:
http://cr.openjdk.java.net/~mli/8232446/webrev.00/
Thank you
-Hamlin
Regards, Peter
On 11/6/19 3:07 AM, Hamlin Li wrote:
Would you please review the patch?
bug: https://bugs.openjdk.java.net/browse/JDK-8232446
webrev: http://cr.openjdk.java.net/~mli/8232446/webrev.00/
We have some intermittent failures in rmi related to socket
closing, this is to add more logging to help diagnose the issues.
Thanks you
-Hamlin
