This is an automated email from the ASF dual-hosted git repository. mmiller pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/accumulo.git
The following commit(s) were added to refs/heads/master by this push: new 58e7087 Refactor use of ServerConfigurationFactory (#1572) 58e7087 is described below commit 58e70870db8e1600ee860c984e062efaeac7e4e6 Author: Mike Miller <mmil...@apache.org> AuthorDate: Wed Apr 8 08:38:12 2020 -0400 Refactor use of ServerConfigurationFactory (#1572) * Minimize calls to ServerConfFactory by in-lining methods in ServerContext --- .../org/apache/accumulo/server/ServerContext.java | 37 +++++++++++++++- .../accumulo/server/ServiceEnvironmentImpl.java | 5 +-- .../server/client/ClientServiceHandler.java | 18 +++----- .../accumulo/server/conf/ConfigSanityCheck.java | 2 +- .../accumulo/server/conf/TableConfiguration.java | 7 --- .../accumulo/server/conf/ZooConfiguration.java | 3 +- .../apache/accumulo/server/init/Initialize.java | 7 ++- .../balancer/HostRegexTableLoadBalancer.java | 6 +-- .../server/master/balancer/RegexGroupBalancer.java | 4 +- .../server/master/balancer/TableLoadBalancer.java | 6 +-- .../server/replication/ReplicationUtil.java | 2 +- .../tabletserver/LargestFirstMemoryManager.java | 7 --- .../accumulo/server/util/LoginProperties.java | 2 +- ...tRegexTableLoadBalancerReconfigurationTest.java | 8 +++- .../balancer/HostRegexTableLoadBalancerTest.java | 8 +++- .../master/balancer/TableLoadBalancerTest.java | 51 +++++++--------------- .../java/org/apache/accumulo/master/Master.java | 10 +---- .../apache/accumulo/master/TabletGroupWatcher.java | 3 +- .../accumulo/master/replication/WorkMaker.java | 2 +- .../tableOps/tableExport/WriteExportFiles.java | 2 +- .../accumulo/master/upgrade/Upgrader9to10.java | 5 +-- .../org/apache/accumulo/tracer/TraceServer.java | 9 ++-- .../org/apache/accumulo/tserver/FileManager.java | 5 +-- .../org/apache/accumulo/tserver/TabletServer.java | 28 +++++------- 24 files changed, 110 insertions(+), 127 deletions(-) diff --git a/server/base/src/main/java/org/apache/accumulo/server/ServerContext.java b/server/base/src/main/java/org/apache/accumulo/server/ServerContext.java index 7a48eac..c844fde 100644 --- a/server/base/src/main/java/org/apache/accumulo/server/ServerContext.java +++ b/server/base/src/main/java/org/apache/accumulo/server/ServerContext.java @@ -24,16 +24,23 @@ import java.io.IOException; import org.apache.accumulo.core.clientImpl.ClientContext; import org.apache.accumulo.core.conf.AccumuloConfiguration; +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.crypto.CryptoServiceFactory; import org.apache.accumulo.core.crypto.CryptoServiceFactory.ClassloaderType; +import org.apache.accumulo.core.data.NamespaceId; +import org.apache.accumulo.core.data.TableId; import org.apache.accumulo.core.metadata.schema.Ample; import org.apache.accumulo.core.rpc.SslConnectionParams; import org.apache.accumulo.core.singletons.SingletonReservation; import org.apache.accumulo.core.spi.crypto.CryptoService; +import org.apache.accumulo.fate.zookeeper.ZooCache; import org.apache.accumulo.fate.zookeeper.ZooReaderWriter; +import org.apache.accumulo.server.conf.NamespaceConfiguration; import org.apache.accumulo.server.conf.ServerConfigurationFactory; +import org.apache.accumulo.server.conf.TableConfiguration; +import org.apache.accumulo.server.conf.ZooConfiguration; import org.apache.accumulo.server.fs.VolumeManager; import org.apache.accumulo.server.metadata.ServerAmpleImpl; import org.apache.accumulo.server.rpc.SaslServerConnectionParams; @@ -51,10 +58,12 @@ import org.apache.hadoop.security.UserGroupInformation; public class ServerContext extends ClientContext { private final ServerInfo info; + private final ZooReaderWriter zooReaderWriter; private TableManager tableManager; private UniqueNameAllocator nameAllocator; - private ZooReaderWriter zooReaderWriter; private ServerConfigurationFactory serverConfFactory = null; + private DefaultConfiguration defaultConfig = null; + private AccumuloConfiguration systemConfig = null; private AuthenticationTokenSecretManager secretManager; private CryptoService cryptoService = null; @@ -103,6 +112,10 @@ public class ServerContext extends ClientContext { cryptoService = CryptoServiceFactory.newInstance(acuConf, ClassloaderType.ACCUMULO); } + public SiteConfiguration getSiteConfiguration() { + return info.getSiteConfiguration(); + } + public synchronized ServerConfigurationFactory getServerConfFactory() { if (serverConfFactory == null) { serverConfFactory = new ServerConfigurationFactory(this, info.getSiteConfiguration()); @@ -112,7 +125,26 @@ public class ServerContext extends ClientContext { @Override public AccumuloConfiguration getConfiguration() { - return getServerConfFactory().getSystemConfiguration(); + if (systemConfig == null) { + ZooCache propCache = new ZooCache(getZooKeepers(), getZooKeepersSessionTimeOut()); + systemConfig = new ZooConfiguration(this, propCache, getSiteConfiguration()); + } + return systemConfig; + } + + public TableConfiguration getTableConfiguration(TableId id) { + return getServerConfFactory().getTableConfiguration(id); + } + + public NamespaceConfiguration getNamespaceConfiguration(NamespaceId namespaceId) { + return getServerConfFactory().getNamespaceConfiguration(namespaceId); + } + + public DefaultConfiguration getDefaultConfiguration() { + if (defaultConfig == null) { + defaultConfig = DefaultConfiguration.getInstance(); + } + return defaultConfig; } /** @@ -224,4 +256,5 @@ public class ServerContext extends ClientContext { public Ample getAmple() { return new ServerAmpleImpl(this); } + } diff --git a/server/base/src/main/java/org/apache/accumulo/server/ServiceEnvironmentImpl.java b/server/base/src/main/java/org/apache/accumulo/server/ServiceEnvironmentImpl.java index a7a78d5..28732ea 100644 --- a/server/base/src/main/java/org/apache/accumulo/server/ServiceEnvironmentImpl.java +++ b/server/base/src/main/java/org/apache/accumulo/server/ServiceEnvironmentImpl.java @@ -121,7 +121,7 @@ public class ServiceEnvironmentImpl implements ServiceEnvironment { @Override public Configuration getConfiguration(TableId tableId) { - return new ConfigurationImpl(srvCtx.getServerConfFactory().getTableConfiguration(tableId)); + return new ConfigurationImpl(srvCtx.getTableConfiguration(tableId)); } @Override @@ -138,8 +138,7 @@ public class ServiceEnvironmentImpl implements ServiceEnvironment { @Override public <T> T instantiate(TableId tableId, String className, Class<T> base) throws ReflectiveOperationException, IOException { - String ctx = - srvCtx.getServerConfFactory().getTableConfiguration(tableId).get(Property.TABLE_CLASSPATH); + String ctx = srvCtx.getTableConfiguration(tableId).get(Property.TABLE_CLASSPATH); return ConfigurationTypeHelper.getClassInstance(ctx, className, base); } } diff --git a/server/base/src/main/java/org/apache/accumulo/server/client/ClientServiceHandler.java b/server/base/src/main/java/org/apache/accumulo/server/client/ClientServiceHandler.java index 3c4ab35..bbfa6c0 100644 --- a/server/base/src/main/java/org/apache/accumulo/server/client/ClientServiceHandler.java +++ b/server/base/src/main/java/org/apache/accumulo/server/client/ClientServiceHandler.java @@ -60,7 +60,6 @@ import org.apache.accumulo.core.security.TablePermission; import org.apache.accumulo.core.securityImpl.thrift.TCredentials; import org.apache.accumulo.core.trace.thrift.TInfo; import org.apache.accumulo.server.ServerContext; -import org.apache.accumulo.server.conf.ServerConfigurationFactory; import org.apache.accumulo.server.fs.VolumeManager; import org.apache.accumulo.server.security.AuditedSecurityOperation; import org.apache.accumulo.server.security.SecurityOperation; @@ -310,14 +309,13 @@ public class ClientServiceHandler implements ClientService.Iface { @Override public Map<String,String> getConfiguration(TInfo tinfo, TCredentials credentials, ConfigurationType type) throws TException { - ServerConfigurationFactory factory = context.getServerConfFactory(); switch (type) { case CURRENT: - return conf(credentials, factory.getSystemConfiguration()); + return conf(credentials, context.getConfiguration()); case SITE: - return conf(credentials, factory.getSiteConfiguration()); + return conf(credentials, context.getSiteConfiguration()); case DEFAULT: - return conf(credentials, factory.getDefaultConfiguration()); + return conf(credentials, context.getDefaultConfiguration()); } throw new RuntimeException("Unexpected configuration type " + type); } @@ -326,7 +324,7 @@ public class ClientServiceHandler implements ClientService.Iface { public Map<String,String> getTableConfiguration(TInfo tinfo, TCredentials credentials, String tableName) throws TException, ThriftTableOperationException { TableId tableId = checkTableId(context, tableName, null); - AccumuloConfiguration config = context.getServerConfFactory().getTableConfiguration(tableId); + AccumuloConfiguration config = context.getTableConfiguration(tableId); return conf(credentials, config); } @@ -392,7 +390,7 @@ public class ClientServiceHandler implements ClientService.Iface { try { shouldMatch = loader.loadClass(interfaceMatch); - AccumuloConfiguration conf = context.getServerConfFactory().getTableConfiguration(tableId); + AccumuloConfiguration conf = context.getTableConfiguration(tableId); String context = conf.get(Property.TABLE_CLASSPATH); @@ -427,8 +425,7 @@ public class ClientServiceHandler implements ClientService.Iface { try { shouldMatch = loader.loadClass(interfaceMatch); - AccumuloConfiguration conf = - context.getServerConfFactory().getNamespaceConfiguration(namespaceId); + AccumuloConfiguration conf = context.getNamespaceConfiguration(namespaceId); String context = conf.get(Property.TABLE_CLASSPATH); @@ -489,8 +486,7 @@ public class ClientServiceHandler implements ClientService.Iface { throw new ThriftTableOperationException(null, ns, null, TableOperationExceptionType.NAMESPACE_NOTFOUND, why); } - AccumuloConfiguration config = - context.getServerConfFactory().getNamespaceConfiguration(namespaceId); + AccumuloConfiguration config = context.getNamespaceConfiguration(namespaceId); return conf(credentials, config); } diff --git a/server/base/src/main/java/org/apache/accumulo/server/conf/ConfigSanityCheck.java b/server/base/src/main/java/org/apache/accumulo/server/conf/ConfigSanityCheck.java index ce8c9e2..b31a4c2 100644 --- a/server/base/src/main/java/org/apache/accumulo/server/conf/ConfigSanityCheck.java +++ b/server/base/src/main/java/org/apache/accumulo/server/conf/ConfigSanityCheck.java @@ -29,7 +29,7 @@ public class ConfigSanityCheck implements KeywordExecutable { public static void main(String[] args) { try (var context = new ServerContext(SiteConfiguration.auto())) { - context.getServerConfFactory().getSystemConfiguration(); + context.getConfiguration(); } } diff --git a/server/base/src/main/java/org/apache/accumulo/server/conf/TableConfiguration.java b/server/base/src/main/java/org/apache/accumulo/server/conf/TableConfiguration.java index b065c6d..491421c 100644 --- a/server/base/src/main/java/org/apache/accumulo/server/conf/TableConfiguration.java +++ b/server/base/src/main/java/org/apache/accumulo/server/conf/TableConfiguration.java @@ -141,13 +141,6 @@ public class TableConfiguration extends AccumuloConfiguration { } /** - * returns the actual NamespaceConfiguration that corresponds to the current parent namespace. - */ - public NamespaceConfiguration getNamespaceConfiguration() { - return context.getServerConfFactory().getNamespaceConfiguration(parent.namespaceId); - } - - /** * Gets the parent configuration of this configuration. * * @return parent configuration diff --git a/server/base/src/main/java/org/apache/accumulo/server/conf/ZooConfiguration.java b/server/base/src/main/java/org/apache/accumulo/server/conf/ZooConfiguration.java index 468391f..b611a6e 100644 --- a/server/base/src/main/java/org/apache/accumulo/server/conf/ZooConfiguration.java +++ b/server/base/src/main/java/org/apache/accumulo/server/conf/ZooConfiguration.java @@ -46,8 +46,7 @@ public class ZooConfiguration extends AccumuloConfiguration { private final Map<String,String> fixedProps = Collections.synchronizedMap(new HashMap<>()); private final String propPathPrefix; - protected ZooConfiguration(ServerContext context, ZooCache propCache, - AccumuloConfiguration parent) { + public ZooConfiguration(ServerContext context, ZooCache propCache, AccumuloConfiguration parent) { this.context = context; this.propCache = propCache; this.parent = parent; diff --git a/server/base/src/main/java/org/apache/accumulo/server/init/Initialize.java b/server/base/src/main/java/org/apache/accumulo/server/init/Initialize.java index 09f7aec..d2d5da3 100644 --- a/server/base/src/main/java/org/apache/accumulo/server/init/Initialize.java +++ b/server/base/src/main/java/org/apache/accumulo/server/init/Initialize.java @@ -416,13 +416,12 @@ public class Initialize implements KeywordExecutable { // If they did not, fall back to the credentials present in accumulo.properties that the // servers will use themselves. try { - final var siteConf = context.getServerConfFactory().getSiteConfiguration(); - if (siteConf.getBoolean(Property.INSTANCE_RPC_SASL_ENABLED)) { + if (siteConfig.getBoolean(Property.INSTANCE_RPC_SASL_ENABLED)) { final UserGroupInformation ugi = UserGroupInformation.getCurrentUser(); // We don't have any valid creds to talk to HDFS if (!ugi.hasKerberosCredentials()) { - final String accumuloKeytab = siteConf.get(Property.GENERAL_KERBEROS_KEYTAB), - accumuloPrincipal = siteConf.get(Property.GENERAL_KERBEROS_PRINCIPAL); + final String accumuloKeytab = siteConfig.get(Property.GENERAL_KERBEROS_KEYTAB), + accumuloPrincipal = siteConfig.get(Property.GENERAL_KERBEROS_PRINCIPAL); // Fail if the site configuration doesn't contain appropriate credentials to login as // servers diff --git a/server/base/src/main/java/org/apache/accumulo/server/master/balancer/HostRegexTableLoadBalancer.java b/server/base/src/main/java/org/apache/accumulo/server/master/balancer/HostRegexTableLoadBalancer.java index 3560a47..abf84f4 100644 --- a/server/base/src/main/java/org/apache/accumulo/server/master/balancer/HostRegexTableLoadBalancer.java +++ b/server/base/src/main/java/org/apache/accumulo/server/master/balancer/HostRegexTableLoadBalancer.java @@ -329,15 +329,13 @@ public class HostRegexTableLoadBalancer extends TableLoadBalancer { public void init(ServerContext context) { super.init(context); - this.hrtlbConf = - context.getServerConfFactory().getSystemConfiguration().newDeriver(HrtlbConf::new); + this.hrtlbConf = context.getConfiguration().newDeriver(HrtlbConf::new); tablesRegExCache = CacheBuilder.newBuilder().expireAfterAccess(1, TimeUnit.HOURS).build(new CacheLoader<>() { @Override public Deriver<Map<String,String>> load(TableId key) throws Exception { - return context.getServerConfFactory().getTableConfiguration(key) - .newDeriver(conf -> getRegexes(conf)); + return context.getTableConfiguration(key).newDeriver(conf -> getRegexes(conf)); } }); diff --git a/server/base/src/main/java/org/apache/accumulo/server/master/balancer/RegexGroupBalancer.java b/server/base/src/main/java/org/apache/accumulo/server/master/balancer/RegexGroupBalancer.java index c427bf0..a6a4e48 100644 --- a/server/base/src/main/java/org/apache/accumulo/server/master/balancer/RegexGroupBalancer.java +++ b/server/base/src/main/java/org/apache/accumulo/server/master/balancer/RegexGroupBalancer.java @@ -63,7 +63,7 @@ public class RegexGroupBalancer extends GroupBalancer { @Override protected long getWaitTime() { - Map<String,String> customProps = context.getServerConfFactory().getTableConfiguration(tableId) + Map<String,String> customProps = context.getTableConfiguration(tableId) .getAllPropertiesWithPrefix(Property.TABLE_ARBITRARY_PROP_PREFIX); if (customProps.containsKey(WAIT_TIME_PROPERTY)) { return ConfigurationTypeHelper.getTimeInMillis(customProps.get(WAIT_TIME_PROPERTY)); @@ -75,7 +75,7 @@ public class RegexGroupBalancer extends GroupBalancer { @Override protected Function<KeyExtent,String> getPartitioner() { - Map<String,String> customProps = context.getServerConfFactory().getTableConfiguration(tableId) + Map<String,String> customProps = context.getTableConfiguration(tableId) .getAllPropertiesWithPrefix(Property.TABLE_ARBITRARY_PROP_PREFIX); String regex = customProps.get(REGEX_PROPERTY); final String defaultGroup = customProps.get(DEFAUT_GROUP_PROPERTY); diff --git a/server/base/src/main/java/org/apache/accumulo/server/master/balancer/TableLoadBalancer.java b/server/base/src/main/java/org/apache/accumulo/server/master/balancer/TableLoadBalancer.java index 5ecc41d..0872847 100644 --- a/server/base/src/main/java/org/apache/accumulo/server/master/balancer/TableLoadBalancer.java +++ b/server/base/src/main/java/org/apache/accumulo/server/master/balancer/TableLoadBalancer.java @@ -48,8 +48,7 @@ public class TableLoadBalancer extends TabletBalancer { private TabletBalancer constructNewBalancerForTable(String clazzName, TableId tableId) throws Exception { String context = null; - context = this.context.getServerConfFactory().getTableConfiguration(tableId) - .get(Property.TABLE_CLASSPATH); + context = this.context.getTableConfiguration(tableId).get(Property.TABLE_CLASSPATH); Class<? extends TabletBalancer> clazz; if (context != null && !context.equals("")) clazz = AccumuloVFSClassLoader.getContextManager().loadClass(context, clazzName, @@ -65,8 +64,7 @@ public class TableLoadBalancer extends TabletBalancer { if (tableState == null) return null; if (tableState.equals(TableState.ONLINE)) - return this.context.getServerConfFactory().getTableConfiguration(table) - .get(Property.TABLE_LOAD_BALANCER); + return this.context.getTableConfiguration(table).get(Property.TABLE_LOAD_BALANCER); return null; } diff --git a/server/base/src/main/java/org/apache/accumulo/server/replication/ReplicationUtil.java b/server/base/src/main/java/org/apache/accumulo/server/replication/ReplicationUtil.java index e34766f..926d0f9 100644 --- a/server/base/src/main/java/org/apache/accumulo/server/replication/ReplicationUtil.java +++ b/server/base/src/main/java/org/apache/accumulo/server/replication/ReplicationUtil.java @@ -124,7 +124,7 @@ public class ReplicationUtil { continue; } - TableConfiguration tableConf = context.getServerConfFactory().getTableConfiguration(localId); + TableConfiguration tableConf = context.getTableConfiguration(localId); if (tableConf == null) { log.trace("Could not get configuration for table {} (it no longer exists)", table); continue; diff --git a/server/base/src/main/java/org/apache/accumulo/server/tabletserver/LargestFirstMemoryManager.java b/server/base/src/main/java/org/apache/accumulo/server/tabletserver/LargestFirstMemoryManager.java index dda6d13..dedc4a6 100644 --- a/server/base/src/main/java/org/apache/accumulo/server/tabletserver/LargestFirstMemoryManager.java +++ b/server/base/src/main/java/org/apache/accumulo/server/tabletserver/LargestFirstMemoryManager.java @@ -121,13 +121,6 @@ public class LargestFirstMemoryManager implements MemoryManager { } } - LargestFirstMemoryManager(long maxMemory, int maxConcurrentMincs, int numWaitingMultiplier) { - this(); - this.maxMemory = maxMemory; - this.maxConcurrentMincs = maxConcurrentMincs; - this.numWaitingMultiplier = numWaitingMultiplier; - } - @Override public void init(ServerConfiguration conf) { this.config = conf; diff --git a/server/base/src/main/java/org/apache/accumulo/server/util/LoginProperties.java b/server/base/src/main/java/org/apache/accumulo/server/util/LoginProperties.java index 47039c8..3b0633d 100644 --- a/server/base/src/main/java/org/apache/accumulo/server/util/LoginProperties.java +++ b/server/base/src/main/java/org/apache/accumulo/server/util/LoginProperties.java @@ -46,7 +46,7 @@ public class LoginProperties implements KeywordExecutable { @Override public void execute(String[] args) throws Exception { try (var context = new ServerContext(SiteConfiguration.auto())) { - AccumuloConfiguration config = context.getServerConfFactory().getSystemConfiguration(); + AccumuloConfiguration config = context.getConfiguration(); Authenticator authenticator = AccumuloVFSClassLoader.getClassLoader() .loadClass(config.get(Property.INSTANCE_SECURITY_AUTHENTICATOR)) .asSubclass(Authenticator.class).getDeclaredConstructor().newInstance(); diff --git a/server/base/src/test/java/org/apache/accumulo/server/master/balancer/HostRegexTableLoadBalancerReconfigurationTest.java b/server/base/src/test/java/org/apache/accumulo/server/master/balancer/HostRegexTableLoadBalancerReconfigurationTest.java index 79fcd52..6ebf71d 100644 --- a/server/base/src/test/java/org/apache/accumulo/server/master/balancer/HostRegexTableLoadBalancerReconfigurationTest.java +++ b/server/base/src/test/java/org/apache/accumulo/server/master/balancer/HostRegexTableLoadBalancerReconfigurationTest.java @@ -54,7 +54,13 @@ public class HostRegexTableLoadBalancerReconfigurationTest replay(context1); final TestServerConfigurationFactory factory = new TestServerConfigurationFactory(context1); ServerContext context2 = createMockContext(); - expect(context2.getServerConfFactory()).andReturn(factory).anyTimes(); + expect(context2.getConfiguration()).andReturn(factory.getSystemConfiguration()).anyTimes(); + expect(context2.getTableConfiguration(FOO.getId())) + .andReturn(factory.getTableConfiguration(FOO.getId())).anyTimes(); + expect(context2.getTableConfiguration(BAR.getId())) + .andReturn(factory.getTableConfiguration(BAR.getId())).anyTimes(); + expect(context2.getTableConfiguration(BAZ.getId())) + .andReturn(factory.getTableConfiguration(BAZ.getId())).anyTimes(); replay(context2); init(context2); Map<KeyExtent,TServerInstance> unassigned = new HashMap<>(); diff --git a/server/base/src/test/java/org/apache/accumulo/server/master/balancer/HostRegexTableLoadBalancerTest.java b/server/base/src/test/java/org/apache/accumulo/server/master/balancer/HostRegexTableLoadBalancerTest.java index 81899b1..c8057f0 100644 --- a/server/base/src/test/java/org/apache/accumulo/server/master/balancer/HostRegexTableLoadBalancerTest.java +++ b/server/base/src/test/java/org/apache/accumulo/server/master/balancer/HostRegexTableLoadBalancerTest.java @@ -62,7 +62,13 @@ public class HostRegexTableLoadBalancerTest extends BaseHostRegexTableLoadBalanc private void initFactory(ServerConfigurationFactory factory) { ServerContext context = createMockContext(); - expect(context.getServerConfFactory()).andReturn(factory).anyTimes(); + expect(context.getConfiguration()).andReturn(factory.getSystemConfiguration()).anyTimes(); + expect(context.getTableConfiguration(FOO.getId())) + .andReturn(factory.getTableConfiguration(FOO.getId())).anyTimes(); + expect(context.getTableConfiguration(BAR.getId())) + .andReturn(factory.getTableConfiguration(BAR.getId())).anyTimes(); + expect(context.getTableConfiguration(BAZ.getId())) + .andReturn(factory.getTableConfiguration(BAZ.getId())).anyTimes(); replay(context); init(context); } diff --git a/server/base/src/test/java/org/apache/accumulo/server/master/balancer/TableLoadBalancerTest.java b/server/base/src/test/java/org/apache/accumulo/server/master/balancer/TableLoadBalancerTest.java index 8e11b33..37e9225 100644 --- a/server/base/src/test/java/org/apache/accumulo/server/master/balancer/TableLoadBalancerTest.java +++ b/server/base/src/test/java/org/apache/accumulo/server/master/balancer/TableLoadBalancerTest.java @@ -18,6 +18,8 @@ */ package org.apache.accumulo.server.master.balancer; +import static org.easymock.EasyMock.createMock; +import static org.easymock.EasyMock.expect; import static org.easymock.EasyMock.replay; import static org.junit.Assert.assertEquals; @@ -33,9 +35,7 @@ import java.util.TreeMap; import java.util.UUID; import org.apache.accumulo.core.client.admin.TableOperations; -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.dataImpl.KeyExtent; import org.apache.accumulo.core.master.thrift.TableInfo; @@ -43,8 +43,6 @@ import org.apache.accumulo.core.master.thrift.TabletServerStatus; import org.apache.accumulo.core.tabletserver.thrift.TabletStats; import org.apache.accumulo.core.util.HostAndPort; import org.apache.accumulo.server.ServerContext; -import org.apache.accumulo.server.conf.NamespaceConfiguration; -import org.apache.accumulo.server.conf.ServerConfigurationFactory; import org.apache.accumulo.server.conf.TableConfiguration; import org.apache.accumulo.server.master.state.TServerInstance; import org.apache.accumulo.server.master.state.TabletMigration; @@ -126,49 +124,32 @@ public class TableLoadBalancerTest { @Override protected TableOperations getTableOperations() { - TableOperations tops = EasyMock.createMock(TableOperations.class); - EasyMock.expect(tops.tableIdMap()).andReturn(TABLE_ID_MAP).anyTimes(); + TableOperations tops = createMock(TableOperations.class); + expect(tops.tableIdMap()).andReturn(TABLE_ID_MAP).anyTimes(); replay(tops); return tops; } } private ServerContext createMockContext() { - ServerContext context = EasyMock.createMock(ServerContext.class); + ServerContext context = createMock(ServerContext.class); final String instanceId = UUID.nameUUIDFromBytes(new byte[] {1, 2, 3, 4, 5, 6, 7, 8, 9, 0}).toString(); - EasyMock.expect(context.getProperties()).andReturn(new Properties()).anyTimes(); - EasyMock.expect(context.getInstanceID()).andReturn(instanceId).anyTimes(); - EasyMock.expect(context.getZooKeepers()).andReturn("10.0.0.1:1234").anyTimes(); - EasyMock.expect(context.getZooKeepersSessionTimeOut()).andReturn(30_000).anyTimes(); - EasyMock.expect(context.getZooKeeperRoot()).andReturn("/root/").anyTimes(); + expect(context.getProperties()).andReturn(new Properties()).anyTimes(); + expect(context.getInstanceID()).andReturn(instanceId).anyTimes(); + expect(context.getZooKeepers()).andReturn("10.0.0.1:1234").anyTimes(); + expect(context.getZooKeepersSessionTimeOut()).andReturn(30_000).anyTimes(); + expect(context.getZooKeeperRoot()).andReturn("/root/").anyTimes(); return context; } @Test public void test() { final ServerContext context = createMockContext(); - replay(context); - ServerConfigurationFactory confFactory = - new ServerConfigurationFactory(context, SiteConfiguration.auto()) { - @Override - public TableConfiguration getTableConfiguration(TableId tableId) { - // create a dummy namespaceConfiguration to satisfy requireNonNull in TableConfiguration - // constructor - NamespaceConfiguration dummyConf = new NamespaceConfiguration(null, context, null); - return new TableConfiguration(context, tableId, dummyConf) { - @Override - public String get(Property property) { - // fake the get table configuration so the test doesn't try to look in zookeeper for - // per-table classpath stuff - return DefaultConfiguration.getInstance().get(property); - } - }; - } - }; - final ServerContext context2 = createMockContext(); - EasyMock.expect(context2.getServerConfFactory()).andReturn(confFactory).anyTimes(); - replay(context2); + TableConfiguration conf = createMock(TableConfiguration.class); + expect(conf.get(Property.TABLE_CLASSPATH)).andReturn("").anyTimes(); + expect(context.getTableConfiguration(EasyMock.anyObject())).andReturn(conf).anyTimes(); + replay(context, conf); String t1Id = TABLE_ID_MAP.get("t1"), t2Id = TABLE_ID_MAP.get("t2"), t3Id = TABLE_ID_MAP.get("t3"); @@ -179,13 +160,13 @@ public class TableLoadBalancerTest { Set<KeyExtent> migrations = Collections.emptySet(); List<TabletMigration> migrationsOut = new ArrayList<>(); TableLoadBalancer tls = new TableLoadBalancer(); - tls.init(context2); + tls.init(context); tls.balance(state, migrations, migrationsOut); assertEquals(0, migrationsOut.size()); state.put(mkts("10.0.0.2", "0x02030405"), status()); tls = new TableLoadBalancer(); - tls.init(context2); + tls.init(context); tls.balance(state, migrations, migrationsOut); int count = 0; Map<TableId,Integer> movedByTable = new HashMap<>(); diff --git a/server/master/src/main/java/org/apache/accumulo/master/Master.java b/server/master/src/main/java/org/apache/accumulo/master/Master.java index be683d8..9996d2e 100644 --- a/server/master/src/main/java/org/apache/accumulo/master/Master.java +++ b/server/master/src/main/java/org/apache/accumulo/master/Master.java @@ -99,7 +99,6 @@ import org.apache.accumulo.server.AbstractServer; import org.apache.accumulo.server.HighlyAvailableService; import org.apache.accumulo.server.ServerContext; import org.apache.accumulo.server.ServerOpts; -import org.apache.accumulo.server.conf.ServerConfigurationFactory; import org.apache.accumulo.server.fs.VolumeManager; import org.apache.accumulo.server.log.WalStateManager; import org.apache.accumulo.server.log.WalStateManager.WalMarkerException; @@ -275,8 +274,6 @@ public class Master extends AbstractServer private Future<Void> upgradeMetadataFuture; - private final ServerConfigurationFactory serverConfig; - private MasterClientServiceHandler clientHandler; private int assignedOrHosted(TableId tableId) { @@ -374,9 +371,8 @@ public class Master extends AbstractServer Master(ServerOpts opts, String[] args) throws IOException { super("master", opts, args); ServerContext context = super.getContext(); - this.serverConfig = context.getServerConfFactory(); - AccumuloConfiguration aconf = serverConfig.getSystemConfiguration(); + AccumuloConfiguration aconf = context.getConfiguration(); log.info("Version {}", Constants.VERSION); log.info("Instance {}", getInstanceID()); @@ -1591,10 +1587,6 @@ public class Master extends AbstractServer return nextEvent; } - public ServerConfigurationFactory getConfigurationFactory() { - return serverConfig; - } - public VolumeManager getVolumeManager() { return getContext().getVolumeManager(); } diff --git a/server/master/src/main/java/org/apache/accumulo/master/TabletGroupWatcher.java b/server/master/src/main/java/org/apache/accumulo/master/TabletGroupWatcher.java index 5f29286..6dc3919 100644 --- a/server/master/src/main/java/org/apache/accumulo/master/TabletGroupWatcher.java +++ b/server/master/src/main/java/org/apache/accumulo/master/TabletGroupWatcher.java @@ -211,8 +211,7 @@ abstract class TabletGroupWatcher extends Daemon { eventListener.waitForEvents(Master.TIME_TO_WAIT_BETWEEN_SCANS); } TableId tableId = tls.extent.getTableId(); - TableConfiguration tableConf = - this.master.getConfigurationFactory().getTableConfiguration(tableId); + TableConfiguration tableConf = this.master.getContext().getTableConfiguration(tableId); MergeStats mergeStats = mergeStatsCache.get(tableId); if (mergeStats == null) { diff --git a/server/master/src/main/java/org/apache/accumulo/master/replication/WorkMaker.java b/server/master/src/main/java/org/apache/accumulo/master/replication/WorkMaker.java index db57c4e..174852b 100644 --- a/server/master/src/main/java/org/apache/accumulo/master/replication/WorkMaker.java +++ b/server/master/src/main/java/org/apache/accumulo/master/replication/WorkMaker.java @@ -115,7 +115,7 @@ public class WorkMaker { } // Get the table configuration for the table specified by the status record - tableConf = context.getServerConfFactory().getTableConfiguration(tableId); + tableConf = context.getTableConfiguration(tableId); // getTableConfiguration(String) returns null if the table no longer exists if (tableConf == null) { diff --git a/server/master/src/main/java/org/apache/accumulo/master/tableOps/tableExport/WriteExportFiles.java b/server/master/src/main/java/org/apache/accumulo/master/tableOps/tableExport/WriteExportFiles.java index 0227817..a65c663 100644 --- a/server/master/src/main/java/org/apache/accumulo/master/tableOps/tableExport/WriteExportFiles.java +++ b/server/master/src/main/java/org/apache/accumulo/master/tableOps/tableExport/WriteExportFiles.java @@ -262,7 +262,7 @@ class WriteExportFiles extends MasterRepo { Map<String,String> siteConfig = context.instanceOperations().getSiteConfiguration(); Map<String,String> systemConfig = context.instanceOperations().getSystemConfiguration(); - TableConfiguration tableConfig = context.getServerConfFactory().getTableConfiguration(tableID); + TableConfiguration tableConfig = context.getTableConfiguration(tableID); OutputStreamWriter osw = new OutputStreamWriter(dataOut, UTF_8); diff --git a/server/master/src/main/java/org/apache/accumulo/master/upgrade/Upgrader9to10.java b/server/master/src/main/java/org/apache/accumulo/master/upgrade/Upgrader9to10.java index 064f63b..f5e7291 100644 --- a/server/master/src/main/java/org/apache/accumulo/master/upgrade/Upgrader9to10.java +++ b/server/master/src/main/java/org/apache/accumulo/master/upgrade/Upgrader9to10.java @@ -306,9 +306,8 @@ public class Upgrader9to10 implements Upgrader { long maxTime = -1; try (FileSKVIterator reader = FileOperations.getInstance().newReaderBuilder() .forFile(path.toString(), ns, ns.getConf(), context.getCryptoService()) - .withTableConfiguration( - context.getServerConfFactory().getTableConfiguration(RootTable.ID)) - .seekToBeginning().build()) { + .withTableConfiguration(context.getTableConfiguration(RootTable.ID)).seekToBeginning() + .build()) { while (reader.hasTop()) { maxTime = Math.max(maxTime, reader.getTopKey().getTimestamp()); reader.next(); diff --git a/server/tracer/src/main/java/org/apache/accumulo/tracer/TraceServer.java b/server/tracer/src/main/java/org/apache/accumulo/tracer/TraceServer.java index 09a5920..14a2575 100644 --- a/server/tracer/src/main/java/org/apache/accumulo/tracer/TraceServer.java +++ b/server/tracer/src/main/java/org/apache/accumulo/tracer/TraceServer.java @@ -58,7 +58,6 @@ import org.apache.accumulo.fate.zookeeper.ZooUtil.NodeExistsPolicy; import org.apache.accumulo.server.ServerContext; import org.apache.accumulo.server.ServerOpts; import org.apache.accumulo.server.ServerUtil; -import org.apache.accumulo.server.conf.ServerConfigurationFactory; import org.apache.accumulo.server.security.SecurityUtil; import org.apache.accumulo.server.util.time.SimpleTimer; import org.apache.accumulo.start.classloader.vfs.AccumuloVFSClassLoader; @@ -84,7 +83,6 @@ import org.slf4j.LoggerFactory; public class TraceServer implements Watcher, AutoCloseable { private static final Logger log = LoggerFactory.getLogger(TraceServer.class); - private final ServerConfigurationFactory serverConfiguration; private final ServerContext context; private final TServer server; private final AtomicReference<BatchWriter> writer; @@ -197,10 +195,9 @@ public class TraceServer implements Watcher, AutoCloseable { public TraceServer(ServerContext context, String hostname) throws Exception { this.context = context; - this.serverConfiguration = context.getServerConfFactory(); log.info("Version {}", Constants.VERSION); log.info("Instance {}", context.getInstanceID()); - AccumuloConfiguration conf = serverConfiguration.getSystemConfiguration(); + AccumuloConfiguration conf = context.getConfiguration(); tableName = conf.get(Property.TRACE_TABLE); accumuloClient = ensureTraceTableExists(conf); @@ -306,8 +303,8 @@ public class TraceServer implements Watcher, AutoCloseable { } public void run() { - SimpleTimer.getInstance(serverConfiguration.getSystemConfiguration()).schedule(() -> flush(), - SCHEDULE_DELAY, SCHEDULE_PERIOD); + SimpleTimer.getInstance(context.getConfiguration()).schedule(() -> flush(), SCHEDULE_DELAY, + SCHEDULE_PERIOD); server.serve(); } diff --git a/server/tserver/src/main/java/org/apache/accumulo/tserver/FileManager.java b/server/tserver/src/main/java/org/apache/accumulo/tserver/FileManager.java index 36404a6..0a9496d 100644 --- a/server/tserver/src/main/java/org/apache/accumulo/tserver/FileManager.java +++ b/server/tserver/src/main/java/org/apache/accumulo/tserver/FileManager.java @@ -318,8 +318,7 @@ public class FileManager { // log.debug("Opening "+file + " path " + path); FileSKVIterator reader = FileOperations.getInstance().newReaderBuilder() .forFile(path.toString(), ns, ns.getConf(), context.getCryptoService()) - .withTableConfiguration( - context.getServerConfFactory().getTableConfiguration(tablet.getTableId())) + .withTableConfiguration(context.getTableConfiguration(tablet.getTableId())) .withCacheProvider(cacheProvider).withFileLenCache(fileLenCache).build(); readersReserved.put(reader, file); } catch (Exception e) { @@ -481,7 +480,7 @@ public class FileManager { this.tablet = tablet; this.cacheProvider = cacheProvider; - continueOnFailure = context.getServerConfFactory().getTableConfiguration(tablet.getTableId()) + continueOnFailure = context.getTableConfiguration(tablet.getTableId()) .getBoolean(Property.TABLE_FAILURES_IGNORE); if (tablet.isMeta()) { diff --git a/server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServer.java b/server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServer.java index d1ef5ef..4ba635c 100644 --- a/server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServer.java +++ b/server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServer.java @@ -181,7 +181,6 @@ import org.apache.accumulo.server.ServerContext; import org.apache.accumulo.server.ServerOpts; import org.apache.accumulo.server.TabletLevel; import org.apache.accumulo.server.client.ClientServiceHandler; -import org.apache.accumulo.server.conf.ServerConfigurationFactory; import org.apache.accumulo.server.conf.TableConfiguration; import org.apache.accumulo.server.data.ServerMutation; import org.apache.accumulo.server.fs.VolumeChooserEnvironment; @@ -350,7 +349,6 @@ public class TabletServer extends AbstractServer { public static final AtomicLong seekCount = new AtomicLong(0); private final AtomicLong totalMinorCompactions = new AtomicLong(0); - private final ServerConfigurationFactory confFactory; private final ZooAuthenticationKeyWatcher authKeyWatcher; private final WalStateManager walMarker; @@ -367,7 +365,6 @@ public class TabletServer extends AbstractServer { context.setupCrypto(); this.masterLockCache = new ZooCache(context.getZooReaderWriter(), null); this.watcher = new TransactionWatcher(context); - this.confFactory = context.getServerConfFactory(); this.fs = context.getVolumeManager(); final AccumuloConfiguration aconf = getConfiguration(); log.info("Version " + Constants.VERSION); @@ -603,8 +600,7 @@ public class TabletServer extends AbstractServer { return null; } - return getContext().getServerConfFactory().getTableConfiguration(extent.getTableId()) - .getScanDispatcher(); + return getContext().getTableConfiguration(extent.getTableId()).getScanDispatcher(); } @Override @@ -1345,7 +1341,7 @@ public class TabletServer extends AbstractServer { final CompressedIterators compressedIters = new CompressedIterators(symbols); ConditionCheckerContext checkerContext = new ConditionCheckerContext(getContext(), - compressedIters, confFactory.getTableConfiguration(cs.tableId)); + compressedIters, getContext().getTableConfiguration(cs.tableId)); while (iter.hasNext()) { final Entry<KeyExtent,List<ServerConditionalMutation>> entry = iter.next(); @@ -2072,10 +2068,9 @@ public class TabletServer extends AbstractServer { SecurityErrorCode.PERMISSION_DENIED).asThriftException(); } - ServerConfigurationFactory factory = getContext().getServerConfFactory(); ExecutorService es = resourceManager.getSummaryPartitionExecutor(); Future<SummaryCollection> future = new Gatherer(getContext(), request, - factory.getTableConfiguration(tableId), getContext().getCryptoService()).gather(es); + getContext().getTableConfiguration(tableId), getContext().getCryptoService()).gather(es); return startSummaryOperation(credentials, future); } @@ -2090,11 +2085,12 @@ public class TabletServer extends AbstractServer { SecurityErrorCode.PERMISSION_DENIED).asThriftException(); } - ServerConfigurationFactory factory = getContext().getServerConfFactory(); ExecutorService spe = resourceManager.getSummaryRemoteExecutor(); - Future<SummaryCollection> future = new Gatherer(getContext(), request, - factory.getTableConfiguration(TableId.of(request.getTableId())), - getContext().getCryptoService()).processPartition(spe, modulus, remainder); + TableConfiguration tableConfig = + getContext().getTableConfiguration(TableId.of(request.getTableId())); + Future<SummaryCollection> future = + new Gatherer(getContext(), request, tableConfig, getContext().getCryptoService()) + .processPartition(spe, modulus, remainder); return startSummaryOperation(credentials, future); } @@ -2111,7 +2107,7 @@ public class TabletServer extends AbstractServer { ExecutorService srp = resourceManager.getSummaryRetrievalExecutor(); TableConfiguration tableCfg = - confFactory.getTableConfiguration(TableId.of(request.getTableId())); + getContext().getTableConfiguration(TableId.of(request.getTableId())); BlockCache summaryCache = resourceManager.getSummaryCache(); BlockCache indexCache = resourceManager.getIndexCache(); Cache<String,Long> fileLenCache = resourceManager.getFileLenCache(); @@ -3177,9 +3173,9 @@ public class TabletServer extends AbstractServer { private Durability getMincEventDurability(KeyExtent extent) { TableConfiguration conf; if (extent.isMeta()) { - conf = confFactory.getTableConfiguration(RootTable.ID); + conf = getContext().getTableConfiguration(RootTable.ID); } else { - conf = confFactory.getTableConfiguration(MetadataTable.ID); + conf = getContext().getTableConfiguration(MetadataTable.ID); } return DurabilityImpl.fromString(conf.get(Property.TABLE_DURABILITY)); } @@ -3228,7 +3224,7 @@ public class TabletServer extends AbstractServer { } public TableConfiguration getTableConfiguration(KeyExtent extent) { - return confFactory.getTableConfiguration(extent.getTableId()); + return getContext().getTableConfiguration(extent.getTableId()); } public DfsLogger.ServerResources getServerConfig() {