[TRAFODION-1988] Better Java exception handling in Trafodion - Part2 #559
Conversation
This time the focus is on hbasetmlib2 improvements
|
Check Test Started: https://jenkins.esgyn.com/job/Check-PR-master/834/ |
|
Test Passed. https://jenkins.esgyn.com/job/Check-PR-master/834/ |
|
jenkins, extra tests |
|
Check Test Started: https://jenkins.esgyn.com/job/ExtraTest-PR-master/13/ |
|
Extra Test Failed. https://jenkins.esgyn.com/job/ExtraTest-PR-master/13/ |
| } catch(InterruptedException ie) { | ||
| LOG.warn("Interrupting commitDDLLock.wait, but retrying ", ie); | ||
| loopBack = true; | ||
| } while (loopBack); |
There was a problem hiding this comment.
This doesn't look right to me. Does "try" have a looping semantic when followed by "while"? Or did you intend to code a do { } around the try/catch block? (I'm thinking that the "while (loopBack);" does nothing if loopBack is false but loops infinitely if loopBack is true.)
There was a problem hiding this comment.
Interesting. It is not clear why didn't Java compiler catch this issue. I am fixing the code to ignore InterruptedException.
Fixed as per the review comments
|
New Check Test Started: https://jenkins.esgyn.com/job/Check-PR-master/840/ |
|
Test Passed. https://jenkins.esgyn.com/job/Check-PR-master/840/ |
|
Looks good, Selva. |
|
Is this ready to merge? |
| boolean loopBack = false; | ||
| do { | ||
| try { | ||
| rt.join(); |
There was a problem hiding this comment.
Should there be a loopBack = false after rt.join in the case where an exception occurred but then the operation succeeded?
Changes as per the review comments
Changes as per the review comments
|
Check Test Started: https://jenkins.esgyn.com/job/Check-PR-master/879/ |
|
Test Failed. https://jenkins.esgyn.com/job/Check-PR-master/879/ |
|
jenkins, retest |
|
New Check Test Started: https://jenkins.esgyn.com/job/Check-PR-master/888/ |
|
Test Passed. https://jenkins.esgyn.com/job/Check-PR-master/888/ |
|
Are there any more review comments that I need to take care of? |
|
Looked at the 4 sql .java files . Changes look good . Think it's ready to merge . |
This time the focus is on hbasetmlib2 improvements