hbase-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From li...@apache.org
Subject svn commit: r1525275 - /hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV2.java
Date Sat, 21 Sep 2013 18:18:20 GMT
Author: liyin
Date: Sat Sep 21 18:18:20 2013
New Revision: 1525275

URL: http://svn.apache.org/r1525275
Log:
[HBASE-9102] Added finally block to unlock preloader lock in case of error.

Author: mahmoudeariby

Summary:
This diff adds finally blocks where we can unlock the scan preload lock in case of unexpected
failure.

Test Plan: Run unit tests.

Reviewers: liyintang, rshroff, manukranthk

Reviewed By: liyintang

CC: hbase-eng@

Differential Revision: https://phabricator.fb.com/D978149

Modified:
    hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV2.java

Modified: hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV2.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV2.java?rev=1525275&r1=1525274&r2=1525275&view=diff
==============================================================================
--- hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV2.java
(original)
+++ hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV2.java
Sat Sep 21 18:18:20 2013
@@ -625,7 +625,7 @@ public class HFileReaderV2 extends Abstr
       long lastRequestOffset;
       long seekBacks;
       long seekBackNonData;
-      long indexBlocksRead;
+      long nonDataBlocksRead;
       long inWrongPlaceCount;
       long emptyQueue;
       long ready;
@@ -634,10 +634,9 @@ public class HFileReaderV2 extends Abstr
         return "Ready = " + ready + ", Last requested offset = "
             + lastRequestOffset + ", Seek back count = " + seekBacks
             + ", Seek back for non data blocks = " + seekBackNonData
-            + ", Index blocks read = " + indexBlocksRead
+            + ", Non Data blocks read by preloader = " + nonDataBlocksRead
             + ", In a wrong place = " + inWrongPlaceCount
-            + ", Empty queue = " + emptyQueue
-            + ", Index blocks read by preloader " + indexBlocksRead;
+            + ", Empty queue = " + emptyQueue;
       }
     }
     
@@ -690,43 +689,43 @@ public class HFileReaderV2 extends Abstr
         public void run() {
           while (leftToPreload.get() > 0 && run) {
             lock.lock();
-            // double check in case we acquired the lock after being already stopped
-            if (!run) {
-              lock.unlock();
-              return;
-            }
-            HFileBlock block = null;
             try {
-              block =
-                  reader.readBlock(startOffset, startSize, cacheBlocks,
-                    isCompaction, ON_PRELOAD, null, preloaderKvContext);
-            } catch (Throwable e) {
-              // in case of ANY kind of error, we'll mark this block as attempted and let
the IPC
-              // Caller handler catch this exception
+              // double check in case we acquired the lock after being already stopped
+              if (!run) {
+                return;
+              }
+              HFileBlock block = null;
+              try {
+                block =
+                    reader.readBlock(startOffset, startSize, cacheBlocks,
+                      isCompaction, ON_PRELOAD, null, preloaderKvContext);
+              } catch (Throwable e) {
+                // in case of ANY kind of error, we'll mark this block as attempted and let
the IPC
+                // Caller handler catch this exception
+                preloadAttempted.add(startOffset);
+                LOG.error("Exception occured while attempting preload", e);
+                return;
+              }
               preloadAttempted.add(startOffset);
+              if (block == null) {
+                return;
+              }
+              if (block.getBlockType().isData()
+                  && !preloaderKvContext.getObtainedFromCache()) {
+                evictQueue.add(startOffset);
+              } else if (!block.getBlockType().isData()) {
+                metrics.nonDataBlocksRead++;
+              }
+              // otherwise we preloaded this block successfully ready to move on next block
+              startOffset = block.getOffset() + block.getOnDiskSizeWithHeader();
+              startSize = block.getNextBlockOnDiskSizeWithHeader();
+              leftToPreload.decrementAndGet();
+              if (evictQueue.size() > scanPreloadBlocksKeptInCache) {
+                long offset = evictQueue.poll();
+                hfileReaderV2.evictBlock(offset, isCompaction);
+              }
+            } finally {
               lock.unlock();
-              LOG.error("Exception occured while attempting preload", e);
-              return;
-            }
-            preloadAttempted.add(startOffset);
-            if (block == null) {
-              lock.unlock();
-              return;
-            }
-            if (block.getBlockType().isData()
-                && !preloaderKvContext.getObtainedFromCache()) {
-              evictQueue.add(startOffset);
-            } else if (!block.getBlockType().isData()) {
-              metrics.indexBlocksRead++;
-            }
-            // otherwise we preloaded this block successfully ready to move on next block
-            startOffset = block.getOffset() + block.getOnDiskSizeWithHeader();
-            startSize = block.getNextBlockOnDiskSizeWithHeader();
-            leftToPreload.decrementAndGet();
-            lock.unlock();
-            if (evictQueue.size() > scanPreloadBlocksKeptInCache) {
-              long offset = evictQueue.poll();
-              hfileReaderV2.evictBlock(offset, isCompaction);
             }
           }
           run = false;
@@ -764,13 +763,18 @@ public class HFileReaderV2 extends Abstr
           block =
               reader.readBlock(offset, size, cacheBlocks, isCompaction, false,
                 blocktype, kvContext);
+          metrics.lastRequestOffset = offset;
+          return block;
         } else {
           // wait for preloader to finish the current block being read
           lock.lock();
-          // This will make sure that this preloader will never run.
-          lastTask.run = false;
-          // Unlock the lock for future tasks
-          lock.unlock();
+          try {
+            // This will make sure that this preloader will never run.
+            lastTask.run = false;
+          } finally {
+            // Unlock the lock for future tasks
+            lock.unlock();
+          }
           // see if we already preloaded
           read = preloadAttempted.peek();
           if (read != null && read.equals(offset)) {
@@ -799,16 +803,16 @@ public class HFileReaderV2 extends Abstr
             expectedType = blocktype;
             leftToPreload.set(scanPreloadBlocksCount);
             startNewPreloader();
+            if (offset < metrics.lastRequestOffset) {
+              metrics.seekBacks++;
+              if (block != null && !block.getBlockType().isData()) {
+                metrics.seekBackNonData++;
+              }
+            }
           }
+          metrics.lastRequestOffset = offset;
+          return block;
         }
-        if (offset < metrics.lastRequestOffset) {
-          metrics.seekBacks++;
-          if (block != null && !block.getBlockType().isData()) {
-            metrics.seekBackNonData++;
-          }
-        }
-        metrics.lastRequestOffset = offset;
-        return block;
       }
 
       public boolean startNewPreloader() {
@@ -821,8 +825,11 @@ public class HFileReaderV2 extends Abstr
 
       public void close() {
         lock.lock();
-        lastTask.run = false;
-        lock.unlock();
+        try {
+          lastTask.run = false;
+        } finally {
+          lock.unlock();
+        }
         while (!evictQueue.isEmpty()) {
           hfileReaderV2.evictBlock(evictQueue.poll(), isCompaction);
         }



Mime
View raw message