Umesh Agashe commented on HBASE-19988:

Thanks [~appy] for digging into the code and posting your thoughts, comments.
{quote}Currently the comment says "// We will retry when other exceptions, but 
we should stop if we timeout ."
Should be updated with reasons why we break out for each type.
I agree, I have modified the comment and uploaded the new patch. Please see 
patch 002.

However I prefer current exception handling over suggested as the general 
guideline is to handle specific exception first and then generic exceptions. As 
for the log message, getRowLockInternal() itself logs following warning message:
2018-02-10 04:24:25,503 WARN [PutThread] regionserver.HRegion(5636): Thread 
interrupted waiting for lock on row: row0{code}
I think this message has details required for debugging without being too 
chatty. Additionally, with run time type checking code will look like:
if (isAtomic() or ioe instanceof IIOE or ioe instanceof TIOE) { throw ioe; 
As you can see, readability will suffer, if we identify additional exception/ 
condition for retrying the operation in future.

> HRegion#lockRowsAndBuildMiniBatch() is too chatty when interrupted while 
> waiting for a row lock
> -----------------------------------------------------------------------------------------------
>                 Key: HBASE-19988
>                 URL: https://issues.apache.org/jira/browse/HBASE-19988
>             Project: HBase
>          Issue Type: Improvement
>          Components: amv2
>    Affects Versions: 2.0.0-beta-1
>            Reporter: Umesh Agashe
>            Assignee: Umesh Agashe
>            Priority: Minor
>             Fix For: 2.0.0-beta-2
>         Attachments: hbase-19988.master.001.patch, 
> hbase-19988.master.001.patch, hbase-19988.master.002.patch
> See HBASE-19970, TestHRegionWithInMemoryFlush created 4.2g log file.

This message was sent by Atlassian JIRA

Reply via email to