From e9aafde1f124c396f629749d9d78968614c0ad34 Mon Sep 17 00:00:00 2001
From: Michael Stack <stack@apache.org>
Date: Sat, 25 Aug 2007 00:42:45 +0000
Subject: [PATCH] HADOOP-1780 Regions are still being doubly assigned

M src/contrib/hbase/src/java/org/apache/hadoop/hbase/HStore.java
    Fix outputing fail message on each compaction though there was none.
M  src/contrib/hbase/src/java/org/apache/hadoop/hbase/HStoreFile.java
    (rename): Refactor so return only happens on end..
M  src/contrib/hbase/src/java/org/apache/hadoop/hbase/HMaster.java
    (assignRegions): Make synchronized.  In presence of concurrent visits
    by regionservers, both visiting threads could grab same set of regions
    for assignment.



git-svn-id: https://svn.apache.org/repos/asf/lucene/hadoop/trunk/src/contrib/hbase@569589 13f79535-47bb-0310-9956-ffa450edef68
---
 CHANGES.txt                                    |  1 +
 src/java/org/apache/hadoop/hbase/HMaster.java  |  2 +-
 src/java/org/apache/hadoop/hbase/HStore.java   |  9 +++++----
 .../org/apache/hadoop/hbase/HStoreFile.java    | 18 +++++++++---------
 4 files changed, 16 insertions(+), 14 deletions(-)

diff --git a/CHANGES.txt b/CHANGES.txt
index 8e08513492a..09b14c7e621 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -20,6 +20,7 @@ Trunk (unreleased changes)
     HADOOP-1747 On a cluster, on restart, regions multiply assigned
     HADOOP-1776 Fix for sporadic compaction failures closing and moving
     compaction result
+    HADOOP-1780 Regions are still being doubly assigned
 
   IMPROVEMENTS
     HADOOP-1737 Make HColumnDescriptor data publically members settable
diff --git a/src/java/org/apache/hadoop/hbase/HMaster.java b/src/java/org/apache/hadoop/hbase/HMaster.java
index 58cf4f981fe..26c2fb8bb88 100644
--- a/src/java/org/apache/hadoop/hbase/HMaster.java
+++ b/src/java/org/apache/hadoop/hbase/HMaster.java
@@ -1468,7 +1468,7 @@ HMasterRegionInterface, Runnable {
    * @param serverName
    * @param returnMsgs
    */
-  private void assignRegions(HServerInfo info, String serverName,
+  private synchronized void assignRegions(HServerInfo info, String serverName,
       ArrayList<HMsg> returnMsgs) {
     
     TreeSet<Text> regionsToAssign = getRegionsToAssign();
diff --git a/src/java/org/apache/hadoop/hbase/HStore.java b/src/java/org/apache/hadoop/hbase/HStore.java
index d76b0ba46b8..67bc5dd284b 100644
--- a/src/java/org/apache/hadoop/hbase/HStore.java
+++ b/src/java/org/apache/hadoop/hbase/HStore.java
@@ -923,7 +923,7 @@ class HStore implements HConstants {
       if (!fs.exists(doneFile)) {
         // The last execution didn't finish the compaction, so there's nothing 
         // we can do.  We'll just have to redo it. Abandon it and return.
-        LOG.warn("Redoing a failed compaction");
+        LOG.warn("Redo failed compaction (missing 'done' file)");
         return;
       }
 
@@ -986,10 +986,11 @@ class HStore implements HConstants {
         this.readers.put(orderVal,
             finalCompactedFile.getReader(this.fs, this.bloomFilter));
         this.storefiles.put(orderVal, finalCompactedFile);
-      } finally {
-        LOG.warn("Failed replacing compacted files.  Compacted fle is " +
+      } catch (IOException e) {
+        LOG.error("Failed replacing compacted files. Compacted file is " +
           finalCompactedFile.toString() + ".  Files replaced are " +
-          toCompactFiles.toString() + " some of which may have been removed");
+          toCompactFiles.toString() +
+          " some of which may have been already removed", e);
       }
     } finally {
       // 7. Releasing the write-lock
diff --git a/src/java/org/apache/hadoop/hbase/HStoreFile.java b/src/java/org/apache/hadoop/hbase/HStoreFile.java
index 4ed76690ef1..fc4ba05ceb0 100644
--- a/src/java/org/apache/hadoop/hbase/HStoreFile.java
+++ b/src/java/org/apache/hadoop/hbase/HStoreFile.java
@@ -644,15 +644,15 @@ public class HStoreFile implements HConstants, WritableComparable {
     boolean success = fs.rename(src, hsf.getMapFilePath());
     if (!success) {
       LOG.warn("Failed rename of " + src + " to " + hsf.getMapFilePath());
-      return success;
-    }
-    src = getInfoFilePath();
-    if (!fs.exists(src)) {
-      throw new FileNotFoundException(src.toString());
-    }
-    success = fs.rename(src, hsf.getInfoFilePath());
-    if (!success) {
-      LOG.warn("Failed rename of " + src + " to " + hsf.getInfoFilePath());
+    } else {
+      src = getInfoFilePath();
+      if (!fs.exists(src)) {
+        throw new FileNotFoundException(src.toString());
+      }
+      success = fs.rename(src, hsf.getInfoFilePath());
+      if (!success) {
+        LOG.warn("Failed rename of " + src + " to " + hsf.getInfoFilePath());
+      }
     }
     return success;
   }