Skip to content

Commit

Permalink
adds a static method for creating a configuration object
Browse files Browse the repository at this point in the history
Adds static methods for creating PluginEnv.Configuration objects from a
Map<String,String> and updates tests to use them.  These new methods
make it easier for downstream projects to create these object using only
public APIs.
  • Loading branch information
keith-turner committed Oct 7, 2024
1 parent 9a460ce commit faa5249
Show file tree
Hide file tree
Showing 5 changed files with 47 additions and 31 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,10 @@
import java.util.function.Function;
import java.util.function.Supplier;

import org.apache.accumulo.core.conf.ConfigurationCopy;
import org.apache.accumulo.core.conf.DefaultConfiguration;
import org.apache.accumulo.core.data.TableId;
import org.apache.accumulo.core.util.ConfigurationImpl;

/**
* This interface exposes Accumulo system level information to plugins in a stable manner. The
Expand Down Expand Up @@ -131,6 +134,20 @@ interface Configuration extends Iterable<Entry<String,String>> {
* reflected in the returned value.
*/
<T> Supplier<T> getDerived(Function<Configuration,T> computeDerivedValue);

/**
* Creates a configuration object from a map of properties, this is useful for testing.
*
* @param includeDefaults If true will include accumulo's default properties and layer the
* passed in map on top of these.
* @since 4.0.0
*/
static Configuration from(Map<String,String> properties, boolean includeDefaults) {
ConfigurationCopy config = includeDefaults
? new ConfigurationCopy(DefaultConfiguration.getInstance()) : new ConfigurationCopy();
properties.forEach(config::set);
return new ConfigurationImpl(config);
}
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@
*/
package org.apache.accumulo.core.spi.common;

import java.util.Map;

import org.apache.accumulo.core.client.PluginEnvironment;
import org.apache.accumulo.core.data.TableId;

Expand All @@ -39,6 +41,16 @@ public interface ServiceEnvironment extends PluginEnvironment {
*/
interface Configuration extends PluginEnvironment.Configuration {

/**
* Creates a configuration object from a map of properties, this is useful for testing.
*
* @param includeDefaults If true will include accumulo's default properties and layer the
* passed in map on top of these.
* @since 4.0.0
*/
static Configuration from(Map<String,String> properties, boolean includeDefaults) {
return (Configuration) PluginEnvironment.Configuration.from(properties, includeDefaults);
}
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,6 @@
import java.util.regex.Pattern;

import org.apache.accumulo.core.Constants;
import org.apache.accumulo.core.conf.ConfigurationCopy;
import org.apache.accumulo.core.conf.SiteConfiguration;
import org.apache.accumulo.core.data.TableId;
import org.apache.accumulo.core.data.TabletId;
import org.apache.accumulo.core.dataImpl.thrift.TKeyExtent;
Expand All @@ -53,8 +51,8 @@
import org.apache.accumulo.core.spi.balancer.data.TabletMigration;
import org.apache.accumulo.core.spi.balancer.data.TabletServerId;
import org.apache.accumulo.core.spi.balancer.data.TabletStatistics;
import org.apache.accumulo.core.spi.common.ServiceEnvironment;
import org.apache.accumulo.core.tabletserver.thrift.TabletStats;
import org.apache.accumulo.core.util.ConfigurationImpl;
import org.apache.accumulo.core.util.UtilWaitThread;
import org.junit.jupiter.api.Test;

Expand All @@ -67,9 +65,7 @@ public void init(Map<String,String> tableProperties) {
tables.put(BAR.getTableName(), BAR.getId());
tables.put(BAZ.getTableName(), BAZ.getId());

ConfigurationCopy config = new ConfigurationCopy(SiteConfiguration.empty().build());
tableProperties.forEach(config::set);
ConfigurationImpl configImpl = new ConfigurationImpl(config);
var configImpl = ServiceEnvironment.Configuration.from(tableProperties, false);
BalancerEnvironment environment = createMock(BalancerEnvironment.class);
expect(environment.getConfiguration()).andReturn(configImpl).anyTimes();
expect(environment.getTableIdMap()).andReturn(tables).anyTimes();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@
import java.util.stream.Collectors;

import org.apache.accumulo.core.Constants;
import org.apache.accumulo.core.conf.ConfigurationCopy;
import org.apache.accumulo.core.conf.Property;
import org.apache.accumulo.core.data.TableId;
import org.apache.accumulo.core.data.TabletId;
Expand All @@ -52,8 +51,8 @@
import org.apache.accumulo.core.spi.balancer.data.TabletMigration;
import org.apache.accumulo.core.spi.balancer.data.TabletServerId;
import org.apache.accumulo.core.spi.balancer.data.TabletStatistics;
import org.apache.accumulo.core.spi.common.ServiceEnvironment;
import org.apache.accumulo.core.tabletserver.thrift.TabletStats;
import org.apache.accumulo.core.util.ConfigurationImpl;
import org.apache.hadoop.io.Text;
import org.junit.jupiter.api.Test;

Expand Down Expand Up @@ -121,9 +120,9 @@ public List<TabletStatistics> getOnlineTabletsForTable(TabletServerId tserver,
@Test
public void test() {
BalancerEnvironment environment = createMock(BalancerEnvironment.class);
ConfigurationCopy cc = new ConfigurationCopy(
Map.of(Property.TABLE_LOAD_BALANCER.getKey(), TestSimpleLoadBalancer.class.getName()));
ConfigurationImpl tableConfig = new ConfigurationImpl(cc);
var tableConfig = ServiceEnvironment.Configuration.from(
Map.of(Property.TABLE_LOAD_BALANCER.getKey(), TestSimpleLoadBalancer.class.getName()),
false);

Map<String,TableId> tableIdMap = TABLE_ID_MAP.entrySet().stream()
.collect(Collectors.toMap(Map.Entry::getKey, e -> TableId.of(e.getValue())));
Expand Down Expand Up @@ -201,13 +200,11 @@ public String getResourceGroup() {
@Test
public void testNeedsReassignment() {

ConfigurationCopy cc1 =
new ConfigurationCopy(Map.of(TableLoadBalancer.TABLE_ASSIGNMENT_GROUP_PROPERTY, "G1"));
ConfigurationImpl table1Config = new ConfigurationImpl(cc1);

ConfigurationCopy cc2 =
new ConfigurationCopy(Map.of(TableLoadBalancer.TABLE_ASSIGNMENT_GROUP_PROPERTY, "G2"));
ConfigurationImpl table2Config = new ConfigurationImpl(cc2);
var table1Config = ServiceEnvironment.Configuration
.from(Map.of(TableLoadBalancer.TABLE_ASSIGNMENT_GROUP_PROPERTY, "G1"), false);
var table2Config = ServiceEnvironment.Configuration
.from(Map.of(TableLoadBalancer.TABLE_ASSIGNMENT_GROUP_PROPERTY, "G2"), false);
var table3Config = ServiceEnvironment.Configuration.from(Map.of(), false);

var tid1 = TableId.of("1");
var tid2 = TableId.of("2");
Expand All @@ -216,8 +213,7 @@ public void testNeedsReassignment() {
BalancerEnvironment environment = createMock(BalancerEnvironment.class);
expect(environment.getConfiguration(tid1)).andReturn(table1Config).anyTimes();
expect(environment.getConfiguration(tid2)).andReturn(table2Config).anyTimes();
expect(environment.getConfiguration(tid3))
.andReturn(new ConfigurationImpl(new ConfigurationCopy())).anyTimes();
expect(environment.getConfiguration(tid3)).andReturn(table3Config).anyTimes();
replay(environment);

var tls = new TableLoadBalancer() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,17 +39,13 @@
import java.util.stream.IntStream;

import org.apache.accumulo.core.client.admin.compaction.CompactableFile;
import org.apache.accumulo.core.conf.ConfigurationCopy;
import org.apache.accumulo.core.conf.ConfigurationTypeHelper;
import org.apache.accumulo.core.conf.DefaultConfiguration;
import org.apache.accumulo.core.conf.Property;
import org.apache.accumulo.core.conf.SiteConfiguration;
import org.apache.accumulo.core.data.TableId;
import org.apache.accumulo.core.spi.common.ServiceEnvironment;
import org.apache.accumulo.core.spi.common.ServiceEnvironment.Configuration;
import org.apache.accumulo.core.spi.compaction.CompactionPlan.Builder;
import org.apache.accumulo.core.spi.compaction.CompactionPlanner.InitParameters;
import org.apache.accumulo.core.util.ConfigurationImpl;
import org.apache.accumulo.core.util.compaction.CompactionJobImpl;
import org.apache.accumulo.core.util.compaction.CompactionJobPrioritizer;
import org.apache.accumulo.core.util.compaction.CompactionPlanImpl;
Expand All @@ -67,7 +63,7 @@ private static <T> T getOnlyElement(Collection<T> c) {
}

private static final Configuration defaultConf =
new ConfigurationImpl(DefaultConfiguration.getInstance());
ServiceEnvironment.Configuration.from(Map.of(), true);
private static final CompactionServiceId csid = CompactionServiceId.of("cs1");
private static final String prefix = Property.COMPACTION_SERVICE_PREFIX.getKey();

Expand Down Expand Up @@ -182,9 +178,8 @@ public void testRunningCompaction() {

@Test
public void testUserCompaction() {
ConfigurationCopy aconf = new ConfigurationCopy(DefaultConfiguration.getInstance());
aconf.set(prefix + "cs1.planner.opts.maxOpen", "15");
ConfigurationImpl config = new ConfigurationImpl(aconf);
var config = ServiceEnvironment.Configuration
.from(Map.of(prefix + "cs1.planner.opts.maxOpen", "15"), true);

String groups = "[{'group':'small','maxSize':'32M'}, {'group':'medium','maxSize':'128M'},"
+ "{'group':'large','maxSize':'512M'}, {'group':'huge'}]";
Expand Down Expand Up @@ -544,7 +539,7 @@ public void testMaxTabletFiles() {
Map<String,String> overrides = new HashMap<>();
overrides.put(Property.COMPACTION_SERVICE_PREFIX.getKey() + "cs1.planner.opts.maxOpen", "10");
overrides.put(Property.TABLE_FILE_MAX.getKey(), "7");
var conf = new ConfigurationImpl(SiteConfiguration.empty().withOverrides(overrides).build());
var conf = ServiceEnvironment.Configuration.from(overrides, false);

// For this case need to compact three files and the highest ratio that achieves that is 1.8
var planner = createPlanner(conf, groups);
Expand Down Expand Up @@ -620,7 +615,7 @@ public void testMaxTabletFilesNoCompaction() {
Map<String,String> overrides = new HashMap<>();
overrides.put(Property.COMPACTION_SERVICE_PREFIX.getKey() + "cs1.planner.opts.maxOpen", "10");
overrides.put(Property.TABLE_FILE_MAX.getKey(), "7");
var conf = new ConfigurationImpl(SiteConfiguration.empty().withOverrides(overrides).build());
var conf = ServiceEnvironment.Configuration.from(overrides, false);

// ensure that when a compaction would be over the max size limit that it is not planned
var planner = createPlanner(conf, groups);
Expand Down Expand Up @@ -658,7 +653,7 @@ public void testMaxTableFilesFallback() {
overrides.put(Property.COMPACTION_SERVICE_PREFIX.getKey() + "cs1.planner.opts.maxOpen", "10");
overrides.put(Property.TABLE_FILE_MAX.getKey(), "0");
overrides.put(Property.TSERV_SCAN_MAX_OPENFILES.getKey(), "5");
var conf = new ConfigurationImpl(SiteConfiguration.empty().withOverrides(overrides).build());
var conf = ServiceEnvironment.Configuration.from(overrides, false);

var planner = createPlanner(conf, groups);
var all = createCFs(1000, 1.9, 1.8, 1.7, 1.6, 1.5, 1.4, 1.3, 1.2, 1.1);
Expand Down

0 comments on commit faa5249

Please sign in to comment.