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

[Roger Riggs]

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
>>>>>>>
>>>>>>
>>>>
>>