accumulo-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From md...@apache.org
Subject [1/9] git commit: ACCUMULO-2520 made GC validate data read from !METADATA table
Date Mon, 24 Mar 2014 17:26:56 GMT
Repository: accumulo
Updated Branches:
  refs/heads/1.5.2-SNAPSHOT eac9585b3 -> 0dc92ca12
  refs/heads/1.6.0-SNAPSHOT 4df3959b5 -> 9677fd46b
  refs/heads/master 0af793b54 -> 997f6fe19


ACCUMULO-2520 made GC validate data read from !METADATA table


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

Branch: refs/heads/master
Commit: ca3f77894466caddf71c89d78a98f9fee8fc7fd8
Parents: 4fdefd7
Author: Keith Turner <kturner@apache.org>
Authored: Fri Mar 21 14:58:05 2014 -0400
Committer: Keith Turner <kturner@apache.org>
Committed: Mon Mar 24 10:54:00 2014 -0400

----------------------------------------------------------------------
 .../server/gc/SimpleGarbageCollector.java       | 48 ++++++++++++++++
 .../server/test/GCLotsOfCandidatesTest.java     |  2 +-
 .../server/gc/SimpleGarbageCollectorTest.java   | 58 ++++++++++++++++++++
 .../accumulo/server/gc/TestConfirmDeletes.java  | 35 ++++++++++++
 test/system/auto/simple/gc.py                   |  9 +++
 5 files changed, 151 insertions(+), 1 deletion(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/accumulo/blob/ca3f7789/src/server/src/main/java/org/apache/accumulo/server/gc/SimpleGarbageCollector.java
----------------------------------------------------------------------
diff --git a/src/server/src/main/java/org/apache/accumulo/server/gc/SimpleGarbageCollector.java
b/src/server/src/main/java/org/apache/accumulo/server/gc/SimpleGarbageCollector.java
index 22c3c0e..d09c557 100644
--- a/src/server/src/main/java/org/apache/accumulo/server/gc/SimpleGarbageCollector.java
+++ b/src/server/src/main/java/org/apache/accumulo/server/gc/SimpleGarbageCollector.java
@@ -31,6 +31,7 @@ import java.util.TreeSet;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
 import java.util.concurrent.TimeUnit;
+import java.util.regex.Pattern;
 
 import org.apache.accumulo.cloudtrace.instrument.CountSampler;
 import org.apache.accumulo.cloudtrace.instrument.Sampler;
@@ -108,6 +109,9 @@ public class SimpleGarbageCollector implements Iface {
   
   private static final Logger log = Logger.getLogger(SimpleGarbageCollector.class);
   
+  private static final Pattern dirPattern = Pattern.compile("/[\\p{Punct}\\p{Alnum}&&[^/]]+/[\\p{Punct}\\p{Alnum}&&[^/]]+");
+  private static final Pattern filePattern = Pattern.compile("/[\\p{Punct}\\p{Alnum}&&[^/]]+/[\\p{Punct}\\p{Alnum}&&[^/]]+/[\\p{Punct}\\p{Alnum}&&[^/]]+");
+
   private Instance instance;
   private AuthInfo credentials;
   private long gcStartDelay;
@@ -419,6 +423,43 @@ public class SimpleGarbageCollector implements Iface {
     return new InetSocketAddress(Accumulo.getLocalAddress(new String[] {"--address", address}),
port);
   }
   
+  // visible for testing
+  static boolean isValidFileDelete(String path) {
+    return filePattern.matcher(path).matches();
+  }
+
+  // visible for testing
+  static boolean isValidDirDelete(String path) {
+    return dirPattern.matcher(path).matches();
+  }
+
+  // visible for testing
+  static boolean isValidCandidate(String candidate) {
+    return isValidDirDelete(candidate) || isValidFileDelete(candidate);
+  }
+
+  private void validateDir(String path) {
+    if (!isValidDirDelete(path)) {
+      throw new IllegalArgumentException("Invalid dir " + path);
+    }
+  }
+
+  private void validateFile(String delete) {
+    if (!isValidFileDelete(delete)) {
+      throw new IllegalArgumentException("Invalid file " + delete);
+    }
+  }
+
+  private void removeInvalidCandidates(SortedSet<String> candidates) {
+    for (Iterator<String> iterator = candidates.iterator(); iterator.hasNext();) {
+      String candidate = (String) iterator.next();
+      if (!isValidCandidate(candidate)) {
+        log.error("Ingoring invalid deletion candidate " + candidate);
+        iterator.remove();
+      }
+    }
+  }
+
   /**
    * This method gets a set of candidates for deletion by scanning the METADATA table deleted
flag keyspace
    */
@@ -441,6 +482,8 @@ public class SimpleGarbageCollector implements Iface {
         log.error("Unable to check the filesystem for offline candidates. Removing all candidates
for deletion to be safe.", e);
         candidates.clear();
       }
+
+      removeInvalidCandidates(candidates);
       return candidates;
     }
     
@@ -469,6 +512,7 @@ public class SimpleGarbageCollector implements Iface {
       }
     }
     
+    removeInvalidCandidates(candidates);
     return candidates;
   }
   
@@ -513,6 +557,7 @@ public class SimpleGarbageCollector implements Iface {
       
       for (Entry<Key,Value> entry : scanner) {
         String blipPath = entry.getKey().getRow().toString().substring(Constants.METADATA_BLIP_FLAG_PREFIX.length());
+        validateDir(blipPath);
         Iterator<String> tailIter = candidates.tailSet(blipPath).iterator();
         int count = 0;
         while (tailIter.hasNext()) {
@@ -556,15 +601,18 @@ public class SimpleGarbageCollector implements Iface {
           }
           // WARNING: This line is EXTREMELY IMPORTANT.
           // You MUST REMOVE candidates that are still in use
+          validateFile(delete);
           if (candidates.remove(delete))
             log.debug("Candidate was still in use in the METADATA table: " + delete);
           
           String path = delete.substring(0, delete.lastIndexOf('/'));
+          validateDir(path);
           if (candidates.remove(path))
             log.debug("Candidate was still in use in the METADATA table: " + path);
         } else if (Constants.METADATA_DIRECTORY_COLUMN.hasColumns(entry.getKey())) {
           String table = new String(KeyExtent.tableOfMetadataRow(entry.getKey().getRow()));
           String delete = "/" + table + entry.getValue().toString();
+          validateDir(delete);
           if (candidates.remove(delete))
             log.debug("Candidate was still in use in the METADATA table: " + delete);
         } else

http://git-wip-us.apache.org/repos/asf/accumulo/blob/ca3f7789/src/server/src/main/java/org/apache/accumulo/server/test/GCLotsOfCandidatesTest.java
----------------------------------------------------------------------
diff --git a/src/server/src/main/java/org/apache/accumulo/server/test/GCLotsOfCandidatesTest.java
b/src/server/src/main/java/org/apache/accumulo/server/test/GCLotsOfCandidatesTest.java
index 722fd1a..c984a94 100644
--- a/src/server/src/main/java/org/apache/accumulo/server/test/GCLotsOfCandidatesTest.java
+++ b/src/server/src/main/java/org/apache/accumulo/server/test/GCLotsOfCandidatesTest.java
@@ -45,7 +45,7 @@ public class GCLotsOfCandidatesTest {
     
     for (int i = 0; i < 10000; ++i) {
       final Text emptyText = new Text("");
-      Text row = new Text(String.format("%s%s%020d%s", Constants.METADATA_DELETE_FLAG_PREFIX,
"/", i,
+      Text row = new Text(String.format("%s/%020d/%s", Constants.METADATA_DELETE_FLAG_PREFIX,
i,
           "aaaaaaaaaabbbbbbbbbbccccccccccddddddddddeeeeeeeeeeffffffffffgggggggggghhhhhhhhhhiiiiiiiiiijjjjjjjjjj"));
       Mutation delFlag = new Mutation(row);
       delFlag.put(emptyText, emptyText, new Value(new byte[] {}));

http://git-wip-us.apache.org/repos/asf/accumulo/blob/ca3f7789/src/server/src/test/java/org/apache/accumulo/server/gc/SimpleGarbageCollectorTest.java
----------------------------------------------------------------------
diff --git a/src/server/src/test/java/org/apache/accumulo/server/gc/SimpleGarbageCollectorTest.java
b/src/server/src/test/java/org/apache/accumulo/server/gc/SimpleGarbageCollectorTest.java
new file mode 100644
index 0000000..c89aad3
--- /dev/null
+++ b/src/server/src/test/java/org/apache/accumulo/server/gc/SimpleGarbageCollectorTest.java
@@ -0,0 +1,58 @@
+/*
+ * 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.accumulo.server.gc;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+public class SimpleGarbageCollectorTest {
+  @Test
+  public void testValidation() {
+
+    Assert.assertTrue(SimpleGarbageCollector.isValidDirDelete("/a/b"));
+    Assert.assertTrue(SimpleGarbageCollector.isValidDirDelete("/45/t-00004"));
+    Assert.assertTrue(SimpleGarbageCollector.isValidDirDelete("/45/b-00006"));
+
+    Assert.assertFalse(SimpleGarbageCollector.isValidDirDelete(""));
+    Assert.assertFalse(SimpleGarbageCollector.isValidDirDelete("/"));
+    Assert.assertFalse(SimpleGarbageCollector.isValidDirDelete("/a"));
+    Assert.assertFalse(SimpleGarbageCollector.isValidDirDelete("//a"));
+    Assert.assertFalse(SimpleGarbageCollector.isValidDirDelete("//"));
+    Assert.assertFalse(SimpleGarbageCollector.isValidDirDelete("////"));
+    Assert.assertFalse(SimpleGarbageCollector.isValidDirDelete("/45//t-00004"));
+    Assert.assertFalse(SimpleGarbageCollector.isValidDirDelete("45///b-00006"));
+    Assert.assertFalse(SimpleGarbageCollector.isValidDirDelete("/45/b-00006/C00000.rf"));
+    Assert.assertFalse(SimpleGarbageCollector.isValidDirDelete("/ /b"));
+    Assert.assertFalse(SimpleGarbageCollector.isValidDirDelete("/a/b/c"));
+    Assert.assertFalse(SimpleGarbageCollector.isValidDirDelete("zz/a/b"));
+
+    Assert.assertTrue(SimpleGarbageCollector.isValidFileDelete("/45/t-000c4/C00000.rf"));
+    Assert.assertTrue(SimpleGarbageCollector.isValidFileDelete("/a7/b-00006/F0000a.rf"));
+
+    Assert.assertFalse(SimpleGarbageCollector.isValidFileDelete(""));
+    Assert.assertFalse(SimpleGarbageCollector.isValidFileDelete("/"));
+    Assert.assertFalse(SimpleGarbageCollector.isValidFileDelete("a7/b-00006/F0000a.rf"));
+    Assert.assertFalse(SimpleGarbageCollector.isValidFileDelete("//a7/b-00006/F0000a.rf"));
+    Assert.assertFalse(SimpleGarbageCollector.isValidFileDelete("///"));
+    Assert.assertFalse(SimpleGarbageCollector.isValidFileDelete("//////"));
+    Assert.assertFalse(SimpleGarbageCollector.isValidFileDelete("///F0000a.rf"));
+    Assert.assertFalse(SimpleGarbageCollector.isValidFileDelete("hh/a7/b-00006/F0000a.rf"));
+    Assert.assertFalse(SimpleGarbageCollector.isValidFileDelete("/a7/b-00006/"));
+    Assert.assertFalse(SimpleGarbageCollector.isValidFileDelete("/a7/b-00006"));
+
+  }
+}

http://git-wip-us.apache.org/repos/asf/accumulo/blob/ca3f7789/src/server/src/test/java/org/apache/accumulo/server/gc/TestConfirmDeletes.java
----------------------------------------------------------------------
diff --git a/src/server/src/test/java/org/apache/accumulo/server/gc/TestConfirmDeletes.java
b/src/server/src/test/java/org/apache/accumulo/server/gc/TestConfirmDeletes.java
index be444dd..16a1912 100644
--- a/src/server/src/test/java/org/apache/accumulo/server/gc/TestConfirmDeletes.java
+++ b/src/server/src/test/java/org/apache/accumulo/server/gc/TestConfirmDeletes.java
@@ -53,6 +53,37 @@ public class TestConfirmDeletes {
     return result;
   }
 
+  @Test(expected = IllegalArgumentException.class)
+  public void testInvalidBlip() throws Exception {
+    String metadata[] = new String[] {"1636< file:/b-0001/someFile 10,100", "1636<
last:3353986642a66eb 192.168.117.9:9997",
+        "1636< srv:dir /default_tablet", "1636< srv:flush 2", "1636< srv:lock tservers/192.168.117.9:9997/zlock-0000000000$3353986642a66eb",
+        "1636< srv:time M1328505870023", "1636< ~tab:~pr \0"};
+    String deletes[] = {"~blip/", "~del/1636/b-0001/someFile"};
+
+    test1(metadata, deletes, 1, 0);
+  }
+
+  @Test(expected = IllegalArgumentException.class)
+  public void testInvalidFile() throws Exception {
+    String metadata[] = new String[] {"1636< file:/b-0001/someFile/ooopppsss/garbage 10,100",
"1636< last:3353986642a66eb 192.168.117.9:9997",
+        "1636< srv:dir /default_tablet", "1636< srv:flush 2", "1636< srv:lock tservers/192.168.117.9:9997/zlock-0000000000$3353986642a66eb",
+        "1636< srv:time M1328505870023", "1636< ~tab:~pr \0"};
+    String deletes[] = {"~del/1636/b-0001/someFile"};
+
+    test1(metadata, deletes, 1, 0);
+  }
+
+  @Test
+  public void testInvalidDeletes() throws Exception {
+    // a few invalid deletes, should be ignored
+    String metadata[] = new String[] {"1636< file:/default_tablet/someFile 10,100", "1636<
last:3353986642a66eb 192.168.117.9:9997",
+        "1636< srv:dir /default_tablet", "1636< srv:flush 2", "1636< srv:lock tservers/192.168.117.9:9997/zlock-0000000000$3353986642a66eb",
+        "1636< srv:time M1328505870023", "1636< ~tab:~pr \0"};
+    String deletes[] = {"~del", "~del/"};
+
+    test1(metadata, deletes, 0, 0);
+  }
+
   @Test
   public void test() throws Exception {
     
@@ -88,6 +119,10 @@ public class TestConfirmDeletes {
     deletes = new String[] {"~del/9/default_tablet", "~del/9/default_tablet/someFile"};
     test1(metadata, deletes, 2, 0);
     
+    metadata = new String[] {"1636< file:/b-0001/I0000 10,100", "1636< last:3353986642a66eb
192.168.117.9:9997", "1636< srv:dir /default_tablet",
+        "1636< srv:flush 2", "1636< srv:lock tservers/192.168.117.9:9997/zlock-0000000000$3353986642a66eb",
"1636< srv:time M1328505870023",
+        "1636< ~tab:~pr \0"};
+
     deletes = new String[] {"~blip/1636/b-0001", "~del/1636/b-0001/I0000"};
     test1(metadata, deletes, 1, 0);
   }

http://git-wip-us.apache.org/repos/asf/accumulo/blob/ca3f7789/test/system/auto/simple/gc.py
----------------------------------------------------------------------
diff --git a/test/system/auto/simple/gc.py b/test/system/auto/simple/gc.py
index 4697246..2b7e2c4 100755
--- a/test/system/auto/simple/gc.py
+++ b/test/system/auto/simple/gc.py
@@ -60,6 +60,8 @@ class GCTest(SunnyDayTest):
     def runTest(self):
         self.waitForStop(self.ingester, 60)
         self.shell(self.masterHost(), 'flush -t test_ingest')
+        #following statemets caused GC to delete everything ACCUMULO-2520
+        self.shell(self.masterHost(), 'grant Table.WRITE -u root -t !METADATA\ntable !METADATA\ninsert
~del testDel test valueTest\n')
         self.stop_gc(self.masterHost())
 
         count = self.waitForFileCountToStabilize()
@@ -74,6 +76,13 @@ class GCTest(SunnyDayTest):
                             glob.glob(os.path.join(ACCUMULO_HOME,'logs',ID,'gc_*')))
         out, err = handle.communicate()
         self.assert_(handle.returncode != 0)
+
+        handle = self.runOn(self.masterHost(),
+                            ['grep', '-q', 'Ingoring invalid deletion candidate'] +
+                            glob.glob(os.path.join(ACCUMULO_HOME,'logs',ID,'gc_*')))
+        out, err = handle.communicate()
+        self.assert_(handle.returncode == 0)
+
         self.pkill(self.masterHost(), 'java.*Main gc$', signal.SIGHUP)
         self.wait(gc)
         log.info("Verifying Ingestion")


Mime
View raw message