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

[Hamlin Li]

Hi Roger,

Thanks for reviewing.

Thank you

-Hamlin

On 2019/11/27 11:09 PM, Roger Riggs wrote:
> Looks good.
>
> Thanks for the revisions,  Roger
>
> On 11/26/19 8:51 PM, Hamlin Li wrote:
>>
>> 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
>>>>>>>>
>>>>>>>
>>>>>
>>>
>