You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@accumulo.apache.org by kt...@apache.org on 2014/03/26 23:26:37 UTC
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
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 <kt...@apache.org>
Authored: Wed Mar 26 18:31:16 2014 -0400
Committer: Keith Turner <kt...@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[] {}));