From faa5249f2c3b3a62d43d647f13d0f4bded7d9985 Mon Sep 17 00:00:00 2001 From: Keith Turner Date: Mon, 7 Oct 2024 20:38:27 +0000 Subject: [PATCH] adds a static method for creating a configuration object Adds static methods for creating PluginEnv.Configuration objects from a Map and updates tests to use them. These new methods make it easier for downstream projects to create these object using only public APIs. --- .../core/client/PluginEnvironment.java | 17 +++++++++++++ .../core/spi/common/ServiceEnvironment.java | 12 ++++++++++ .../HostRegexTableLoadBalancerTest.java | 8 ++----- .../spi/balancer/TableLoadBalancerTest.java | 24 ++++++++----------- .../RatioBasedCompactionPlannerTest.java | 17 +++++-------- 5 files changed, 47 insertions(+), 31 deletions(-) diff --git a/core/src/main/java/org/apache/accumulo/core/client/PluginEnvironment.java b/core/src/main/java/org/apache/accumulo/core/client/PluginEnvironment.java index 4372a15e616..1d590110b5e 100644 --- a/core/src/main/java/org/apache/accumulo/core/client/PluginEnvironment.java +++ b/core/src/main/java/org/apache/accumulo/core/client/PluginEnvironment.java @@ -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 @@ -131,6 +134,20 @@ interface Configuration extends Iterable> { * reflected in the returned value. */ Supplier getDerived(Function 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 properties, boolean includeDefaults) { + ConfigurationCopy config = includeDefaults + ? new ConfigurationCopy(DefaultConfiguration.getInstance()) : new ConfigurationCopy(); + properties.forEach(config::set); + return new ConfigurationImpl(config); + } } /** diff --git a/core/src/main/java/org/apache/accumulo/core/spi/common/ServiceEnvironment.java b/core/src/main/java/org/apache/accumulo/core/spi/common/ServiceEnvironment.java index 09f65bd6246..5d6fd3abac4 100644 --- a/core/src/main/java/org/apache/accumulo/core/spi/common/ServiceEnvironment.java +++ b/core/src/main/java/org/apache/accumulo/core/spi/common/ServiceEnvironment.java @@ -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; @@ -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 properties, boolean includeDefaults) { + return (Configuration) PluginEnvironment.Configuration.from(properties, includeDefaults); + } } /** diff --git a/core/src/test/java/org/apache/accumulo/core/spi/balancer/HostRegexTableLoadBalancerTest.java b/core/src/test/java/org/apache/accumulo/core/spi/balancer/HostRegexTableLoadBalancerTest.java index f3927dbc3fa..6507c91566f 100644 --- a/core/src/test/java/org/apache/accumulo/core/spi/balancer/HostRegexTableLoadBalancerTest.java +++ b/core/src/test/java/org/apache/accumulo/core/spi/balancer/HostRegexTableLoadBalancerTest.java @@ -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; @@ -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; @@ -67,9 +65,7 @@ public void init(Map 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(); diff --git a/core/src/test/java/org/apache/accumulo/core/spi/balancer/TableLoadBalancerTest.java b/core/src/test/java/org/apache/accumulo/core/spi/balancer/TableLoadBalancerTest.java index 0ce1cff351e..8974924e534 100644 --- a/core/src/test/java/org/apache/accumulo/core/spi/balancer/TableLoadBalancerTest.java +++ b/core/src/test/java/org/apache/accumulo/core/spi/balancer/TableLoadBalancerTest.java @@ -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; @@ -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; @@ -121,9 +120,9 @@ public List 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 tableIdMap = TABLE_ID_MAP.entrySet().stream() .collect(Collectors.toMap(Map.Entry::getKey, e -> TableId.of(e.getValue()))); @@ -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"); @@ -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() { diff --git a/core/src/test/java/org/apache/accumulo/core/spi/compaction/RatioBasedCompactionPlannerTest.java b/core/src/test/java/org/apache/accumulo/core/spi/compaction/RatioBasedCompactionPlannerTest.java index 8811360b6c9..6f14f6eab98 100644 --- a/core/src/test/java/org/apache/accumulo/core/spi/compaction/RatioBasedCompactionPlannerTest.java +++ b/core/src/test/java/org/apache/accumulo/core/spi/compaction/RatioBasedCompactionPlannerTest.java @@ -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; @@ -67,7 +63,7 @@ private static T getOnlyElement(Collection 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(); @@ -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'}]"; @@ -544,7 +539,7 @@ public void testMaxTabletFiles() { Map 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); @@ -620,7 +615,7 @@ public void testMaxTabletFilesNoCompaction() { Map 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); @@ -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);