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

[Hamlin Li]

Hi Roger,

Thanks for reviewing, I have updated as you suggested: 
http://cr.openjdk.java.net/~mli/8232446/webrev.01/

Thank you

-Hamlin

On 2019/11/27 2:47 AM, Roger Riggs wrote:
> 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
>>>>>>
>>>>>
>>>
>