You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hbase.apache.org by jy...@apache.org on 2013/06/06 01:10:55 UTC

svn commit: r1490068 - in /hbase/branches/0.94/src: main/java/org/apache/hadoop/hbase/regionserver/ test/java/org/apache/hadoop/hbase/ test/java/org/apache/hadoop/hbase/regionserver/

Author: jyates
Date: Wed Jun  5 23:10:55 2013
New Revision: 1490068

URL: http://svn.apache.org/r1490068
Log:
HBASE-8684: Table Coprocessor can't access external HTable by default

Added:
    hbase/branches/0.94/src/test/java/org/apache/hadoop/hbase/TestOpenTableInCoprocessor.java
Modified:
    hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/regionserver/CompoundConfiguration.java
    hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/regionserver/RegionCoprocessorHost.java
    hbase/branches/0.94/src/test/java/org/apache/hadoop/hbase/regionserver/TestCompoundConfiguration.java

Modified: hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/regionserver/CompoundConfiguration.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/regionserver/CompoundConfiguration.java?rev=1490068&r1=1490067&r2=1490068&view=diff
==============================================================================
--- hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/regionserver/CompoundConfiguration.java (original)
+++ hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/regionserver/CompoundConfiguration.java Wed Jun  5 23:10:55 2013
@@ -25,10 +25,13 @@ import java.io.IOException;
 import java.io.OutputStream;
 import java.util.ArrayList;
 import java.util.Collection;
+import java.util.HashMap;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
+import java.util.Map.Entry;
 
+import org.apache.commons.collections.iterators.UnmodifiableIterator;
 import org.apache.hadoop.classification.InterfaceAudience;
 import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.hbase.io.ImmutableBytesWritable;
@@ -61,7 +64,7 @@ class CompoundConfiguration extends Conf
 
   // Devs: these APIs are the same contract as their counterparts in
   // Configuration.java
-  private static interface ImmutableConfigMap {
+  private static interface ImmutableConfigMap extends Iterable<Map.Entry<String,String>> {
     String get(String key);
     String getRaw(String key);
     Class<?> getClassByName(String name) throws ClassNotFoundException;
@@ -114,6 +117,11 @@ class CompoundConfiguration extends Conf
       public String toString() {
         return c.toString();
       }
+
+      @Override
+      public Iterator<Entry<String, String>> iterator() {
+        return c.iterator();
+      }
     });
     return this;
   }
@@ -166,6 +174,55 @@ class CompoundConfiguration extends Conf
       public String toString() {
         return m.toString();
       }
+
+      @Override
+      public Iterator<Entry<String, String>> iterator() {
+        final Iterator<Entry<ImmutableBytesWritable, ImmutableBytesWritable>> entries = m
+            .entrySet().iterator();
+        return new Iterator<Entry<String, String>>() {
+
+          @Override
+          public boolean hasNext() {
+            return entries.hasNext();
+          }
+
+          @Override
+          public Entry<String, String> next() {
+            final Entry<ImmutableBytesWritable, ImmutableBytesWritable> e = entries.next();
+            return new Entry<String, String>() {
+
+              @Override
+              public String setValue(String value) {
+                throw new UnsupportedOperationException(
+                    "Cannot set value on entry from a CompoundConfiguration!");
+              }
+
+              @Override
+              public String getValue() {
+                ImmutableBytesWritable bytes = e.getValue();
+                // unlike regular configuration, ImmutableBytesWritableMaps can take a null value
+                if (bytes != null) {
+                  return Bytes.toString(bytes.get(), bytes.getOffset(), bytes.getLength());
+                }
+                return null;
+              }
+
+              @Override
+              public String getKey() {
+                ImmutableBytesWritable bytes = e.getKey();
+                return Bytes.toString(bytes.get(), bytes.getOffset(), bytes.getLength());
+              }
+            };
+          }
+
+          @Override
+          public void remove() {
+            throw new UnsupportedOperationException(
+                "Cannot remove an entry from a CompoundConfiguration iterator");
+          }
+        };
+
+      }
     });
     return this;
   }
@@ -227,6 +284,25 @@ class CompoundConfiguration extends Conf
     return ret;
   }
 
+  @Override
+  public Iterator<Map.Entry<String, String>> iterator() {
+    Map<String, String> ret = new HashMap<String, String>();
+
+    // add in reverse order so that oldest get overridden.
+    if (!configs.isEmpty()) {
+      for (int i = configs.size() - 1; i >= 0; i--) {
+        ImmutableConfigMap map = configs.get(i);
+        Iterator<Map.Entry<String, String>> iter = map.iterator();
+        while (iter.hasNext()) {
+          Map.Entry<String, String> entry = iter.next();
+          ret.put(entry.getKey(), entry.getValue());
+        }
+      }
+    }
+
+    return UnmodifiableIterator.decorate(ret.entrySet().iterator());
+  }
+
   /***************************************************************************
    * You should just ignore everything below this line unless there's a bug in
    * Configuration.java...
@@ -404,11 +480,6 @@ class CompoundConfiguration extends Conf
   }
 
   @Override
-  public Iterator<Map.Entry<String, String>> iterator() {
-    throw new UnsupportedOperationException("Immutable Configuration");
-  }
-
-  @Override
   public void set(String name, String value) {
     throw new UnsupportedOperationException("Immutable Configuration");
   }

Modified: hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/regionserver/RegionCoprocessorHost.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/regionserver/RegionCoprocessorHost.java?rev=1490068&r1=1490067&r2=1490068&view=diff
==============================================================================
--- hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/regionserver/RegionCoprocessorHost.java (original)
+++ hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/regionserver/RegionCoprocessorHost.java Wed Jun  5 23:10:55 2013
@@ -37,6 +37,7 @@ import org.apache.hadoop.conf.Configurat
 import org.apache.hadoop.fs.Path;
 import org.apache.hadoop.hbase.Coprocessor;
 import org.apache.hadoop.hbase.CoprocessorEnvironment;
+import org.apache.hadoop.hbase.HBaseConfiguration;
 import org.apache.hadoop.hbase.HConstants;
 import org.apache.hadoop.hbase.HRegionInfo;
 import org.apache.hadoop.hbase.HTableDescriptor;
@@ -182,7 +183,9 @@ public class RegionCoprocessorHost
             }
             if (cfgSpec != null) {
               cfgSpec = cfgSpec.substring(cfgSpec.indexOf('|') + 1);
-              Configuration newConf = new Configuration(conf);
+              // do an explicit deep copy of the passed configuration
+              Configuration newConf = new Configuration(false);
+              HBaseConfiguration.merge(newConf, conf);
               Matcher m = HConstants.CP_HTD_ATTR_VALUE_PARAM_PATTERN.matcher(cfgSpec);
               while (m.find()) {
                 newConf.set(m.group(1), m.group(2));

Added: hbase/branches/0.94/src/test/java/org/apache/hadoop/hbase/TestOpenTableInCoprocessor.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.94/src/test/java/org/apache/hadoop/hbase/TestOpenTableInCoprocessor.java?rev=1490068&view=auto
==============================================================================
--- hbase/branches/0.94/src/test/java/org/apache/hadoop/hbase/TestOpenTableInCoprocessor.java (added)
+++ hbase/branches/0.94/src/test/java/org/apache/hadoop/hbase/TestOpenTableInCoprocessor.java Wed Jun  5 23:10:55 2013
@@ -0,0 +1,122 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hbase;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+
+import java.io.IOException;
+
+import org.apache.hadoop.hbase.client.HBaseAdmin;
+import org.apache.hadoop.hbase.client.HTable;
+import org.apache.hadoop.hbase.client.HTableInterface;
+import org.apache.hadoop.hbase.client.Put;
+import org.apache.hadoop.hbase.client.Result;
+import org.apache.hadoop.hbase.client.ResultScanner;
+import org.apache.hadoop.hbase.client.Scan;
+import org.apache.hadoop.hbase.coprocessor.BaseRegionObserver;
+import org.apache.hadoop.hbase.coprocessor.ObserverContext;
+import org.apache.hadoop.hbase.coprocessor.RegionCoprocessorEnvironment;
+import org.apache.hadoop.hbase.regionserver.wal.WALEdit;
+import org.apache.hadoop.hbase.util.Bytes;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+/**
+ * Test that a coprocessor can open a connection and write to another table, inside a hook.
+ */
+@Category(MediumTests.class)
+public class TestOpenTableInCoprocessor {
+
+  private static final byte[] otherTable = Bytes.toBytes("otherTable");
+  private static final byte[] family = new byte[] { 'f' };
+
+  private static boolean completed = false;
+
+  /**
+   * Custom coprocessor that just copies the write to another table.
+   */
+  public static class SendToOtherTableCoprocessor extends BaseRegionObserver {
+
+    @Override
+    public void prePut(ObserverContext<RegionCoprocessorEnvironment> e, Put put, WALEdit edit,
+        boolean writeToWAL) throws IOException {
+      HTableInterface table = e.getEnvironment().getTable(otherTable);
+      Put p = new Put(new byte[] { 'a' });
+      p.add(family, null, new byte[] { 'a' });
+      table.put(put);
+      table.flushCommits();
+      completed = true;
+      table.close();
+    }
+
+  }
+
+  @Test
+  public void testCoprocessorCanCreateConnectionToRemoteTable() throws Throwable {
+    HBaseTestingUtility UTIL = new HBaseTestingUtility();
+    HTableDescriptor primary = new HTableDescriptor("primary");
+    primary.addFamily(new HColumnDescriptor(family));
+    // add our coprocessor
+    primary.addCoprocessor(SendToOtherTableCoprocessor.class.getName());
+
+    HTableDescriptor other = new HTableDescriptor(otherTable);
+    other.addFamily(new HColumnDescriptor(family));
+    UTIL.startMiniCluster();
+
+    HBaseAdmin admin = UTIL.getHBaseAdmin();
+    admin.createTable(primary);
+    admin.createTable(other);
+    admin.close();
+
+    HTable table = new HTable(UTIL.getConfiguration(), "primary");
+    Put p = new Put(new byte[] { 'a' });
+    p.add(family, null, new byte[] { 'a' });
+    table.put(p);
+    table.flushCommits();
+    table.close();
+
+    HTable target = new HTable(UTIL.getConfiguration(), otherTable);
+    assertTrue("Didn't complete update to target table!", completed);
+    assertEquals("Didn't find inserted row", 1, getKeyValueCount(target));
+    target.close();
+
+    UTIL.shutdownMiniCluster();
+  }
+
+  /**
+   * Count the number of keyvalue in the table. Scans all possible versions
+   * @param table table to scan
+   * @return number of keyvalues over all rows in the table
+   * @throws IOException
+   */
+  private int getKeyValueCount(HTable table) throws IOException {
+    Scan scan = new Scan();
+    scan.setMaxVersions(Integer.MAX_VALUE - 1);
+
+    ResultScanner results = table.getScanner(scan);
+    int count = 0;
+    for (Result res : results) {
+      count += res.list().size();
+      System.out.println(count + ") " + res);
+    }
+    results.close();
+
+    return count;
+  }
+}
\ No newline at end of file

Modified: hbase/branches/0.94/src/test/java/org/apache/hadoop/hbase/regionserver/TestCompoundConfiguration.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.94/src/test/java/org/apache/hadoop/hbase/regionserver/TestCompoundConfiguration.java?rev=1490068&r1=1490067&r2=1490068&view=diff
==============================================================================
--- hbase/branches/0.94/src/test/java/org/apache/hadoop/hbase/regionserver/TestCompoundConfiguration.java (original)
+++ hbase/branches/0.94/src/test/java/org/apache/hadoop/hbase/regionserver/TestCompoundConfiguration.java Wed Jun  5 23:10:55 2013
@@ -36,6 +36,7 @@ import junit.framework.TestCase;
 @Category(SmallTests.class)
 public class TestCompoundConfiguration extends TestCase {
   private Configuration baseConf;
+  private int baseConfSize;
 
   @Override
   protected void setUp() throws Exception {
@@ -43,6 +44,7 @@ public class TestCompoundConfiguration e
     baseConf.set("A", "1");
     baseConf.setInt("B", 2);
     baseConf.set("C", "3");
+    baseConfSize = baseConf.size();
   }
 
   @Test
@@ -81,6 +83,15 @@ public class TestCompoundConfiguration e
     assertEquals(4, compoundConf.getInt("D", 0));
     assertNull(compoundConf.get("E"));
     assertEquals(6, compoundConf.getInt("F", 6));
+
+    int cnt = 0;
+    for (Map.Entry<String,String> entry : compoundConf) {
+      cnt++;
+      if (entry.getKey().equals("B")) assertEquals("2b", entry.getValue());
+      else if (entry.getKey().equals("G")) assertEquals(null, entry.getValue());
+    }
+    // verify that entries from ImmutableConfigMap's are merged in the iterator's view
+    assertEquals(baseConfSize + 1, cnt);
   }
 
   private ImmutableBytesWritable strToIbw(String s) {
@@ -108,6 +119,44 @@ public class TestCompoundConfiguration e
     assertNull(compoundConf.get("E"));
     assertEquals(6, compoundConf.getInt("F", 6));
     assertNull(compoundConf.get("G"));
+
+    int cnt = 0;
+    for (Map.Entry<String,String> entry : compoundConf) {
+      cnt++;
+      if (entry.getKey().equals("B")) assertEquals("2b", entry.getValue());
+      else if (entry.getKey().equals("G")) assertEquals(null, entry.getValue());
+    }
+    // verify that entries from ImmutableConfigMap's are merged in the iterator's view
+    assertEquals(baseConfSize + 2, cnt);
+  }
+
+  @Test
+  public void testLaterConfigsOverrideEarlier() {
+    Configuration map1 = new Configuration(false);
+    map1.set("A", "2");
+    map1.set("D", "5");
+    Configuration map2 = new Configuration(false);
+    String newValueForA = "3", newValueForB = "4";
+    map2.set("A", newValueForA);
+    map2.set("B", newValueForB);
+
+    CompoundConfiguration compoundConf = new CompoundConfiguration()
+      .add(map1).add(baseConf);
+    assertEquals("1", compoundConf.get("A"));
+    assertEquals("5", compoundConf.get("D"));
+    compoundConf.add(map2);
+    assertEquals(newValueForA, compoundConf.get("A"));
+    assertEquals(newValueForB, compoundConf.get("B"));
+    assertEquals("5", compoundConf.get("D"));
+
+    int cnt = 0;
+    for (Map.Entry<String,String> entry : compoundConf) {
+      cnt++;
+      if (entry.getKey().equals("A")) assertEquals(newValueForA, entry.getValue());
+      else if (entry.getKey().equals("B")) assertEquals(newValueForB, entry.getValue());
+    }
+    // verify that entries from ImmutableConfigMap's are merged in the iterator's view
+    assertEquals(baseConfSize + 1, cnt);
   }
 
   @org.junit.Rule