ddanielr commented on code in PR #4092:
URL: https://github.com/apache/accumulo/pull/4092#discussion_r1433262306
##########
core/src/test/java/org/apache/accumulo/core/spi/compaction/DefaultCompactionPlannerTest.java:
##########
@@ -486,80 +485,36 @@ public Builder createPlanBuilder() {
};
}
- private static DefaultCompactionPlanner createPlanner(boolean
withHugeExecutor) {
- DefaultCompactionPlanner planner = new DefaultCompactionPlanner();
- Configuration conf = EasyMock.createMock(Configuration.class);
-
EasyMock.expect(conf.isSet(EasyMock.anyString())).andReturn(false).anyTimes();
-
- ServiceEnvironment senv = EasyMock.createMock(ServiceEnvironment.class);
- EasyMock.expect(senv.getConfiguration()).andReturn(conf).anyTimes();
-
- EasyMock.replay(conf, senv);
-
- StringBuilder execBldr =
- new StringBuilder("[{'name':'small','type':
'internal','maxSize':'32M','numThreads':1},"
- + "{'name':'medium','type':
'internal','maxSize':'128M','numThreads':2},"
- + "{'name':'large','type':
'internal','maxSize':'512M','numThreads':3}");
-
- if (withHugeExecutor) {
- execBldr.append(",{'name':'huge','type': 'internal','numThreads':4}]");
- } else {
- execBldr.append("]");
- }
-
- String executors = execBldr.toString().replaceAll("'", "\"");
-
- planner.init(new CompactionPlanner.InitParameters() {
+ private static CompactionPlanner.InitParameters getInitParams(Configuration
conf,
+ String executors) {
- @Override
- public ServiceEnvironment getServiceEnvironment() {
- return senv;
+ @SuppressWarnings("removal")
+ var deprecatedMaxOpen = Property.TSERV_MAJC_THREAD_MAXOPEN;
+ String maxOpen;
+ maxOpen =
+ conf.get(Property.TSERV_COMPACTION_SERVICE_PREFIX.getKey() +
"cs1.planner.opts.maxOpen");
+ if (maxOpen == null) {
+ if (conf.isSet(deprecatedMaxOpen.getKey())) {
+ maxOpen = conf.get(deprecatedMaxOpen.getKey());
+ } else {
+ maxOpen =
Property.TSERV_COMPACTION_SERVICE_DEFAULT_MAX_OPEN.getDefaultValue();
Review Comment:
While the DefaultCompactionPlanner code looks to see if the `maxOpen`
property is set, it never uses that value and instead checks the `options` for
the `maxOpen` vialue.
```
this.maxFilesToCompact =
Integer.parseInt(params.getOptions().getOrDefault("maxOpen", "10"));
```
So, for our test we have to include the property in the conf and also in the
`options` map since that is where the property values would eventually end up
after being parsed by CompactionServicesConfig.
Since you pointed that out, I'm going to change the logic in
determineMaxFilesToCompact to make this a bit simpler.
--
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]
