keith-turner commented on code in PR #4083:
URL: https://github.com/apache/accumulo/pull/4083#discussion_r1441163615
##########
core/src/test/java/org/apache/accumulo/core/spi/compaction/DefaultCompactionPlannerTest.java:
##########
@@ -433,114 +424,57 @@ public void
testUserCompactionDoesNotWaitOnSystemCompaction() {
}
@Test
- public void testQueueCreation() throws Exception {
+ public void testQueueCreation() {
DefaultCompactionPlanner planner = new DefaultCompactionPlanner();
- String queues = "[{\"name\": \"small\",
\"maxSize\":\"32M\"},{\"name\":\"midsize\"}]";
- planner.init(getInitParamQueues(defaultConf, queues));
+ String groups = "[{\"name\": \"small\",
\"maxSize\":\"32M\"},{\"name\":\"midsize\"}]";
+ planner.init(getInitParams(defaultConf, groups));
var all = createCFs("F1", "1M", "F2", "1M", "F3", "1M", "F4", "1M");
var params = createPlanningParams(all, all, Set.of(), 2,
CompactionKind.SYSTEM);
var plan = planner.makePlan(params);
var job = getOnlyElement(plan.getJobs());
assertEquals(all, job.getFiles());
- assertEquals(CompactionExecutorIdImpl.externalId("small"),
job.getExecutor());
+ assertEquals(CompactionGroupIdImpl.groupId("small"), job.getGroup());
all = createCFs("F1", "100M", "F2", "100M", "F3", "100M", "F4", "100M");
params = createPlanningParams(all, all, Set.of(), 2,
CompactionKind.SYSTEM);
plan = planner.makePlan(params);
job = getOnlyElement(plan.getJobs());
assertEquals(all, job.getFiles());
- assertEquals(CompactionExecutorIdImpl.externalId("midsize"),
job.getExecutor());
+ assertEquals(CompactionGroupIdImpl.groupId("midsize"), job.getGroup());
}
/**
* Tests that additional fields in the JSON objects cause errors to be
thrown.
*/
@Test
public void testErrorAdditionalConfigFields() {
- DefaultCompactionPlanner QueuePlanner = new DefaultCompactionPlanner();
+ DefaultCompactionPlanner planner = new DefaultCompactionPlanner();
- String queues =
+ String groups =
"[{\"name\":\"smallQueue\", \"maxSize\":\"32M\"},
{\"name\":\"largeQueue\", \"type\":\"internal\", \"foo\":\"bar\",
\"queue\":\"broken\"}]";
- final InitParameters queueParams = getInitParamQueues(defaultConf, queues);
- assertNotNull(queueParams);
- var e = assertThrows(JsonParseException.class, () ->
QueuePlanner.init(queueParams),
- "Failed to throw error");
+ final InitParameters params = getInitParams(defaultConf, groups);
Review Comment:
This test is much simpler now.
##########
core/src/main/java/org/apache/accumulo/core/spi/compaction/DefaultCompactionPlanner.java:
##########
@@ -58,85 +58,48 @@
* compaction service you are configuring.
*
* <ul>
- * <li>{@code compaction.service.<service>.opts.executors} This is a json
array of objects where
- * each object has the fields:
+ * <li>Note that the CompactionCoordinator and at least one Compactor for the
"large" compaction
Review Comment:
Thinking we may want to use the terminology "compactor group" when expanding
"group". If that sounds good there are a few other places that could change.
```suggestion
* <li>Note that the CompactionCoordinator and at least one Compactor for
the "large" compactor
```
##########
core/src/main/java/org/apache/accumulo/core/spi/compaction/DefaultCompactionPlanner.java:
##########
@@ -58,85 +58,48 @@
* compaction service you are configuring.
*
* <ul>
- * <li>{@code compaction.service.<service>.opts.executors} This is a json
array of objects where
- * each object has the fields:
+ * <li>Note that the CompactionCoordinator and at least one Compactor for the
"large" compaction
+ * group must be running.
+ * <li>{@code compaction.service.<service>.opts.maxOpen} This determines the
maximum number of files
+ * that will be included in a single compaction.
+ * <li>{@code compaction.service.<service>.opts.groups} This is a json array
of group objects which
+ * have the following fields:
* <table>
- * <caption>Default Compaction Planner Executor options</caption>
+ * <caption>Default Compaction Planner Group options</caption>
* <tr>
* <th>Field Name</th>
* <th>Description</th>
* </tr>
* <tr>
* <td>name</td>
- * <td>name or alias of the executor (required)</td>
- * </tr>
- * <tr>
- * <td>type</td>
- * <td>valid values 'internal' or 'external' (required)</td>
+ * <td>name of the group (required)</td>
* </tr>
* <tr>
* <td>maxSize</td>
* <td>threshold sum of the input files (required for all but one of the
configs)</td>
* </tr>
- * <tr>
- * <td>numThreads</td>
- * <td>number of threads for this executor configuration (required for
'internal', cannot be
- * specified for 'external')</td>
- * </tr>
- * <tr>
- * <td>group</td>
- * <td>name of the external compaction group (required for 'external', cannot
be specified for
- * 'internal')</td>
- * </tr>
* </table>
* <br>
- * Note: The "executors" option has been deprecated in 3.1 and will be removed
in a future release.
- * This example uses the new `compaction.service` prefix. The property prefix
- * "tserver.compaction.major.service" has also been deprecated in 3.1 and will
be removed in a
- * future release. The maxSize field determines the maximum size of compaction
that will run on an
- * executor. The maxSize field can have a suffix of K,M,G for kilobytes,
megabytes, or gigabytes and
- * represents the sum of the input files for a given compaction. One executor
can have no max size
- * and it will run everything that is too large for the other executors. If
all executors have a max
- * size, then system compactions will only run for compactions smaller than
the largest max size.
- * User, chop, and selector compactions will always run, even if there is no
executor for their
- * size. These compactions will run on the executor with the largest max size.
The following example
- * value for this property will create 3 threads to run compactions of files
whose file size sum is
- * less than 100M, 3 threads to run compactions of files whose file size sum
is less than 500M, and
- * run all other compactions on Compactors configured to run compactions for
Queue1:
+ * This 'groups' object is used for defining compaction groups. The maxSize
field determines the
Review Comment:
```suggestion
* This 'groups' object provides information that is used for mapping a
compaction job to a compactor group. The maxSize field determines the
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
