RFR of JDK-8232446: logging enhancement for rmi when socket closed

[Roger Riggs]

Hi Hamlin,

ok, but it looses the logging of the connection close when the socket is 
null.
I intended to suggest that the logging happened before/outside the test
for socket != null.

if (TCPTransport.tcpLog.isLoggable(Log.BRIEF)) {
TCPTransport.tcpLog.log(Log.BRIEF,"close connection, socket: " + socket);
}if (socket != null) { Thanks, Roger

On 11/22/19 2:54 AM, Hamlin Li wrote:
> 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
>>>>>
>>>>
>>