accumulo-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ktur...@apache.org
Subject git commit: ACCUMULO-2520 GC changes : disallow a delete marker of hdfs://nn/, log a warning about invalid delete, and add test for bad delete markers
Date Wed, 26 Mar 2014 22:26:37 GMT
Repository: accumulo
Updated Branches:
  refs/heads/1.6.0-SNAPSHOT 20e2b0a6a -> a64151e63


ACCUMULO-2520 GC changes : disallow a delete marker of hdfs://nn/, log a warning about invalid
delete, and add test for bad delete markers


Project: http://git-wip-us.apache.org/repos/asf/accumulo/repo
Commit: http://git-wip-us.apache.org/repos/asf/accumulo/commit/a64151e6
Tree: http://git-wip-us.apache.org/repos/asf/accumulo/tree/a64151e6
Diff: http://git-wip-us.apache.org/repos/asf/accumulo/diff/a64151e6

Branch: refs/heads/1.6.0-SNAPSHOT
Commit: a64151e6362034fac0c5f243c7b04d924f096df0
Parents: 20e2b0a
Author: Keith Turner <kturner@apache.org>
Authored: Wed Mar 26 18:31:16 2014 -0400
Committer: Keith Turner <kturner@apache.org>
Committed: Wed Mar 26 18:31:16 2014 -0400

----------------------------------------------------------------------
 .../accumulo/gc/GarbageCollectionAlgorithm.java | 17 +++---
 .../accumulo/gc/GarbageCollectionTest.java      | 23 +++++++
 .../test/functional/GarbageCollectorIT.java     | 64 +++++++++++++++++++-
 3 files changed, 96 insertions(+), 8 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/accumulo/blob/a64151e6/server/gc/src/main/java/org/apache/accumulo/gc/GarbageCollectionAlgorithm.java
----------------------------------------------------------------------
diff --git a/server/gc/src/main/java/org/apache/accumulo/gc/GarbageCollectionAlgorithm.java
b/server/gc/src/main/java/org/apache/accumulo/gc/GarbageCollectionAlgorithm.java
index 464d0d9..40fb847 100644
--- a/server/gc/src/main/java/org/apache/accumulo/gc/GarbageCollectionAlgorithm.java
+++ b/server/gc/src/main/java/org/apache/accumulo/gc/GarbageCollectionAlgorithm.java
@@ -85,10 +85,7 @@ public class GarbageCollectionAlgorithm {
       tokens = tmp.toArray(new String[tmp.size()]);
     }
 
-    if (tokens.length > 3) {
-      if (!path.contains(":"))
-        throw new IllegalArgumentException(path);
-
+    if (tokens.length > 3 && path.contains(":")) {
       if (tokens[tokens.length - 4].equals(ServerConstants.TABLE_DIR) && (expectedLen
== 0 || expectedLen == 3)) {
         relPath = tokens[tokens.length - 3] + "/" + tokens[tokens.length - 2] + "/" + tokens[tokens.length
- 1];
       } else if (tokens[tokens.length - 3].equals(ServerConstants.TABLE_DIR) && (expectedLen
== 0 || expectedLen == 2)) {
@@ -96,9 +93,9 @@ public class GarbageCollectionAlgorithm {
       } else {
         throw new IllegalArgumentException(path);
       }
-    } else if (tokens.length == 3 && (expectedLen == 0 || expectedLen == 3)) {
+    } else if (tokens.length == 3 && (expectedLen == 0 || expectedLen == 3) &&
!path.contains(":")) {
       relPath = tokens[0] + "/" + tokens[1] + "/" + tokens[2];
-    } else if (tokens.length == 2 && (expectedLen == 0 || expectedLen == 2)) {
+    } else if (tokens.length == 2 && (expectedLen == 0 || expectedLen == 2) &&
!path.contains(":")) {
       relPath = tokens[0] + "/" + tokens[1];
     } else {
       throw new IllegalArgumentException(path);
@@ -112,7 +109,13 @@ public class GarbageCollectionAlgorithm {
     SortedMap<String,String> ret = new TreeMap<String,String>();
 
     for (String candidate : candidates) {
-      String relPath = makeRelative(candidate, 0);
+      String relPath;
+      try {
+        relPath = makeRelative(candidate, 0);
+      } catch (IllegalArgumentException iae) {
+        log.warn("Ingoring invalid deletion candidate " + candidate);
+        continue;
+      }
       ret.put(relPath, candidate);
     }
 

http://git-wip-us.apache.org/repos/asf/accumulo/blob/a64151e6/server/gc/src/test/java/org/apache/accumulo/gc/GarbageCollectionTest.java
----------------------------------------------------------------------
diff --git a/server/gc/src/test/java/org/apache/accumulo/gc/GarbageCollectionTest.java b/server/gc/src/test/java/org/apache/accumulo/gc/GarbageCollectionTest.java
index 1e9c1dd..537fc46 100644
--- a/server/gc/src/test/java/org/apache/accumulo/gc/GarbageCollectionTest.java
+++ b/server/gc/src/test/java/org/apache/accumulo/gc/GarbageCollectionTest.java
@@ -448,6 +448,29 @@ public class GarbageCollectionTest {
   }
 
   @Test
+  public void testBadDeletes() throws Exception {
+    GarbageCollectionAlgorithm gca = new GarbageCollectionAlgorithm();
+
+    TestGCE gce = new TestGCE();
+    gce.candidates.add("");
+    gce.candidates.add("A");
+    gce.candidates.add("/");
+    gce.candidates.add("//");
+    gce.candidates.add("///");
+    gce.candidates.add("////");
+    gce.candidates.add("/1/2/3/4");
+    gce.candidates.add("/a");
+    gce.candidates.add("hdfs://foo.com:6000/accumulo/tbls/5/F00.rf");
+    gce.candidates.add("hdfs://foo.com:6000/");
+    gce.candidates.add("hdfs://foo.com:6000/accumulo/tables/");
+    gce.candidates.add("hdfs://foo.com:6000/user/foo/tables/a/t-0/t-1/F00.rf");
+
+    gca.collect(gce);
+    System.out.println(gce.deletes);
+    assertRemoved(gce);
+  }
+
+  @Test
   public void test() throws Exception {
 
     GarbageCollectionAlgorithm gca = new GarbageCollectionAlgorithm();

http://git-wip-us.apache.org/repos/asf/accumulo/blob/a64151e6/test/src/test/java/org/apache/accumulo/test/functional/GarbageCollectorIT.java
----------------------------------------------------------------------
diff --git a/test/src/test/java/org/apache/accumulo/test/functional/GarbageCollectorIT.java
b/test/src/test/java/org/apache/accumulo/test/functional/GarbageCollectorIT.java
index 3a9b404..8557625 100644
--- a/test/src/test/java/org/apache/accumulo/test/functional/GarbageCollectorIT.java
+++ b/test/src/test/java/org/apache/accumulo/test/functional/GarbageCollectorIT.java
@@ -19,20 +19,25 @@ package org.apache.accumulo.test.functional;
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
 
+import java.io.IOException;
 import java.util.Collections;
 import java.util.HashMap;
+import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
+import java.util.Map.Entry;
 
 import org.apache.accumulo.core.Constants;
 import org.apache.accumulo.core.cli.BatchWriterOpts;
 import org.apache.accumulo.core.cli.ScannerOpts;
 import org.apache.accumulo.core.client.BatchWriter;
+import org.apache.accumulo.core.client.BatchWriterConfig;
 import org.apache.accumulo.core.client.Connector;
 import org.apache.accumulo.core.client.Instance;
 import org.apache.accumulo.core.client.Scanner;
 import org.apache.accumulo.core.client.ZooKeeperInstance;
 import org.apache.accumulo.core.conf.Property;
+import org.apache.accumulo.core.data.Key;
 import org.apache.accumulo.core.data.Mutation;
 import org.apache.accumulo.core.data.Value;
 import org.apache.accumulo.core.metadata.MetadataTable;
@@ -164,6 +169,63 @@ public class GarbageCollectorIT extends ConfigurableMacIT {
     FunctionalTestUtils.count(scanner);
   }
 
+  private Mutation createDelMutation(String path, String cf, String cq, String val) {
+    Text row = new Text(MetadataSchema.DeletesSection.getRowPrefix() + path);
+    Mutation delFlag = new Mutation(row);
+    delFlag.put(cf, cq, val);
+    return delFlag;
+  }
+
+  @Test(timeout = 60 * 1000)
+  public void testInvalidDelete() throws Exception {
+    killMacGc();
+
+    String table = getTableNames(1)[0];
+    getConnector().tableOperations().create(table);
+
+    BatchWriter bw2 = getConnector().createBatchWriter(table, new BatchWriterConfig());
+    Mutation m1 = new Mutation("r1");
+    m1.put("cf1", "cq1", "v1");
+    bw2.addMutation(m1);
+    bw2.close();
+
+    getConnector().tableOperations().flush(table, null, null, true);
+
+    // ensure an invalid delete entry does not cause GC to go berserk ACCUMULO-2520
+    getConnector().securityOperations().grantTablePermission(getConnector().whoami(), MetadataTable.NAME,
TablePermission.WRITE);
+    BatchWriter bw3 = getConnector().createBatchWriter(MetadataTable.NAME, new BatchWriterConfig());
+
+    bw3.addMutation(createDelMutation("", "", "", ""));
+    bw3.addMutation(createDelMutation("", "testDel", "test", "valueTest"));
+    bw3.addMutation(createDelMutation("/", "", "", ""));
+    bw3.close();
+
+    Process gc = cluster.exec(SimpleGarbageCollector.class);
+    try {
+      String output = "";
+      while (!output.contains("Ingoring invalid deletion candidate")) {
+        UtilWaitThread.sleep(250);
+        try {
+          output = FunctionalTestUtils.readAll(cluster, SimpleGarbageCollector.class, gc);
+        } catch (IOException ioe) {
+          ioe.printStackTrace();
+        }
+      }
+    } finally {
+      gc.destroy();
+    }
+
+    Scanner scanner = getConnector().createScanner(table, Authorizations.EMPTY);
+    Iterator<Entry<Key,Value>> iter = scanner.iterator();
+    assertTrue(iter.hasNext());
+    Entry<Key,Value> entry = iter.next();
+    Assert.assertEquals("r1", entry.getKey().getRow().toString());
+    Assert.assertEquals("cf1", entry.getKey().getColumnFamily().toString());
+    Assert.assertEquals("cq1", entry.getKey().getColumnQualifier().toString());
+    Assert.assertEquals("v1", entry.getValue().toString());
+    Assert.assertFalse(iter.hasNext());
+  }
+
   @Test(timeout = 60 * 1000)
   public void testProperPortAdvertisement() throws Exception {
 
@@ -227,7 +289,7 @@ public class GarbageCollectorIT extends ConfigurableMacIT {
 
     for (int i = 0; i < 100000; ++i) {
       final Text emptyText = new Text("");
-      Text row = new Text(String.format("%s%s%020d%s", MetadataSchema.DeletesSection.getRowPrefix(),
"/", i,
+      Text row = new Text(String.format("%s/%020d/%s", MetadataSchema.DeletesSection.getRowPrefix(),
i,
           "aaaaaaaaaabbbbbbbbbbccccccccccddddddddddeeeeeeeeeeffffffffffgggggggggghhhhhhhhhhiiiiiiiiiijjjjjjjjjj"));
       Mutation delFlag = new Mutation(row);
       delFlag.put(emptyText, emptyText, new Value(new byte[] {}));


Mime
View raw message