This is an automated email from the ASF dual-hosted git repository.
jmark99 pushed a commit to branch 2.1
in repository https://gitbox.apache.org/repos/asf/accumulo.git
The following commit(s) were added to refs/heads/2.1 by this push:
new 55d972a4c7 Added error details for Compaction Configuration failures
(#3199)
55d972a4c7 is described below
commit 55d972a4c750c81aba8c59f5841d6369ad40d519
Author: Mark Owens <jmar...@apache.org>
AuthorDate: Wed Feb 22 09:29:36 2023 -0500
Added error details for Compaction Configuration failures (#3199)
Added additional error details for a couple of Compaction configuration
failure conditions.
The use of duplicate Compaction Executor ID's in configuration will
return an explicit message detailing the issue in the error output.
Similarly, maxSize errors or null Compaction ID's will detail the issue in
the error output.
---
.../spi/compaction/DefaultCompactionPlanner.java | 4 +-
.../compaction/CompactionPlannerInitParams.java | 4 +-
.../server/conf/CheckCompactionConfigTest.java | 51 ++++++++++++++++++++++
3 files changed, 55 insertions(+), 4 deletions(-)
diff --git
a/core/src/main/java/org/apache/accumulo/core/spi/compaction/DefaultCompactionPlanner.java
b/core/src/main/java/org/apache/accumulo/core/spi/compaction/DefaultCompactionPlanner.java
index 4ffbbb100d..040296e140 100644
---
a/core/src/main/java/org/apache/accumulo/core/spi/compaction/DefaultCompactionPlanner.java
+++
b/core/src/main/java/org/apache/accumulo/core/spi/compaction/DefaultCompactionPlanner.java
@@ -128,8 +128,8 @@ public class DefaultCompactionPlanner implements
CompactionPlanner {
final Long maxSize;
public Executor(CompactionExecutorId ceid, Long maxSize) {
- Preconditions.checkArgument(maxSize == null || maxSize > 0);
- this.ceid = Objects.requireNonNull(ceid);
+ Preconditions.checkArgument(maxSize == null || maxSize > 0, "Invalid
value for maxSize");
+ this.ceid = Objects.requireNonNull(ceid, "Compaction ID is null");
this.maxSize = maxSize;
}
diff --git
a/core/src/main/java/org/apache/accumulo/core/util/compaction/CompactionPlannerInitParams.java
b/core/src/main/java/org/apache/accumulo/core/util/compaction/CompactionPlannerInitParams.java
index bb1093d2b6..0f79ce4df0 100644
---
a/core/src/main/java/org/apache/accumulo/core/util/compaction/CompactionPlannerInitParams.java
+++
b/core/src/main/java/org/apache/accumulo/core/util/compaction/CompactionPlannerInitParams.java
@@ -33,7 +33,6 @@ import
org.apache.accumulo.core.spi.compaction.ExecutorManager;
import com.google.common.base.Preconditions;
public class CompactionPlannerInitParams implements
CompactionPlanner.InitParameters {
-
private final Map<String,String> plannerOpts;
private final Map<CompactionExecutorId,Integer> requestedExecutors;
private final Set<CompactionExecutorId> requestedExternalExecutors;
@@ -72,7 +71,8 @@ public class CompactionPlannerInitParams implements
CompactionPlanner.InitParame
Preconditions.checkArgument(threads > 0, "Positive number of threads
required : %s",
threads);
var ceid = CompactionExecutorIdImpl.internalId(serviceId,
executorName);
- Preconditions.checkState(!getRequestedExecutors().containsKey(ceid));
+ Preconditions.checkState(!getRequestedExecutors().containsKey(ceid),
+ "Duplicate Compaction Executor ID found");
getRequestedExecutors().put(ceid, threads);
return ceid;
}
diff --git
a/server/base/src/test/java/org/apache/accumulo/server/conf/CheckCompactionConfigTest.java
b/server/base/src/test/java/org/apache/accumulo/server/conf/CheckCompactionConfigTest.java
index a1c2737d1a..c80ab28f8d 100644
---
a/server/base/src/test/java/org/apache/accumulo/server/conf/CheckCompactionConfigTest.java
+++
b/server/base/src/test/java/org/apache/accumulo/server/conf/CheckCompactionConfigTest.java
@@ -128,6 +128,57 @@ public class CheckCompactionConfigTest extends
WithTestNames {
assertTrue(e.getMessage().startsWith(expectedErrorMsg));
}
+ @Test
+ public void testRepeatedCompactionExecutorID() throws Exception {
+ String inputString = ("tserver.compaction.major.service.cs1.planner="
+ + "org.apache.accumulo.core.spi.compaction.DefaultCompactionPlanner \n"
+ + "tserver.compaction.major.service.cs1.planner.opts.executors=\\\n"
+ +
"[{'name':'small','type':'internal','maxSize':'16M','numThreads':8},\\\n"
+ +
"{'name':'medium','type':'internal','maxSize':'128M','numThreads':4},\\\n"
+ +
"{'name':'small','type':'internal','numThreads':2}]").replaceAll("'", "\"");
+ String expectedErrorMsg = "Duplicate Compaction Executor ID found";
+
+ String filePath = writeToFileAndReturnPath(inputString);
+
+ var e = assertThrows(IllegalStateException.class,
+ () -> CheckCompactionConfig.main(new String[] {filePath}));
+ assertTrue(e.getMessage().startsWith(expectedErrorMsg));
+ }
+
+ @Test
+ public void testInvalidTypeValue() throws Exception {
+ String inputString = ("tserver.compaction.major.service.cs1.planner="
+ + "org.apache.accumulo.core.spi.compaction.DefaultCompactionPlanner \n"
+ + "tserver.compaction.major.service.cs1.planner.opts.executors=\\\n"
+ +
"[{'name':'small','type':'internal','maxSize':'16M','numThreads':8},\\\n"
+ +
"{'name':'medium','type':'internal','maxSize':'128M','numThreads':4},\\\n"
+ + "{'name':'large','type':'internl','numThreads':2}]").replaceAll("'",
"\"");
+ String expectedErrorMsg = "type must be 'internal' or 'external'";
+
+ String filePath = writeToFileAndReturnPath(inputString);
+
+ var e = assertThrows(IllegalArgumentException.class,
+ () -> CheckCompactionConfig.main(new String[] {filePath}));
+ assertTrue(e.getMessage().startsWith(expectedErrorMsg));
+ }
+
+ @Test
+ public void testInvalidMaxSize() throws Exception {
+ String inputString = ("tserver.compaction.major.service.cs1.planner="
+ + "org.apache.accumulo.core.spi.compaction.DefaultCompactionPlanner \n"
+ + "tserver.compaction.major.service.cs1.planner.opts.executors=\\\n"
+ +
"[{'name':'small','type':'internal','maxSize':'16M','numThreads':8},\\\n"
+ +
"{'name':'medium','type':'internal','maxSize':'0M','numThreads':4},\\\n"
+ +
"{'name':'large','type':'internal','numThreads':2}]").replaceAll("'", "\"");
+ String expectedErrorMsg = "Invalid value for maxSize";
+
+ String filePath = writeToFileAndReturnPath(inputString);
+
+ var e = assertThrows(IllegalArgumentException.class,
+ () -> CheckCompactionConfig.main(new String[] {filePath}));
+ assertTrue(e.getMessage().startsWith(expectedErrorMsg));
+ }
+
@Test
public void testBadPropsFilePath() {
String[] args = {"/home/foo/bar/myProperties.properties"};
