hbase-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From la...@apache.org
Subject hbase git commit: Revert "HBASE-13082 Coarsen StoreScanner locks to RegionScanner."
Date Fri, 06 Mar 2015 23:26:55 GMT
Repository: hbase
Updated Branches:
  refs/heads/branch-1 6e5e5d8cc -> c8610ce36


Revert "HBASE-13082 Coarsen StoreScanner locks to RegionScanner."

This reverts commit 02522615d186e900de39c22d61d6592113f3d134.
Reverted due to test failures.


Project: http://git-wip-us.apache.org/repos/asf/hbase/repo
Commit: http://git-wip-us.apache.org/repos/asf/hbase/commit/c8610ce3
Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/c8610ce3
Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/c8610ce3

Branch: refs/heads/branch-1
Commit: c8610ce36f398f74aefbe46c3f65fcc9a6b4e394
Parents: 6e5e5d8
Author: Lars Hofhansl <larsh@apache.org>
Authored: Fri Mar 6 15:25:59 2015 -0800
Committer: Lars Hofhansl <larsh@apache.org>
Committed: Fri Mar 6 15:25:59 2015 -0800

----------------------------------------------------------------------
 .../hadoop/hbase/regionserver/HRegion.java      |  2 -
 .../hbase/regionserver/KeyValueScanner.java     |  7 ---
 .../regionserver/NonLazyKeyValueScanner.java    |  3 --
 .../regionserver/ReversedStoreScanner.java      | 19 ++++++--
 .../hbase/regionserver/StoreFileScanner.java    |  3 --
 .../hadoop/hbase/regionserver/StoreScanner.java | 46 ++++++++++++++------
 6 files changed, 47 insertions(+), 33 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/hbase/blob/c8610ce3/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
index 14df928..9f192be 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
@@ -5338,8 +5338,6 @@ public class HRegion implements HeapSize, PropagatingConfigurationObserver
{ //
           scan.getFamilyMap().entrySet()) {
         Store store = stores.get(entry.getKey());
         KeyValueScanner scanner = store.getScanner(scan, entry.getValue(), this.readPt);
-        // pass the RegionScanner object to lock out concurrent changes to set of readers
-        scanner.setReaderLock(this);
         if (this.filter == null || !scan.doLoadColumnFamiliesOnDemand()
           || this.filter.isFamilyEssential(entry.getKey())) {
           scanners.add(scanner);

http://git-wip-us.apache.org/repos/asf/hbase/blob/c8610ce3/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/KeyValueScanner.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/KeyValueScanner.java
b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/KeyValueScanner.java
index 782dc93..76a9d0f 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/KeyValueScanner.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/KeyValueScanner.java
@@ -162,11 +162,4 @@ public interface KeyValueScanner {
    * if known, or null otherwise
    */
   public Cell getNextIndexedKey();
-
-  /**
-   * Set the object to lock when the scanner's readers (if any) are updated (this is here
so that the
-   * coprocessors creating {@link StoreScanner}'s do not have to change)
-   * @param obj lock object to use
-   */
-  public void setReaderLock(Object obj);
 }

http://git-wip-us.apache.org/repos/asf/hbase/blob/c8610ce3/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/NonLazyKeyValueScanner.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/NonLazyKeyValueScanner.java
b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/NonLazyKeyValueScanner.java
index 91886a8..957f417 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/NonLazyKeyValueScanner.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/NonLazyKeyValueScanner.java
@@ -71,7 +71,4 @@ public abstract class NonLazyKeyValueScanner implements KeyValueScanner
{
   public Cell getNextIndexedKey() {
     return null;
   }
-
-  @Override
-  public void setReaderLock(Object obj) {/* NO OP */}
 }

http://git-wip-us.apache.org/repos/asf/hbase/blob/c8610ce3/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ReversedStoreScanner.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ReversedStoreScanner.java
b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ReversedStoreScanner.java
index e000501..e319f90 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ReversedStoreScanner.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ReversedStoreScanner.java
@@ -124,13 +124,24 @@ class ReversedStoreScanner extends StoreScanner implements KeyValueScanner
{
 
   @Override
   public boolean seekToPreviousRow(Cell key) throws IOException {
-    checkReseek();
-    return this.heap.seekToPreviousRow(key);
+    lock.lock();
+    try {
+      checkReseek();
+      return this.heap.seekToPreviousRow(key);
+    } finally {
+      lock.unlock();
+    }
+
   }
   
   @Override
   public boolean backwardSeek(Cell key) throws IOException {
-    checkReseek();
-    return this.heap.backwardSeek(key);
+    lock.lock();
+    try {
+      checkReseek();
+      return this.heap.backwardSeek(key);
+    } finally {
+      lock.unlock();
+    }
   }
 }

http://git-wip-us.apache.org/repos/asf/hbase/blob/c8610ce3/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFileScanner.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFileScanner.java
b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFileScanner.java
index bac6f5c..a8ee091 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFileScanner.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFileScanner.java
@@ -489,7 +489,4 @@ public class StoreFileScanner implements KeyValueScanner {
   public Cell getNextIndexedKey() {
     return hfs.getNextIndexedKey();
   }
-
-  @Override
-  public void setReaderLock(Object obj) {/* NO OP */}
 }

http://git-wip-us.apache.org/repos/asf/hbase/blob/c8610ce3/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreScanner.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreScanner.java
b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreScanner.java
index ff89b52..7ce4e0b 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreScanner.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreScanner.java
@@ -25,6 +25,7 @@ import java.util.ArrayList;
 import java.util.List;
 import java.util.NavigableSet;
 import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.locks.ReentrantLock;
 
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
@@ -102,17 +103,10 @@ public class StoreScanner extends NonReversedNonLazyKeyValueScanner
 
   // A flag whether use pread for scan
   private boolean scanUsePread = false;
+  protected ReentrantLock lock = new ReentrantLock();
   
   private final long readPt;
 
-  // lock to use for updateReaders
-  // creator needs to ensure that:
-  // 1. *all* calls to public methods (except updateReaders and close) are locked with this
lock
-  //    (this can be done by passing the RegionScannerImpl object down)
-  // OR
-  // 2. updateReader is *never* called (such as in flushes or compactions)
-  private Object readerLock;
-
   // used by the injection framework to test race between StoreScanner construction and compaction
   enum StoreScannerCompactionRace {
     BEFORE_SEEK,
@@ -401,10 +395,15 @@ public class StoreScanner extends NonReversedNonLazyKeyValueScanner
 
   @Override
   public Cell peek() {
+    lock.lock();
+    try {
     if (this.heap == null) {
       return this.lastTop;
     }
     return this.heap.peek();
+    } finally {
+      lock.unlock();
+    }
   }
 
   @Override
@@ -415,6 +414,8 @@ public class StoreScanner extends NonReversedNonLazyKeyValueScanner
 
   @Override
   public void close() {
+    lock.lock();
+    try {
     if (this.closing) return;
     this.closing = true;
     // under test, we dont have a this.store
@@ -424,13 +425,21 @@ public class StoreScanner extends NonReversedNonLazyKeyValueScanner
       this.heap.close();
     this.heap = null; // CLOSED!
     this.lastTop = null; // If both are null, we are closed.
+    } finally {
+      lock.unlock();
+    }
   }
 
   @Override
   public boolean seek(Cell key) throws IOException {
+    lock.lock();
+    try {
     // reset matcher state, in case that underlying store changed
     checkReseek();
     return this.heap.seek(key);
+    } finally {
+      lock.unlock();
+    }
   }
 
   /**
@@ -455,6 +464,8 @@ public class StoreScanner extends NonReversedNonLazyKeyValueScanner
   @Override
   public NextState next(List<Cell> outResult, int limit, long remainingResultSize)
       throws IOException {
+    lock.lock();
+    try {
     if (checkReseek()) {
       return NextState.makeState(NextState.State.MORE_VALUES, 0);
     }
@@ -606,6 +617,9 @@ public class StoreScanner extends NonReversedNonLazyKeyValueScanner
     // No more keys
     close();
     return NextState.makeState(NextState.State.NO_MORE_VALUES, totalHeapSize);
+    } finally {
+      lock.unlock();
+    }
   }
 
   /*
@@ -649,7 +663,8 @@ public class StoreScanner extends NonReversedNonLazyKeyValueScanner
   // Implementation of ChangedReadersObserver
   @Override
   public void updateReaders() throws IOException {
-    synchronized(readerLock) {
+    lock.lock();
+    try {
     if (this.closing) return;
 
     // All public synchronized API calls will call 'checkReseek' which will cause
@@ -669,6 +684,8 @@ public class StoreScanner extends NonReversedNonLazyKeyValueScanner
     this.heap = null; // the re-seeks could be slow (access HDFS) free up memory ASAP
 
     // Let the next() call handle re-creating and seeking
+    } finally {
+      lock.unlock();
     }
   }
 
@@ -759,6 +776,8 @@ public class StoreScanner extends NonReversedNonLazyKeyValueScanner
 
   @Override
   public boolean reseek(Cell kv) throws IOException {
+    lock.lock();
+    try {
     //Heap will not be null, if this is called from next() which.
     //If called from RegionScanner.reseek(...) make sure the scanner
     //stack is reset if needed.
@@ -767,6 +786,9 @@ public class StoreScanner extends NonReversedNonLazyKeyValueScanner
       return heap.requestSeek(kv, true, useRowColBloom);
     }
     return heap.reseek(kv);
+    } finally {
+      lock.unlock();
+    }
   }
 
   @Override
@@ -841,9 +863,5 @@ public class StoreScanner extends NonReversedNonLazyKeyValueScanner
   public Cell getNextIndexedKey() {
     return this.heap.getNextIndexedKey();
   }
-
-  @Override
-  public void setReaderLock(Object obj) {
-    this.readerLock = obj;
-  }
 }
+


Mime
View raw message