ACCUMULO-3189 add verification of compaction plan
Project: http://git-wip-us.apache.org/repos/asf/accumulo/repo Commit: http://git-wip-us.apache.org/repos/asf/accumulo/commit/9c4967e8 Tree: http://git-wip-us.apache.org/repos/asf/accumulo/tree/9c4967e8 Diff: http://git-wip-us.apache.org/repos/asf/accumulo/diff/9c4967e8 Branch: refs/heads/master Commit: 9c4967e8fd9bb81d69bffd933eaa5a2834196303 Parents: 43f787d Author: Keith Turner <ktur...@apache.org> Authored: Fri Oct 3 14:29:14 2014 -0400 Committer: Keith Turner <ktur...@apache.org> Committed: Fri Oct 3 14:38:44 2014 -0400 ---------------------------------------------------------------------- .../org/apache/accumulo/tserver/Tablet.java | 4 +- .../tserver/compaction/CompactionPlan.java | 32 +++++++ .../tserver/compaction/WriteParameters.java | 6 ++ .../tserver/compaction/CompactionPlanTest.java | 87 ++++++++++++++++++++ 4 files changed, 128 insertions(+), 1 deletion(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/accumulo/blob/9c4967e8/server/tserver/src/main/java/org/apache/accumulo/tserver/Tablet.java ---------------------------------------------------------------------- diff --git a/server/tserver/src/main/java/org/apache/accumulo/tserver/Tablet.java b/server/tserver/src/main/java/org/apache/accumulo/tserver/Tablet.java index 226f3d8..e345210 100644 --- a/server/tserver/src/main/java/org/apache/accumulo/tserver/Tablet.java +++ b/server/tserver/src/main/java/org/apache/accumulo/tserver/Tablet.java @@ -3128,8 +3128,10 @@ public class Tablet { MajorCompactionRequest request = new MajorCompactionRequest(extent, reason, fs, acuTableConf); request.setFiles(allFiles); plan = strategy.getCompactionPlan(request); - if (plan != null) + if (plan != null) { + plan.validate(allFiles.keySet()); inputFiles.addAll(plan.inputFiles); + } } if (inputFiles.isEmpty()) { http://git-wip-us.apache.org/repos/asf/accumulo/blob/9c4967e8/server/tserver/src/main/java/org/apache/accumulo/tserver/compaction/CompactionPlan.java ---------------------------------------------------------------------- diff --git a/server/tserver/src/main/java/org/apache/accumulo/tserver/compaction/CompactionPlan.java b/server/tserver/src/main/java/org/apache/accumulo/tserver/compaction/CompactionPlan.java index 9417624..6f69fb0 100644 --- a/server/tserver/src/main/java/org/apache/accumulo/tserver/compaction/CompactionPlan.java +++ b/server/tserver/src/main/java/org/apache/accumulo/tserver/compaction/CompactionPlan.java @@ -17,10 +17,15 @@ package org.apache.accumulo.tserver.compaction; import java.util.ArrayList; +import java.util.Collections; +import java.util.HashSet; import java.util.List; +import java.util.Set; import org.apache.accumulo.server.fs.FileRef; +import com.google.common.collect.Sets; + /** * A plan for a compaction: the input files, the files that are *not* inputs to a compaction that should simply be deleted, and the optional parameters used to * create the resulting output file. @@ -52,4 +57,31 @@ public class CompactionPlan { } return b.toString(); } + + /** + * Validate compaction plan. + * + * @param allFiles + * All possible files + * @throws IllegalStateException + * thrown when validation fails. + */ + public final void validate(Set<FileRef> allFiles) { + Set<FileRef> inputSet = new HashSet<FileRef>(inputFiles); + Set<FileRef> deleteSet = new HashSet<FileRef>(deleteFiles); + + if (!allFiles.containsAll(inputSet)) { + inputSet.removeAll(allFiles); + throw new IllegalStateException("plan inputs contains files not in allFiles " + inputSet); + } + + if (!allFiles.containsAll(deleteSet)) { + deleteSet.removeAll(allFiles); + throw new IllegalStateException("plan deletes contains files not in allFiles " + deleteSet); + } + + if (!Collections.disjoint(inputSet, deleteSet)) { + throw new IllegalStateException("plan contains overlap in inputFiles and deleteFiles " + Sets.intersection(inputSet, deleteSet)); + } + } } http://git-wip-us.apache.org/repos/asf/accumulo/blob/9c4967e8/server/tserver/src/main/java/org/apache/accumulo/tserver/compaction/WriteParameters.java ---------------------------------------------------------------------- diff --git a/server/tserver/src/main/java/org/apache/accumulo/tserver/compaction/WriteParameters.java b/server/tserver/src/main/java/org/apache/accumulo/tserver/compaction/WriteParameters.java index 42b4e17..edb862e 100644 --- a/server/tserver/src/main/java/org/apache/accumulo/tserver/compaction/WriteParameters.java +++ b/server/tserver/src/main/java/org/apache/accumulo/tserver/compaction/WriteParameters.java @@ -16,6 +16,8 @@ */ package org.apache.accumulo.tserver.compaction; +import com.google.common.base.Preconditions; + public class WriteParameters { private String compressType = null; private long hdfsBlockSize = 0; @@ -36,6 +38,7 @@ public class WriteParameters { } public void setHdfsBlockSize(long hdfsBlockSize) { + Preconditions.checkArgument(hdfsBlockSize >= 0); this.hdfsBlockSize = hdfsBlockSize; } @@ -44,6 +47,7 @@ public class WriteParameters { } public void setBlockSize(long blockSize) { + Preconditions.checkArgument(blockSize >= 0); this.blockSize = blockSize; } @@ -52,6 +56,7 @@ public class WriteParameters { } public void setIndexBlockSize(long indexBlockSize) { + Preconditions.checkArgument(indexBlockSize >= 0); this.indexBlockSize = indexBlockSize; } @@ -60,6 +65,7 @@ public class WriteParameters { } public void setReplication(int replication) { + Preconditions.checkArgument(replication >= 0); this.replication = replication; } } http://git-wip-us.apache.org/repos/asf/accumulo/blob/9c4967e8/server/tserver/src/test/java/org/apache/accumulo/tserver/compaction/CompactionPlanTest.java ---------------------------------------------------------------------- diff --git a/server/tserver/src/test/java/org/apache/accumulo/tserver/compaction/CompactionPlanTest.java b/server/tserver/src/test/java/org/apache/accumulo/tserver/compaction/CompactionPlanTest.java new file mode 100644 index 0000000..988d87f --- /dev/null +++ b/server/tserver/src/test/java/org/apache/accumulo/tserver/compaction/CompactionPlanTest.java @@ -0,0 +1,87 @@ +/* + * 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.tserver.compaction; + +import java.util.Set; + +import com.google.common.collect.ImmutableSet; +import org.apache.accumulo.server.fs.FileRef; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.ExpectedException; + +public class CompactionPlanTest { + + @Rule + public ExpectedException exception = ExpectedException.none(); + + @Test + public void testOverlappingInputAndDelete() { + CompactionPlan cp1 = new CompactionPlan(); + + FileRef fr1 = new FileRef("hdfs://nn1/accumulo/tables/1/t-1/1.rf"); + FileRef fr2 = new FileRef("hdfs://nn1/accumulo/tables/1/t-1/2.rf"); + + cp1.inputFiles.add(fr1); + + cp1.deleteFiles.add(fr1); + cp1.deleteFiles.add(fr2); + + Set<FileRef> allFiles = ImmutableSet.of(fr1, fr2); + + exception.expect(IllegalStateException.class); + cp1.validate(allFiles); + } + + @Test + public void testInputNotInAllFiles() { + CompactionPlan cp1 = new CompactionPlan(); + + FileRef fr1 = new FileRef("hdfs://nn1/accumulo/tables/1/t-1/1.rf"); + FileRef fr2 = new FileRef("hdfs://nn1/accumulo/tables/1/t-1/2.rf"); + FileRef fr3 = new FileRef("hdfs://nn1/accumulo/tables/1/t-2/3.rf"); + + cp1.inputFiles.add(fr1); + cp1.inputFiles.add(fr2); + cp1.inputFiles.add(fr3); + + Set<FileRef> allFiles = ImmutableSet.of(fr1, fr2); + + exception.expect(IllegalStateException.class); + cp1.validate(allFiles); + } + + @Test + public void testDeleteNotInAllFiles() { + CompactionPlan cp1 = new CompactionPlan(); + + FileRef fr1 = new FileRef("hdfs://nn1/accumulo/tables/1/t-1/1.rf"); + FileRef fr2 = new FileRef("hdfs://nn1/accumulo/tables/1/t-1/2.rf"); + FileRef fr3 = new FileRef("hdfs://nn1/accumulo/tables/1/t-2/3.rf"); + + cp1.deleteFiles.add(fr1); + cp1.deleteFiles.add(fr2); + cp1.deleteFiles.add(fr3); + + Set<FileRef> allFiles = ImmutableSet.of(fr1, fr2); + + exception.expect(IllegalStateException.class); + cp1.validate(allFiles); + } + +}
