[GitHub] [phoenix] gjacoby126 commented on a change in pull request #860: PHOENIX-6080: Add a check to Index Rebuild jobs to check region closi…

[GitBox <gi...@apache.org>]

gjacoby126 commented on a change in pull request #860:
URL: https://github.com/apache/phoenix/pull/860#discussion_r471829610



##########
File path: phoenix-core/src/main/java/org/apache/phoenix/coprocessor/IndexRebuildRegionScanner.java
##########
@@ -1301,11 +1301,24 @@ public boolean next(List<Cell> results) throws IOException {
         try {
             localScanner = getLocalScanner();
             if (localScanner == null) {
+                region.closeRegionOperation();
                 return false;
             }
             synchronized (localScanner) {
                 if (!shouldVerify()) {
                     skipped = true;
+                    region.closeRegionOperation();

Review comment:
       @swaroopak - do we need the region.closeRegionOperation() and localScanner.close() calls? They appear to already be called in the finally block, which will execute even if we return false early. And a finally block seems like a safer place for them so some other code path doesn't miss them in the future. 




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org