[05/50] [abbrv] hbase git commit: HBASE-16788 Guard HFile archiving under a separate lock

2016-11-01 Thread larsh 
HBASE-16788 Guard HFile archiving under a separate lock


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

Branch: refs/heads/branch-1
Commit: 89bef67d0c020662599f682309c47a5ed25c9b32
Parents: 59ca4da
Author: Gary Helmling 
Authored: Fri Oct 7 10:42:20 2016 -0700
Committer: Gary Helmling 
Committed: Mon Oct 10 16:06:55 2016 -0700

--
 .../hadoop/hbase/regionserver/HStore.java   |  54 +++--
 .../TestCompactionArchiveConcurrentClose.java   | 198 +++
 2 files changed, 236 insertions(+), 16 deletions(-)
--


http://git-wip-us.apache.org/repos/asf/hbase/blob/89bef67d/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java
--
diff --git 
a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java 
b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java
index 6ee6bb5..74f5a1c 100644
--- 
a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java
+++ 
b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java
@@ -41,6 +41,7 @@ import java.util.concurrent.ExecutorCompletionService;
 import java.util.concurrent.Future;
 import java.util.concurrent.ThreadPoolExecutor;
 import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.concurrent.locks.ReentrantLock;
 import java.util.concurrent.locks.ReentrantReadWriteLock;
 
 import org.apache.commons.logging.Log;
@@ -149,6 +150,19 @@ public class HStore implements Store {
*   - completing a compaction
*/
   final ReentrantReadWriteLock lock = new ReentrantReadWriteLock();
+  /**
+   * Lock specific to archiving compacted store files.  This avoids races 
around
+   * the combination of retrieving the list of compacted files and moving them 
to
+   * the archive directory.  Since this is usually a background process (other 
than
+   * on close), we don't want to handle this with the store write lock, which 
would
+   * block readers and degrade performance.
+   *
+   * Locked by:
+   *   - CompactedHFilesDispatchHandler via closeAndArchiveCompactedFiles()
+   *   - close()
+   */
+  final ReentrantLock archiveLock = new ReentrantLock();
+
   private final boolean verifyBulkLoads;
 
   private ScanInfo scanInfo;
@@ -835,6 +849,7 @@ public class HStore implements Store {
 
   @Override
   public ImmutableCollection close() throws IOException {
+this.archiveLock.lock();
 this.lock.writeLock().lock();
 try {
   // Clear so metrics doesn't find them.
@@ -890,6 +905,7 @@ public class HStore implements Store {
   return result;
 } finally {
   this.lock.writeLock().unlock();
+  this.archiveLock.unlock();
 }
   }
 
@@ -2641,26 +2657,32 @@ public class HStore implements Store {
   }
 
   @Override
-  public void closeAndArchiveCompactedFiles() throws IOException {
-lock.readLock().lock();
-Collection copyCompactedfiles = null;
+  public synchronized void closeAndArchiveCompactedFiles() throws IOException {
+// ensure other threads do not attempt to archive the same files on close()
+archiveLock.lock();
 try {
-  Collection compactedfiles =
-  this.getStoreEngine().getStoreFileManager().getCompactedfiles();
-  if (compactedfiles != null && compactedfiles.size() != 0) {
-// Do a copy under read lock
-copyCompactedfiles = new ArrayList(compactedfiles);
-  } else {
-if (LOG.isTraceEnabled()) {
-  LOG.trace("No compacted files to archive");
-  return;
+  lock.readLock().lock();
+  Collection copyCompactedfiles = null;
+  try {
+Collection compactedfiles =
+this.getStoreEngine().getStoreFileManager().getCompactedfiles();
+if (compactedfiles != null && compactedfiles.size() != 0) {
+  // Do a copy under read lock
+  copyCompactedfiles = new ArrayList(compactedfiles);
+} else {
+  if (LOG.isTraceEnabled()) {
+LOG.trace("No compacted files to archive");
+return;
+  }
 }
+  } finally {
+lock.readLock().unlock();
+  }
+  if (copyCompactedfiles != null && !copyCompactedfiles.isEmpty()) {
+removeCompactedfiles(copyCompactedfiles);
   }
 } finally {
-  lock.readLock().unlock();
-}
-if (copyCompactedfiles != null && !copyCompactedfiles.isEmpty()) {
-  removeCompactedfiles(copyCompactedfiles);
+  archiveLock.unlock();
 }
   }

hbase git commit: HBASE-16788 Guard HFile archiving under a separate lock

2016-10-10 Thread garyh 
Repository: hbase
Updated Branches:
  refs/heads/branch-1.3 3e32164e4 -> 8eea3a577


HBASE-16788 Guard HFile archiving under a separate lock


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

Branch: refs/heads/branch-1.3
Commit: 8eea3a5777a25907dcf6486bfeafd8482a072b80
Parents: 3e32164
Author: Gary Helmling 
Authored: Fri Oct 7 10:42:20 2016 -0700
Committer: Gary Helmling 
Committed: Mon Oct 10 16:11:32 2016 -0700

--
 .../hadoop/hbase/regionserver/HStore.java   |  54 +++--
 .../TestCompactionArchiveConcurrentClose.java   | 198 +++
 2 files changed, 236 insertions(+), 16 deletions(-)
--


http://git-wip-us.apache.org/repos/asf/hbase/blob/8eea3a57/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java
--
diff --git 
a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java 
b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java
index 66eaebc..555a191 100644
--- 
a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java
+++ 
b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java
@@ -41,6 +41,7 @@ import java.util.concurrent.ExecutorCompletionService;
 import java.util.concurrent.Future;
 import java.util.concurrent.ThreadPoolExecutor;
 import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.concurrent.locks.ReentrantLock;
 import java.util.concurrent.locks.ReentrantReadWriteLock;
 
 import org.apache.commons.logging.Log;
@@ -148,6 +149,19 @@ public class HStore implements Store {
*   - completing a compaction
*/
   final ReentrantReadWriteLock lock = new ReentrantReadWriteLock();
+  /**
+   * Lock specific to archiving compacted store files.  This avoids races 
around
+   * the combination of retrieving the list of compacted files and moving them 
to
+   * the archive directory.  Since this is usually a background process (other 
than
+   * on close), we don't want to handle this with the store write lock, which 
would
+   * block readers and degrade performance.
+   *
+   * Locked by:
+   *   - CompactedHFilesDispatchHandler via closeAndArchiveCompactedFiles()
+   *   - close()
+   */
+  final ReentrantLock archiveLock = new ReentrantLock();
+
   private final boolean verifyBulkLoads;
 
   private ScanInfo scanInfo;
@@ -824,6 +838,7 @@ public class HStore implements Store {
 
   @Override
   public ImmutableCollection close() throws IOException {
+this.archiveLock.lock();
 this.lock.writeLock().lock();
 try {
   // Clear so metrics doesn't find them.
@@ -879,6 +894,7 @@ public class HStore implements Store {
   return result;
 } finally {
   this.lock.writeLock().unlock();
+  this.archiveLock.unlock();
 }
   }
 
@@ -2630,26 +2646,32 @@ public class HStore implements Store {
   }
 
   @Override
-  public void closeAndArchiveCompactedFiles() throws IOException {
-lock.readLock().lock();
-Collection copyCompactedfiles = null;
+  public synchronized void closeAndArchiveCompactedFiles() throws IOException {
+// ensure other threads do not attempt to archive the same files on close()
+archiveLock.lock();
 try {
-  Collection compactedfiles =
-  this.getStoreEngine().getStoreFileManager().getCompactedfiles();
-  if (compactedfiles != null && compactedfiles.size() != 0) {
-// Do a copy under read lock
-copyCompactedfiles = new ArrayList(compactedfiles);
-  } else {
-if (LOG.isTraceEnabled()) {
-  LOG.trace("No compacted files to archive");
-  return;
+  lock.readLock().lock();
+  Collection copyCompactedfiles = null;
+  try {
+Collection compactedfiles =
+this.getStoreEngine().getStoreFileManager().getCompactedfiles();
+if (compactedfiles != null && compactedfiles.size() != 0) {
+  // Do a copy under read lock
+  copyCompactedfiles = new ArrayList(compactedfiles);
+} else {
+  if (LOG.isTraceEnabled()) {
+LOG.trace("No compacted files to archive");
+return;
+  }
 }
+  } finally {
+lock.readLock().unlock();
+  }
+  if (copyCompactedfiles != null && !copyCompactedfiles.isEmpty()) {
+removeCompactedfiles(copyCompactedfiles);
   }
 } finally {
-  lock.readLock().unlock();
-}
-if (copyCompactedfiles != null && !copyCompactedfiles.isEmpty()) {
-  removeCompactedfiles(copyCompactedfiles);
+  archiveLock.unlock();
 }
   }

hbase git commit: HBASE-16788 Guard HFile archiving under a separate lock

2016-10-10 Thread garyh 
Repository: hbase
Updated Branches:
  refs/heads/branch-1 59ca4dad7 -> 89bef67d0


HBASE-16788 Guard HFile archiving under a separate lock


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

Branch: refs/heads/branch-1
Commit: 89bef67d0c020662599f682309c47a5ed25c9b32
Parents: 59ca4da
Author: Gary Helmling 
Authored: Fri Oct 7 10:42:20 2016 -0700
Committer: Gary Helmling 
Committed: Mon Oct 10 16:06:55 2016 -0700

--
 .../hadoop/hbase/regionserver/HStore.java   |  54 +++--
 .../TestCompactionArchiveConcurrentClose.java   | 198 +++
 2 files changed, 236 insertions(+), 16 deletions(-)
--


http://git-wip-us.apache.org/repos/asf/hbase/blob/89bef67d/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java
--
diff --git 
a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java 
b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java
index 6ee6bb5..74f5a1c 100644
--- 
a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java
+++ 
b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java
@@ -41,6 +41,7 @@ import java.util.concurrent.ExecutorCompletionService;
 import java.util.concurrent.Future;
 import java.util.concurrent.ThreadPoolExecutor;
 import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.concurrent.locks.ReentrantLock;
 import java.util.concurrent.locks.ReentrantReadWriteLock;
 
 import org.apache.commons.logging.Log;
@@ -149,6 +150,19 @@ public class HStore implements Store {
*   - completing a compaction
*/
   final ReentrantReadWriteLock lock = new ReentrantReadWriteLock();
+  /**
+   * Lock specific to archiving compacted store files.  This avoids races 
around
+   * the combination of retrieving the list of compacted files and moving them 
to
+   * the archive directory.  Since this is usually a background process (other 
than
+   * on close), we don't want to handle this with the store write lock, which 
would
+   * block readers and degrade performance.
+   *
+   * Locked by:
+   *   - CompactedHFilesDispatchHandler via closeAndArchiveCompactedFiles()
+   *   - close()
+   */
+  final ReentrantLock archiveLock = new ReentrantLock();
+
   private final boolean verifyBulkLoads;
 
   private ScanInfo scanInfo;
@@ -835,6 +849,7 @@ public class HStore implements Store {
 
   @Override
   public ImmutableCollection close() throws IOException {
+this.archiveLock.lock();
 this.lock.writeLock().lock();
 try {
   // Clear so metrics doesn't find them.
@@ -890,6 +905,7 @@ public class HStore implements Store {
   return result;
 } finally {
   this.lock.writeLock().unlock();
+  this.archiveLock.unlock();
 }
   }
 
@@ -2641,26 +2657,32 @@ public class HStore implements Store {
   }
 
   @Override
-  public void closeAndArchiveCompactedFiles() throws IOException {
-lock.readLock().lock();
-Collection copyCompactedfiles = null;
+  public synchronized void closeAndArchiveCompactedFiles() throws IOException {
+// ensure other threads do not attempt to archive the same files on close()
+archiveLock.lock();
 try {
-  Collection compactedfiles =
-  this.getStoreEngine().getStoreFileManager().getCompactedfiles();
-  if (compactedfiles != null && compactedfiles.size() != 0) {
-// Do a copy under read lock
-copyCompactedfiles = new ArrayList(compactedfiles);
-  } else {
-if (LOG.isTraceEnabled()) {
-  LOG.trace("No compacted files to archive");
-  return;
+  lock.readLock().lock();
+  Collection copyCompactedfiles = null;
+  try {
+Collection compactedfiles =
+this.getStoreEngine().getStoreFileManager().getCompactedfiles();
+if (compactedfiles != null && compactedfiles.size() != 0) {
+  // Do a copy under read lock
+  copyCompactedfiles = new ArrayList(compactedfiles);
+} else {
+  if (LOG.isTraceEnabled()) {
+LOG.trace("No compacted files to archive");
+return;
+  }
 }
+  } finally {
+lock.readLock().unlock();
+  }
+  if (copyCompactedfiles != null && !copyCompactedfiles.isEmpty()) {
+removeCompactedfiles(copyCompactedfiles);
   }
 } finally {
-  lock.readLock().unlock();
-}
-if (copyCompactedfiles != null && !copyCompactedfiles.isEmpty()) {
-  removeCompactedfiles(copyCompactedfiles);
+  archiveLock.unlock();
 }
   }

hbase git commit: HBASE-16788 Guard HFile archiving under a separate lock

2016-10-10 Thread garyh 
Repository: hbase
Updated Branches:
  refs/heads/master fcef2c02c -> 7493e79f1


HBASE-16788 Guard HFile archiving under a separate lock


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

Branch: refs/heads/master
Commit: 7493e79f15e0a1217dc50ca4431d6ded07df479f
Parents: fcef2c0
Author: Gary Helmling 
Authored: Fri Oct 7 10:42:20 2016 -0700
Committer: Gary Helmling 
Committed: Mon Oct 10 15:58:12 2016 -0700

--
 .../hadoop/hbase/regionserver/HStore.java   |  54 +++--
 .../TestCompactionArchiveConcurrentClose.java   | 198 +++
 2 files changed, 236 insertions(+), 16 deletions(-)
--


http://git-wip-us.apache.org/repos/asf/hbase/blob/7493e79f/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java
--
diff --git 
a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java 
b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java
index e9c05c7..5418138 100644
--- 
a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java
+++ 
b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java
@@ -46,6 +46,7 @@ import java.util.concurrent.ExecutorCompletionService;
 import java.util.concurrent.Future;
 import java.util.concurrent.ThreadPoolExecutor;
 import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.concurrent.locks.ReentrantLock;
 import java.util.concurrent.locks.ReentrantReadWriteLock;
 
 import org.apache.commons.logging.Log;
@@ -147,6 +148,19 @@ public class HStore implements Store {
*   - completing a compaction
*/
   final ReentrantReadWriteLock lock = new ReentrantReadWriteLock();
+  /**
+   * Lock specific to archiving compacted store files.  This avoids races 
around
+   * the combination of retrieving the list of compacted files and moving them 
to
+   * the archive directory.  Since this is usually a background process (other 
than
+   * on close), we don't want to handle this with the store write lock, which 
would
+   * block readers and degrade performance.
+   *
+   * Locked by:
+   *   - CompactedHFilesDispatchHandler via closeAndArchiveCompactedFiles()
+   *   - close()
+   */
+  final ReentrantLock archiveLock = new ReentrantLock();
+
   private final boolean verifyBulkLoads;
 
   private ScanInfo scanInfo;
@@ -794,6 +808,7 @@ public class HStore implements Store {
 
   @Override
   public ImmutableCollection close() throws IOException {
+this.archiveLock.lock();
 this.lock.writeLock().lock();
 try {
   // Clear so metrics doesn't find them.
@@ -849,6 +864,7 @@ public class HStore implements Store {
   return result;
 } finally {
   this.lock.writeLock().unlock();
+  this.archiveLock.unlock();
 }
   }
 
@@ -2357,26 +2373,32 @@ public class HStore implements Store {
   }
 
   @Override
-  public void closeAndArchiveCompactedFiles() throws IOException {
-lock.readLock().lock();
-Collection copyCompactedfiles = null;
+  public synchronized void closeAndArchiveCompactedFiles() throws IOException {
+// ensure other threads do not attempt to archive the same files on close()
+archiveLock.lock();
 try {
-  Collection compactedfiles =
-  this.getStoreEngine().getStoreFileManager().getCompactedfiles();
-  if (compactedfiles != null && compactedfiles.size() != 0) {
-// Do a copy under read lock
-copyCompactedfiles = new ArrayList(compactedfiles);
-  } else {
-if (LOG.isTraceEnabled()) {
-  LOG.trace("No compacted files to archive");
-  return;
+  lock.readLock().lock();
+  Collection copyCompactedfiles = null;
+  try {
+Collection compactedfiles =
+this.getStoreEngine().getStoreFileManager().getCompactedfiles();
+if (compactedfiles != null && compactedfiles.size() != 0) {
+  // Do a copy under read lock
+  copyCompactedfiles = new ArrayList(compactedfiles);
+} else {
+  if (LOG.isTraceEnabled()) {
+LOG.trace("No compacted files to archive");
+return;
+  }
 }
+  } finally {
+lock.readLock().unlock();
+  }
+  if (copyCompactedfiles != null && !copyCompactedfiles.isEmpty()) {
+removeCompactedfiles(copyCompactedfiles);
   }
 } finally {
-  lock.readLock().unlock();
-}
-if (copyCompactedfiles != null && !copyCompactedfiles.isEmpty()) {
-  removeCompactedfiles(copyCompactedfiles);
+  archiveLock.unlock();
 }
   }