HBASE-1502 Aftermath; fix up of broke tests. Fix TestMultiParallel. HRegionLocation compare should do Location part only, not include HRI that its carrying

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

src/main/java/org/apache/hadoop/hbase/HRegionLocation.java

@@ -19,18 +19,23 @@
  */
 package org.apache.hadoop.hbase;
 
-import java.net.InetSocketAddress;
-
 import org.apache.hadoop.hbase.util.Addressing;
 
 /**
  * Data structure to hold HRegionInfo and the address for the hosting
- * HRegionServer.  Immutable.
+ * HRegionServer.  Immutable.  Comparable, but we compare the 'location' only:
+ * i.e. the hostname and port, and *not* the regioninfo.  This means two
+ * instances are the same if they refer to the same 'location' (the same
+ * hostname and port), though they may be carrying different regions.
  */
 public class HRegionLocation implements Comparable<HRegionLocation> {
   private final HRegionInfo regionInfo;
   private final String hostname;
   private final int port;
+  // Cache of the 'toString' result.
+  private String cachedString = null;
+  // Cache of the hostname + port
+  private String cachedHostnamePort;
 
   /**
    * Constructor
@@ -49,9 +54,12 @@ public class HRegionLocation implements Comparable<HRegionLocation> {
    * @see java.lang.Object#toString()
    */
   @Override
-  public String toString() {
-    return "region=" + this.regionInfo.getRegionNameAsString() +
+  public synchronized String toString() {
+    if (this.cachedString == null) {
+      this.cachedString = "region=" + this.regionInfo.getRegionNameAsString() +
       ", hostname=" + this.hostname + ", port=" + this.port;
+    }
+    return this.cachedString;
   }
 
   /**
@@ -76,8 +84,7 @@ public class HRegionLocation implements Comparable<HRegionLocation> {
    */
   @Override
   public int hashCode() {
-    int result = this.regionInfo.hashCode();
-    result ^= this.hostname.hashCode();
+    int result = this.hostname.hashCode();
     result ^= this.port;
     return result;
   }
@@ -105,12 +112,12 @@ public class HRegionLocation implements Comparable<HRegionLocation> {
   /**
    * @return String made of hostname and port formatted as per {@link Addressing#createHostAndPortStr(String, int)}
    */
-  public String getHostnamePort() {
-    return Addressing.createHostAndPortStr(this.hostname, this.port);
-  }
-
-  public InetSocketAddress getInetSocketAddress() {
-    return new InetSocketAddress(this.hostname, this.port);
+  public synchronized String getHostnamePort() {
+    if (this.cachedHostnamePort == null) {
+      this.cachedHostnamePort =
+        Addressing.createHostAndPortStr(this.hostname, this.port);
+    }
+    return this.cachedHostnamePort;
   }
 
   //
@@ -118,9 +125,7 @@ public class HRegionLocation implements Comparable<HRegionLocation> {
   //
 
   public int compareTo(HRegionLocation o) {
-    int result = this.regionInfo.compareTo(o.regionInfo);
-    if (result != 0) return result;
-    result = this.hostname.compareTo(o.getHostname());
+    int result = this.hostname.compareTo(o.getHostname());
     if (result != 0) return result;
     return this.port - o.getPort();
   }

src/test/java/org/apache/hadoop/hbase/client/TestMultiParallel.java

@@ -23,7 +23,6 @@ import java.io.IOException;
 import java.lang.reflect.Field;
 import java.util.ArrayList;
 import java.util.List;
-import java.util.concurrent.Executor;
 import java.util.concurrent.ThreadPoolExecutor;
 
 import org.apache.commons.logging.Log;
@@ -52,6 +51,7 @@ public class TestMultiParallel {
   private static final byte [][] KEYS = makeKeys();
 
   private static final int slaves = 2; // also used for testing HTable pool size
+
   @BeforeClass public static void beforeClass() throws Exception {
     UTIL.startMiniCluster(slaves);
     HTable t = UTIL.createTable(Bytes.toBytes(TEST_TABLE), Bytes.toBytes(FAMILY));
@@ -105,6 +105,27 @@ public class TestMultiParallel {
     return keys.toArray(new byte [][] {new byte [] {}});
   }
 
+
+  /**
+   * This is for testing the active number of threads that were used while
+   * doing a batch operation. It inserts one row per region via the batch
+   * operation, and then checks the number of active threads.
+   * For HBASE-3553
+   * @throws IOException
+   * @throws InterruptedException
+   * @throws NoSuchFieldException
+   * @throws SecurityException
+   */
+  @Test public void testActiveThreadsCount() throws Exception{
+    HTable table = new HTable(UTIL.getConfiguration(), TEST_TABLE);
+    List<Row> puts = constructPutRequests(); // creates a Put for every region
+    table.batch(puts);
+    Field poolField = table.getClass().getDeclaredField("pool");
+    poolField.setAccessible(true);
+    ThreadPoolExecutor tExecutor = (ThreadPoolExecutor) poolField.get(table);
+    assertEquals(slaves, tExecutor.getLargestPoolSize());
+  }
+
   @Test public void testBatchWithGet() throws Exception {
     LOG.info("test=testBatchWithGet");
     HTable table = new HTable(UTIL.getConfiguration(), TEST_TABLE);
@@ -466,24 +487,4 @@ public class TestMultiParallel {
       validateEmpty(result);
     }
   }
-
-  /**
-   * This is for testing the active number of threads that were used while
-   * doing a batch operation. It inserts one row per region via the batch
-   * operation, and then checks the number of active threads.
-   * For HBASE-3553
-   * @throws IOException
-   * @throws InterruptedException
-   * @throws NoSuchFieldException
-   * @throws SecurityException
-   */
-  @Test public void testActiveThreadsCount() throws Exception{
-    HTable table = new HTable(UTIL.getConfiguration(), TEST_TABLE);
-    List<Row> puts = constructPutRequests(); // creates a Put for every region
-    table.batch(puts);
-    Field poolField = table.getClass().getDeclaredField("pool");
-    poolField.setAccessible(true);
-    ThreadPoolExecutor tExecutor = (ThreadPoolExecutor) poolField.get(table);
-    assertEquals(slaves, tExecutor.getLargestPoolSize());
-  }
 }