HBASE-7579 HTableDescriptor equals method fails if results are returned in a different order

git-svn-id: https://svn.apache.org/repos/asf/hbase/trunk@1471052 13f79535-47bb-0310-9956-ffa450edef68

bin/hbase-daemon.sh

@@ -36,7 +36,7 @@
 
 usage="Usage: hbase-daemon.sh [--config <conf-dir>]\
  (start|stop|restart|autorestart) <hbase-command> \
- <args...>"
+ [--formatZK] [--formatFS] <args...>"
 
 # if no args specified, show usage
 if [ $# -le 1 ]; then
@@ -57,6 +57,19 @@ shift
 command=$1
 shift
 
+if [ "$startStop" = "start" ];then
+  for i in 1 2
+  do
+    if [ "$1" = "--formatZK" ];then
+      formatzk=$1
+      shift
+    elif [ "$1" = "--formatFS" ];then
+      formatfs=$1
+      shift
+    fi
+  done
+fi
+
 hbase_rotate_log ()
 {
     log=$1;
@@ -98,6 +111,10 @@ check_before_start(){
     fi
 }
 
+clear_hbase_data() {
+  $bin/hbase-cleanup.sh $formatzk $formatfs
+}
+
 wait_until_done ()
 {
     p=$1
@@ -172,6 +189,7 @@ case $startStop in
 
 (start)
     check_before_start
+    clear_hbase_data
     nohup $thiscmd --config "${HBASE_CONF_DIR}" internal_start $command $args < /dev/null > /dev/null 2>&1  &
   ;;
 

bin/start-hbase.sh

@@ -24,7 +24,7 @@
 
 # Start hadoop hbase daemons.
 # Run this on master node.
-usage="Usage: start-hbase.sh"
+usage="Usage: start-hbase.sh [autorestart] [--formatZK] [--formatFS]"
 
 bin=`dirname "${BASH_SOURCE-$0}"`
 bin=`cd "$bin">/dev/null; pwd`
@@ -37,12 +37,19 @@ if [ $errCode -ne 0 ]
 then
   exit $errCode
 fi
+for i in 1 2 3
+do
+  if [ "$1" = "autorestart" ];then
+    commandToRun="autorestart"
+  elif [ "$1" = "--formatZK" ];then
+    formatzk=$1
+  elif [ "$1" = "--formatFS" ];then
+    formatfs=$1
+  fi
+  shift
+done
 
-
-if [ "$1" = "autorestart" ]
-then
-  commandToRun="autorestart"
-else 
+if [ "$commandToRun" = "" ];then
   commandToRun="start"
 fi
 
@@ -52,10 +59,10 @@ distMode=`$bin/hbase --config "$HBASE_CONF_DIR" org.apache.hadoop.hbase.util.HBa
 
 if [ "$distMode" == 'false' ] 
 then
-  "$bin"/hbase-daemon.sh $commandToRun master
+  "$bin"/hbase-daemon.sh $commandToRun master $formatzk $formatfs
 else
   "$bin"/hbase-daemons.sh --config "${HBASE_CONF_DIR}" $commandToRun zookeeper
-  "$bin"/hbase-daemon.sh --config "${HBASE_CONF_DIR}" $commandToRun master 
+  "$bin"/hbase-daemon.sh --config "${HBASE_CONF_DIR}" $commandToRun master  $formatzk $formatfs
   "$bin"/hbase-daemons.sh --config "${HBASE_CONF_DIR}" \
     --hosts "${HBASE_REGIONSERVERS}" $commandToRun regionserver
   "$bin"/hbase-daemons.sh --config "${HBASE_CONF_DIR}" \

hbase-client/src/main/java/org/apache/hadoop/hbase/HColumnDescriptor.java

@@ -1125,12 +1125,10 @@ public class HColumnDescriptor implements WritableComparable<HColumnDescriptor>
   public int compareTo(HColumnDescriptor o) {
     int result = Bytes.compareTo(this.name, o.getName());
     if (result == 0) {
-      // punt on comparison for ordering, just calculate difference
-      result = this.values.hashCode() - o.values.hashCode();
-      if (result < 0)
-        result = -1;
-      else if (result > 0)
-        result = 1;
+      // The maps interface should compare values, even if they're in different orders
+      if (!this.values.equals(o.values)) {
+        return 1;
+      }
     }
     if (result == 0) {
       result = this.configuration.hashCode() - o.configuration.hashCode();

hbase-client/src/main/java/org/apache/hadoop/hbase/HTableDescriptor.java

@@ -225,9 +225,7 @@ public class HTableDescriptor implements WritableComparable<HTableDescriptor> {
    * catalog tables, <code>.META.</code> and <code>-ROOT-</code>.
    */
   protected HTableDescriptor(final byte [] name, HColumnDescriptor[] families) {
-    this.name = name.clone();
-    this.nameAsString = Bytes.toString(this.name);
-    setMetaFlags(name);
+    setName(name);
     for(HColumnDescriptor descriptor : families) {
       this.families.put(descriptor.getName(), descriptor);
     }
@@ -239,12 +237,7 @@ public class HTableDescriptor implements WritableComparable<HTableDescriptor> {
    */
   protected HTableDescriptor(final byte [] name, HColumnDescriptor[] families,
       Map<ImmutableBytesWritable,ImmutableBytesWritable> values) {
-    this.name = name.clone();
-    this.nameAsString = Bytes.toString(this.name);
-    setMetaFlags(name);
-    for(HColumnDescriptor descriptor : families) {
-      this.families.put(descriptor.getName(), descriptor);
-    }
+    this(name.clone(), families);
     for (Map.Entry<ImmutableBytesWritable, ImmutableBytesWritable> entry:
         values.entrySet()) {
       setValue(entry.getKey(), entry.getValue());
@@ -284,9 +277,7 @@ public class HTableDescriptor implements WritableComparable<HTableDescriptor> {
    */
   public HTableDescriptor(final byte [] name) {
     super();
-    setMetaFlags(this.name);
-    this.name = this.isMetaRegion()? name: isLegalTableName(name);
-    this.nameAsString = Bytes.toString(this.name);
+    setName(name);
   }
 
   /**
@@ -298,9 +289,7 @@ public class HTableDescriptor implements WritableComparable<HTableDescriptor> {
    */
   public HTableDescriptor(final HTableDescriptor desc) {
     super();
-    this.name = desc.name.clone();
-    this.nameAsString = Bytes.toString(this.name);
-    setMetaFlags(this.name);
+    setName(desc.name.clone());
     for (HColumnDescriptor c: desc.families.values()) {
       this.families.put(c.getName(), new HColumnDescriptor(c));
     }
@@ -650,9 +639,13 @@ public class HTableDescriptor implements WritableComparable<HTableDescriptor> {
    * Set the name of the table.
    *
    * @param name name of table
+   * @throws IllegalArgumentException if passed a table name
+   * that is made of other than 'word' characters, underscore or period: i.e.
+   * <code>[a-zA-Z_0-9.].
+   * @see <a href="HADOOP-1581">HADOOP-1581 HBASE: Un-openable tablename bug</a>
    */
   public void setName(byte[] name) {
-    this.name = name;
+    this.name = isMetaTable(name) ? name : isLegalTableName(name);
     this.nameAsString = Bytes.toString(this.name);
     setMetaFlags(this.name);
   }
@@ -987,39 +980,34 @@ public class HTableDescriptor implements WritableComparable<HTableDescriptor> {
    */
   @Override
   public int compareTo(final HTableDescriptor other) {
+    // Check name matches
     int result = Bytes.compareTo(this.name, other.name);
-    if (result == 0) {
-      result = families.size() - other.families.size();
-    }
-    if (result == 0 && families.size() != other.families.size()) {
-      result = Integer.valueOf(families.size()).compareTo(
-          Integer.valueOf(other.families.size()));
-    }
-    if (result == 0) {
-      for (Iterator<HColumnDescriptor> it = families.values().iterator(),
-          it2 = other.families.values().iterator(); it.hasNext(); ) {
-        result = it.next().compareTo(it2.next());
-        if (result != 0) {
-          break;
-        }
+    if (result != 0) return result;
+
+    // Check size matches
+    result = families.size() - other.families.size();
+    if (result != 0) return result;
+
+    // Compare that all column families
+    for (Iterator<HColumnDescriptor> it = families.values().iterator(),
+         it2 = other.families.values().iterator(); it.hasNext(); ) {
+      result = it.next().compareTo(it2.next());
+      if (result != 0) {
+        return result;
       }
     }
-    if (result == 0) {
-      // punt on comparison for ordering, just calculate difference
-      result = this.values.hashCode() - other.values.hashCode();
-      if (result < 0)
-        result = -1;
-      else if (result > 0)
-        result = 1;
+
+    // Compare values
+    if (!values.equals(other.values)) {
+      return 1;
     }
-    if (result == 0) {
-      result = this.configuration.hashCode() - other.configuration.hashCode();
-      if (result < 0)
-        result = -1;
-      else if (result > 0)
-        result = 1;
+
+    // Compare configuration
+    if (!configuration.equals(other.configuration)) {
+      return 1;
     }
-    return result;
+
+    return 0;
   }
 
   /**

hbase-server/src/test/java/org/apache/hadoop/hbase/TestHColumnDescriptor.java

@@ -18,12 +18,14 @@
 package org.apache.hadoop.hbase;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
 
 import org.apache.hadoop.hbase.exceptions.DeserializationException;
 import org.apache.hadoop.hbase.io.compress.Compression;
 import org.apache.hadoop.hbase.io.compress.Compression.Algorithm;
 import org.apache.hadoop.hbase.io.encoding.DataBlockEncoding;
+import org.apache.hadoop.hbase.io.hfile.BlockType;
 import org.apache.hadoop.hbase.regionserver.BloomType;
 import org.junit.experimental.categories.Category;
 
@@ -94,4 +96,71 @@ public class TestHColumnDescriptor {
     desc.removeConfiguration(key);
     assertEquals(null, desc.getConfigurationValue(key));
   }
+
+  @Test
+  public void testEqualsWithFamilyName() {
+    final String name1 = "someFamilyName";
+    HColumnDescriptor hcd1 = new HColumnDescriptor(name1);
+    HColumnDescriptor hcd2 = new HColumnDescriptor("someOtherFamilyName");
+    HColumnDescriptor hcd3 = new HColumnDescriptor(name1);
+
+    assertFalse(hcd1.equals(hcd2));
+    assertFalse(hcd2.equals(hcd1));
+
+    assertTrue(hcd3.equals(hcd1));
+    assertTrue(hcd1.equals(hcd3));
+  }
+
+  @Test
+  public void testEqualsWithAdditionalProperties() {
+    final String name1 = "someFamilyName";
+    HColumnDescriptor hcd1 = new HColumnDescriptor(name1);
+    HColumnDescriptor hcd2 = new HColumnDescriptor(name1);
+    hcd2.setBlocksize(4);
+
+    assertFalse(hcd1.equals(hcd2));
+    assertFalse(hcd2.equals(hcd1));
+
+    hcd1.setBlocksize(4);
+
+    assertTrue(hcd2.equals(hcd1));
+    assertTrue(hcd1.equals(hcd2));
+  }
+
+  @Test
+  public void testEqualsWithDifferentNumberOfProperties() {
+    final String name1 = "someFamilyName";
+    HColumnDescriptor hcd1 = new HColumnDescriptor(name1);
+    HColumnDescriptor hcd2 = new HColumnDescriptor(name1);
+    hcd2.setBlocksize(4);
+    hcd1.setBlocksize(4);
+
+    assertTrue(hcd2.equals(hcd1));
+    assertTrue(hcd1.equals(hcd2));
+
+    hcd2.setBloomFilterType(BloomType.ROW);
+
+    assertFalse(hcd1.equals(hcd2));
+    assertFalse(hcd2.equals(hcd1));
+  }
+
+  @Test
+  public void testEqualsWithDifferentOrderingOfProperties() {
+    final String name1 = "someFamilyName";
+    HColumnDescriptor hcd1 = new HColumnDescriptor(name1);
+    HColumnDescriptor hcd2 = new HColumnDescriptor(name1);
+    hcd2.setBlocksize(4);
+    hcd2.setBloomFilterType(BloomType.ROW);
+    hcd1.setBloomFilterType(BloomType.ROW);
+    hcd1.setBlocksize(4);
+
+    assertTrue(hcd2.equals(hcd1));
+    assertTrue(hcd1.equals(hcd2));
+  }
+
+  @Test
+  public void testEqualityWithSameObject() {
+    HColumnDescriptor hcd1 = new HColumnDescriptor("someName");
+    assertTrue(hcd1.equals(hcd1));
+  }
 }

hbase-server/src/test/java/org/apache/hadoop/hbase/TestHTableDescriptor.java

@@ -198,4 +198,173 @@ public class TestHTableDescriptor {
     desc.removeConfiguration(key);
     assertEquals(null, desc.getConfigurationValue(key));
   }
+
+  @Test
+  public void testEqualsWithDifferentProperties() {
+    // Test basic property difference
+    HTableDescriptor h1 = new HTableDescriptor();
+    h1.setName(Bytes.toBytes("n1"));
+
+    HTableDescriptor h2 = new HTableDescriptor();
+    h2.setName(Bytes.toBytes("n2"));
+
+    assertFalse(h2.equals(h1));
+    assertFalse(h1.equals(h2));
+
+    h2.setName(Bytes.toBytes("n1"));
+    assertTrue(h2.equals(h1));
+    assertTrue(h1.equals(h2));
+  }
+
+  @Test
+  public void testEqualsWithDifferentNumberOfItems() {
+    HTableDescriptor h1 = new HTableDescriptor();
+    HTableDescriptor h2 = new HTableDescriptor();
+
+    // Test diff # of items
+    h1 = new HTableDescriptor();
+    h1.setName(Bytes.toBytes("n1"));
+
+    h2 = new HTableDescriptor();
+    h2.setName(Bytes.toBytes("n1"));
+
+    HColumnDescriptor hcd1 = new HColumnDescriptor(Bytes.toBytes("someName"));
+    HColumnDescriptor hcd2 = new HColumnDescriptor(Bytes.toBytes("someOtherName"));
+
+    h1.addFamily(hcd1);
+    h2.addFamily(hcd1);
+    h1.addFamily(hcd2);
+
+    assertFalse(h2.equals(h1));
+    assertFalse(h1.equals(h2));
+
+    h2.addFamily(hcd2);
+
+    assertTrue(h2.equals(h1));
+    assertTrue(h1.equals(h2));
+  }
+
+  @Test
+  public void testNotEqualsWithDifferentHCDs() {
+    HTableDescriptor h1 = new HTableDescriptor();
+    HTableDescriptor h2 = new HTableDescriptor();
+
+    // Test diff # of items
+    h1 = new HTableDescriptor();
+    h1.setName(Bytes.toBytes("n1"));
+
+    h2 = new HTableDescriptor();
+    h2.setName(Bytes.toBytes("n1"));
+
+    HColumnDescriptor hcd1 = new HColumnDescriptor(Bytes.toBytes("someName"));
+    HColumnDescriptor hcd2 = new HColumnDescriptor(Bytes.toBytes("someOtherName"));
+
+    h1.addFamily(hcd1);
+    h2.addFamily(hcd2);
+
+    assertFalse(h2.equals(h1));
+    assertFalse(h1.equals(h2));
+  }
+
+  @Test
+  public void testEqualsWithDifferentHCDObjects() {
+    HTableDescriptor h1 = new HTableDescriptor();
+    HTableDescriptor h2 = new HTableDescriptor();
+
+    // Test diff # of items
+    h1 = new HTableDescriptor();
+    h1.setName(Bytes.toBytes("n1"));
+
+    h2 = new HTableDescriptor();
+    h2.setName(Bytes.toBytes("n1"));
+
+    HColumnDescriptor hcd1 = new HColumnDescriptor(Bytes.toBytes("someName"));
+    HColumnDescriptor hcd2 = new HColumnDescriptor(Bytes.toBytes("someName"));
+
+    h1.addFamily(hcd1);
+    h2.addFamily(hcd2);
+
+    assertTrue(h2.equals(h1));
+    assertTrue(h1.equals(h2));
+  }
+
+  @Test
+  public void testNotEqualsWithDifferentItems() {
+    HTableDescriptor h1 = new HTableDescriptor();
+    HTableDescriptor h2 = new HTableDescriptor();
+
+    // Test diff # of items
+    h1 = new HTableDescriptor();
+    h1.setName(Bytes.toBytes("n1"));
+
+    h2 = new HTableDescriptor();
+    h2.setName(Bytes.toBytes("n1"));
+
+    HColumnDescriptor hcd1 = new HColumnDescriptor(Bytes.toBytes("someName"));
+    HColumnDescriptor hcd2 = new HColumnDescriptor(Bytes.toBytes("someOtherName"));
+    h1.addFamily(hcd1);
+    h2.addFamily(hcd2);
+
+    assertFalse(h2.equals(h1));
+    assertFalse(h1.equals(h2));
+  }
+
+  @Test
+  public void testEqualsWithDifferentOrderingsOfItems() {
+    HTableDescriptor h1 = new HTableDescriptor();
+    HTableDescriptor h2 = new HTableDescriptor();
+
+    //Test diff # of items
+    h1 = new HTableDescriptor();
+    h1.setName(Bytes.toBytes("n1"));
+
+    h2 = new HTableDescriptor();
+    h2.setName(Bytes.toBytes("n1"));
+
+    HColumnDescriptor hcd1 = new HColumnDescriptor(Bytes.toBytes("someName"));
+    HColumnDescriptor hcd2 = new HColumnDescriptor(Bytes.toBytes("someOtherName"));
+    h1.addFamily(hcd1);
+    h2.addFamily(hcd2);
+    h1.addFamily(hcd2);
+    h2.addFamily(hcd1);
+
+    assertTrue(h2.equals(h1));
+    assertTrue(h1.equals(h2));
+  }
+
+  @Test
+  public void testSingleItemEquals() {
+    HTableDescriptor h1 = new HTableDescriptor();
+    HTableDescriptor h2 = new HTableDescriptor();
+
+    //Test diff # of items
+    h1 = new HTableDescriptor();
+    h1.setName(Bytes.toBytes("n1"));
+
+    h2 = new HTableDescriptor();
+    h2.setName(Bytes.toBytes("n1"));
+
+    HColumnDescriptor hcd1 = new HColumnDescriptor(Bytes.toBytes("someName"));
+    HColumnDescriptor hcd2 = new HColumnDescriptor(Bytes.toBytes("someName"));
+    h1.addFamily(hcd1);
+    h2.addFamily(hcd2);
+
+    assertTrue(h2.equals(h1));
+    assertTrue(h1.equals(h2));
+  }
+
+  @Test
+  public void testEmptyEquals() {
+    HTableDescriptor h1 = new HTableDescriptor();
+    HTableDescriptor h2 = new HTableDescriptor();
+
+    assertTrue(h2.equals(h1));
+    assertTrue(h1.equals(h2));
+  }
+
+  @Test
+  public void testEqualityWithSameObject() {
+    HTableDescriptor htd = new HTableDescriptor("someName");
+    assertTrue(htd.equals(htd));
+  }
 }