You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@accumulo.apache.org by bi...@apache.org on 2011/12/01 23:06:00 UTC

svn commit: r1209272 - in /incubator/accumulo/branches/1.4/src/core/src: main/java/org/apache/accumulo/core/client/ main/java/org/apache/accumulo/core/client/admin/ main/java/org/apache/accumulo/core/client/impl/ main/java/org/apache/accumulo/core/util...

Author: billie
Date: Thu Dec  1 22:05:58 2011
New Revision: 1209272

URL: http://svn.apache.org/viewvc?rev=1209272&view=rev
Log:
ACCUMULO-200 added checks that throw errors on conflicts

Modified:
    incubator/accumulo/branches/1.4/src/core/src/main/java/org/apache/accumulo/core/client/ScannerBase.java
    incubator/accumulo/branches/1.4/src/core/src/main/java/org/apache/accumulo/core/client/admin/TableOperations.java
    incubator/accumulo/branches/1.4/src/core/src/main/java/org/apache/accumulo/core/client/admin/TableOperationsHelper.java
    incubator/accumulo/branches/1.4/src/core/src/main/java/org/apache/accumulo/core/client/impl/ScannerOptions.java
    incubator/accumulo/branches/1.4/src/core/src/main/java/org/apache/accumulo/core/util/shell/commands/DeleteIterCommand.java
    incubator/accumulo/branches/1.4/src/core/src/test/java/org/apache/accumulo/core/client/admin/TableOperationsHelperTest.java

Modified: incubator/accumulo/branches/1.4/src/core/src/main/java/org/apache/accumulo/core/client/ScannerBase.java
URL: http://svn.apache.org/viewvc/incubator/accumulo/branches/1.4/src/core/src/main/java/org/apache/accumulo/core/client/ScannerBase.java?rev=1209272&r1=1209271&r2=1209272&view=diff
==============================================================================
--- incubator/accumulo/branches/1.4/src/core/src/main/java/org/apache/accumulo/core/client/ScannerBase.java (original)
+++ incubator/accumulo/branches/1.4/src/core/src/main/java/org/apache/accumulo/core/client/ScannerBase.java Thu Dec  1 22:05:58 2011
@@ -36,7 +36,8 @@ public interface ScannerBase extends Ite
    * @param cfg
    *          fully specified scan-time iterator, including all options for the iterator. Any changes to the iterator setting after this call are not propagated
    *          to the stored iterator.
-   * 
+   * @throws IllegalArgumentException
+   *           if the setting conflicts with existing iterators
    */
   public void addScanIterator(IteratorSetting cfg);
   

Modified: incubator/accumulo/branches/1.4/src/core/src/main/java/org/apache/accumulo/core/client/admin/TableOperations.java
URL: http://svn.apache.org/viewvc/incubator/accumulo/branches/1.4/src/core/src/main/java/org/apache/accumulo/core/client/admin/TableOperations.java?rev=1209272&r1=1209271&r2=1209272&view=diff
==============================================================================
--- incubator/accumulo/branches/1.4/src/core/src/main/java/org/apache/accumulo/core/client/admin/TableOperations.java (original)
+++ incubator/accumulo/branches/1.4/src/core/src/main/java/org/apache/accumulo/core/client/admin/TableOperations.java Thu Dec  1 22:05:58 2011
@@ -471,6 +471,8 @@ public interface TableOperations {
    * @throws AccumuloException
    * @throws TableNotFoundException
    *           throw if the table no longer exists
+   * @throws IllegalArgumentException
+   *           if the setting conflicts with any existing iterators
    */
   public void attachIterator(String tableName, IteratorSetting setting) throws AccumuloSecurityException, AccumuloException, TableNotFoundException;
   
@@ -522,4 +524,19 @@ public interface TableOperations {
    * @throws TableNotFoundException
    */
   public Set<String> listIterators(String tableName) throws AccumuloSecurityException, AccumuloException, TableNotFoundException;
+  
+  /**
+   * Check whether a given iterator configuration conflicts with existing configuration; in particular, determine if the name or priority are already in use for
+   * the specified scopes.
+   * 
+   * @param tableName
+   *          the name of the table
+   * @param setting
+   *          object specifying the properties of the iterator
+   * @throws AccumuloException
+   * @throws TableNotFoundException
+   * @throws IllegalStateException
+   *           if the setting conflicts with any existing iterators
+   */
+  public void checkIteratorConflicts(String tableName, IteratorSetting setting) throws AccumuloException, TableNotFoundException;
 }

Modified: incubator/accumulo/branches/1.4/src/core/src/main/java/org/apache/accumulo/core/client/admin/TableOperationsHelper.java
URL: http://svn.apache.org/viewvc/incubator/accumulo/branches/1.4/src/core/src/main/java/org/apache/accumulo/core/client/admin/TableOperationsHelper.java?rev=1209272&r1=1209271&r2=1209272&view=diff
==============================================================================
--- incubator/accumulo/branches/1.4/src/core/src/main/java/org/apache/accumulo/core/client/admin/TableOperationsHelper.java (original)
+++ incubator/accumulo/branches/1.4/src/core/src/main/java/org/apache/accumulo/core/client/admin/TableOperationsHelper.java Thu Dec  1 22:05:58 2011
@@ -22,6 +22,7 @@ import java.util.HashSet;
 import java.util.Map;
 import java.util.Map.Entry;
 import java.util.Set;
+import java.util.TreeMap;
 
 import org.apache.accumulo.core.client.AccumuloException;
 import org.apache.accumulo.core.client.AccumuloSecurityException;
@@ -34,7 +35,7 @@ public abstract class TableOperationsHel
   
   @Override
   public void attachIterator(String tableName, IteratorSetting setting) throws AccumuloSecurityException, AccumuloException, TableNotFoundException {
-    removeIterator(tableName, setting.getName(), setting.getScopes());
+    checkIteratorConflicts(tableName, setting);
     for (IteratorScope scope : setting.getScopes()) {
       String root = String.format("%s%s.%s", Property.TABLE_ITERATOR_PREFIX, scope.name().toLowerCase(), setting.getName());
       this.setProperty(tableName, root, setting.getPriority() + "," + setting.getIteratorClass());
@@ -103,4 +104,35 @@ public abstract class TableOperationsHel
     }
     return result;
   }
+  
+  @Override
+  public void checkIteratorConflicts(String tableName, IteratorSetting setting) throws AccumuloException, TableNotFoundException {
+    for (IteratorScope scope : setting.getScopes()) {
+      String scopeStr = String.format("%s%s", Property.TABLE_ITERATOR_PREFIX, scope.name().toLowerCase());
+      String nameStr = String.format("%s.%s", scopeStr, setting.getName());
+      String optStr = String.format("%s.opt.", nameStr);
+      Map<String,String> optionConflicts = new TreeMap<String,String>();
+      for (Entry<String,String> property : this.getProperties(tableName)) {
+        if (property.getKey().startsWith(scopeStr)) {
+          if (property.getKey().equals(nameStr))
+            throw new IllegalArgumentException("iterator name conflict for " + setting.getName() + ": " + property.getKey() + "=" + property.getValue());
+          if (property.getKey().startsWith(optStr))
+            optionConflicts.put(property.getKey(), property.getValue());
+          if (property.getKey().contains(".opt."))
+            continue;
+          String parts[] = property.getValue().split(",");
+          if (parts.length != 2)
+            throw new AccumuloException("Bad value for existing iterator setting: " + property.getKey() + "=" + property.getValue());
+          try {
+            if (Integer.parseInt(parts[0]) == setting.getPriority())
+              throw new IllegalArgumentException("iterator priority conflict: " + property.getKey() + "=" + property.getValue());
+          } catch (NumberFormatException e) {
+            throw new AccumuloException("Bad value for existing iterator setting: " + property.getKey() + "=" + property.getValue());
+          }
+        }
+      }
+      if (optionConflicts.size() > 0)
+        throw new IllegalArgumentException("iterator options conflict for " + setting.getName() + ": " + optionConflicts);
+    }
+  }
 }

Modified: incubator/accumulo/branches/1.4/src/core/src/main/java/org/apache/accumulo/core/client/impl/ScannerOptions.java
URL: http://svn.apache.org/viewvc/incubator/accumulo/branches/1.4/src/core/src/main/java/org/apache/accumulo/core/client/impl/ScannerOptions.java?rev=1209272&r1=1209271&r2=1209272&view=diff
==============================================================================
--- incubator/accumulo/branches/1.4/src/core/src/main/java/org/apache/accumulo/core/client/impl/ScannerOptions.java (original)
+++ incubator/accumulo/branches/1.4/src/core/src/main/java/org/apache/accumulo/core/client/impl/ScannerOptions.java Thu Dec  1 22:05:58 2011
@@ -64,9 +64,12 @@ public class ScannerOptions implements S
     if (serverSideIteratorList.size() == 0)
       serverSideIteratorList = new ArrayList<IterInfo>();
     
-    for (IterInfo ii : serverSideIteratorList)
+    for (IterInfo ii : serverSideIteratorList) {
       if (ii.iterName.equals(si.getName()))
         throw new IllegalArgumentException("Iterator name is already in use " + si.getName());
+      if (ii.getPriority() == si.getPriority())
+        throw new IllegalArgumentException("Iterator priority is already in use " + si.getPriority());
+    }
     
     serverSideIteratorList.add(new IterInfo(si.getPriority(), si.getIteratorClass(), si.getName()));
     

Modified: incubator/accumulo/branches/1.4/src/core/src/main/java/org/apache/accumulo/core/util/shell/commands/DeleteIterCommand.java
URL: http://svn.apache.org/viewvc/incubator/accumulo/branches/1.4/src/core/src/main/java/org/apache/accumulo/core/util/shell/commands/DeleteIterCommand.java?rev=1209272&r1=1209271&r2=1209272&view=diff
==============================================================================
--- incubator/accumulo/branches/1.4/src/core/src/main/java/org/apache/accumulo/core/util/shell/commands/DeleteIterCommand.java (original)
+++ incubator/accumulo/branches/1.4/src/core/src/main/java/org/apache/accumulo/core/util/shell/commands/DeleteIterCommand.java Thu Dec  1 22:05:58 2011
@@ -58,6 +58,8 @@ public class DeleteIterCommand extends C
       scopes.add(IteratorScope.majc);
     if (cl.hasOption(scanScopeOpt.getOpt()))
       scopes.add(IteratorScope.scan);
+    if (scopes.isEmpty())
+      throw new IllegalArgumentException("You must select at least one scope to configure");
     shellState.getConnector().tableOperations().removeIterator(tableName, name, scopes);
     return 0;
   }
@@ -95,4 +97,4 @@ public class DeleteIterCommand extends C
   public int numArgs() {
     return 0;
   }
-}
\ No newline at end of file
+}

Modified: incubator/accumulo/branches/1.4/src/core/src/test/java/org/apache/accumulo/core/client/admin/TableOperationsHelperTest.java
URL: http://svn.apache.org/viewvc/incubator/accumulo/branches/1.4/src/core/src/test/java/org/apache/accumulo/core/client/admin/TableOperationsHelperTest.java?rev=1209272&r1=1209271&r2=1209272&view=diff
==============================================================================
--- incubator/accumulo/branches/1.4/src/core/src/test/java/org/apache/accumulo/core/client/admin/TableOperationsHelperTest.java (original)
+++ incubator/accumulo/branches/1.4/src/core/src/test/java/org/apache/accumulo/core/client/admin/TableOperationsHelperTest.java Thu Dec  1 22:05:58 2011
@@ -204,7 +204,7 @@ public class TableOperationsHelperTest {
     t.check("table", new String[] {"table.iterator.scan.someName=10,foo.bar",});
     t.removeIterator("table", "someName", EnumSet.of(IteratorScope.scan));
     t.check("table", new String[] {});
-
+    
     IteratorSetting setting = new IteratorSetting(10, "someName", "foo.bar");
     setting.setScopes(EnumSet.of(IteratorScope.majc));
     setting.addOptions(Collections.singletonMap("key", "value"));
@@ -213,7 +213,7 @@ public class TableOperationsHelperTest {
     t.attachIterator("table", setting);
     t.check("table", new String[] {"table.iterator.majc.someName=10,foo.bar", "table.iterator.majc.someName.opt.key=value",
         "table.iterator.scan.someName=10,foo.bar",});
-
+    
     setting = new IteratorSetting(20, "otherName", "some.classname");
     setting.setScopes(EnumSet.of(IteratorScope.majc));
     setting.addOptions(Collections.singletonMap("key", "value"));
@@ -227,7 +227,7 @@ public class TableOperationsHelperTest {
     t.removeIterator("table", "someName", EnumSet.allOf(IteratorScope.class));
     t.check("table", new String[] {"table.iterator.majc.otherName=20,some.classname", "table.iterator.majc.otherName.opt.key=value",
         "table.iterator.scan.otherName=20,some.classname",});
-
+    
     setting = t.getIteratorSetting("table", "otherName", IteratorScope.scan);
     Assert.assertEquals(20, setting.getPriority());
     Assert.assertEquals("some.classname", setting.getIteratorClass());
@@ -241,6 +241,18 @@ public class TableOperationsHelperTest {
     t.attachIterator("table", setting);
     t.check("table", new String[] {"table.iterator.majc.otherName=20,some.classname", "table.iterator.majc.otherName.opt.key=value",
         "table.iterator.minc.otherName=20,some.classname", "table.iterator.minc.otherName.opt.key=value", "table.iterator.scan.otherName=20,some.classname",});
+    
+    try {
+      t.attachIterator("table", setting);
+      Assert.fail();
+    } catch (IllegalArgumentException e) {}
+    setting.setName("thirdName");
+    try {
+      t.attachIterator("table", setting);
+      Assert.fail();
+    } catch (IllegalArgumentException e) {}
+    setting.setPriority(10);
+    t.attachIterator("table", setting);
   }
   
 }