You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@accumulo.apache.org by ct...@apache.org on 2022/04/28 17:47:25 UTC
[accumulo] branch main updated: Use memoized suppliers to lazy load resources (#2658)
This is an automated email from the ASF dual-hosted git repository.
ctubbsii pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/accumulo.git
The following commit(s) were added to refs/heads/main by this push:
new ba244f6515 Use memoized suppliers to lazy load resources (#2658)
ba244f6515 is described below
commit ba244f6515be6d62cfdcfae86116beeb79e19960
Author: Christopher Tubbs <ct...@apache.org>
AuthorDate: Thu Apr 28 13:47:21 2022 -0400
Use memoized suppliers to lazy load resources (#2658)
* Remove the need to synchronize on reads for lazily initialized
singleton resources, particularly in the configuration and context
utilities, using Suppliers.memoize
* Apply to DefaultConfiguration.getInstance() to avoid unnecessary
object creation whenever that is called
* Make all ServerContext fields final, and use memoize to lazily load
anything that was previously checking if it was set in a synchronized
getter method
* Use computeIfAbsent in ServerConfigurationFactory, and make its caches
of configuration objects non-static, so they only live as long as the
ServerContext that created it lives; this, along with using
ConcurrentHashMaps, dramatically simplifies this code
* Reduce ServerContext reliance on ServerConfigurationFactory when it
already has the information (notably, it has the instance of
SiteConfiguration it used to construct the ServerConfigurationFactory)
* Add a builder option for SiteConfiguration.empty() for use with tests
* Update related tests, and make ServerContextTest.testCanRun more
robust
---
.../accumulo/core/conf/DefaultConfiguration.java | 9 +-
.../accumulo/core/conf/SiteConfiguration.java | 10 +-
.../accumulo/core/conf/SiteConfigurationTest.java | 8 +-
...tRegexTableLoadBalancerReconfigurationTest.java | 2 +-
.../balancer/HostRegexTableLoadBalancerTest.java | 2 +-
.../org/apache/accumulo/server/ServerContext.java | 122 ++++++----------
.../server/conf/ServerConfigurationFactory.java | 155 ++++++---------------
.../apache/accumulo/server/ServerContextTest.java | 58 +++++---
.../conf/ServerConfigurationFactoryTest.java | 30 ++--
.../BaseHostRegexTableLoadBalancerTest.java | 2 +-
.../server/security/SystemCredentialsTest.java | 2 +-
.../coordinator/CompactionCoordinator.java | 1 -
.../org/apache/accumulo/compactor/Compactor.java | 1 -
.../accumulo/gc/SimpleGarbageCollectorTest.java | 2 +-
.../java/org/apache/accumulo/manager/Manager.java | 3 -
.../accumulo/manager/upgrade/Upgrader9to10.java | 2 -
.../org/apache/accumulo/tserver/TabletServer.java | 5 -
17 files changed, 159 insertions(+), 255 deletions(-)
diff --git a/core/src/main/java/org/apache/accumulo/core/conf/DefaultConfiguration.java b/core/src/main/java/org/apache/accumulo/core/conf/DefaultConfiguration.java
index fe08e5a3b3..7ba8026f62 100644
--- a/core/src/main/java/org/apache/accumulo/core/conf/DefaultConfiguration.java
+++ b/core/src/main/java/org/apache/accumulo/core/conf/DefaultConfiguration.java
@@ -18,9 +18,12 @@
*/
package org.apache.accumulo.core.conf;
+import static com.google.common.base.Suppliers.memoize;
+
import java.util.Arrays;
import java.util.Map;
import java.util.function.Predicate;
+import java.util.function.Supplier;
import java.util.stream.Collectors;
/**
@@ -29,7 +32,9 @@ import java.util.stream.Collectors;
*/
public class DefaultConfiguration extends AccumuloConfiguration {
- private static final Map<String,String> resolvedProps =
+ private static final Supplier<DefaultConfiguration> instance = memoize(DefaultConfiguration::new);
+
+ private final Map<String,String> resolvedProps =
Arrays.stream(Property.values()).filter(p -> p.getType() != PropertyType.PREFIX)
.collect(Collectors.toMap(Property::getKey, Property::getDefaultValue));
@@ -41,7 +46,7 @@ public class DefaultConfiguration extends AccumuloConfiguration {
* @return default configuration
*/
public static DefaultConfiguration getInstance() {
- return new DefaultConfiguration();
+ return instance.get();
}
@Override
diff --git a/core/src/main/java/org/apache/accumulo/core/conf/SiteConfiguration.java b/core/src/main/java/org/apache/accumulo/core/conf/SiteConfiguration.java
index 63c5c28334..e1c6ed3ea7 100644
--- a/core/src/main/java/org/apache/accumulo/core/conf/SiteConfiguration.java
+++ b/core/src/main/java/org/apache/accumulo/core/conf/SiteConfiguration.java
@@ -77,8 +77,7 @@ public class SiteConfiguration extends AccumuloConfiguration {
// visible to package-private for testing only
Builder() {}
- // exists for testing only
- OverridesOption noFile() {
+ private OverridesOption noFile() {
return this;
}
@@ -196,6 +195,13 @@ public class SiteConfiguration extends AccumuloConfiguration {
return new SiteConfiguration.Builder().fromFile(propertiesFileLocation);
}
+ /**
+ * Build a SiteConfiguration that is initially empty with the option to override.
+ */
+ public static SiteConfiguration.OverridesOption empty() {
+ return new SiteConfiguration.Builder().noFile();
+ }
+
/**
* Build a SiteConfiguration from the environmental configuration and no overrides.
*/
diff --git a/core/src/test/java/org/apache/accumulo/core/conf/SiteConfigurationTest.java b/core/src/test/java/org/apache/accumulo/core/conf/SiteConfigurationTest.java
index 3d30520c31..39e1cc1237 100644
--- a/core/src/test/java/org/apache/accumulo/core/conf/SiteConfigurationTest.java
+++ b/core/src/test/java/org/apache/accumulo/core/conf/SiteConfigurationTest.java
@@ -46,7 +46,7 @@ public class SiteConfigurationTest {
var overrides =
Map.of(Property.GENERAL_SECURITY_CREDENTIAL_PROVIDER_PATHS.getKey(), credProvPath);
- var config = new SiteConfiguration.Builder().noFile().withOverrides(overrides).build();
+ var config = SiteConfiguration.empty().withOverrides(overrides).build();
assertEquals("mysecret", config.get(Property.INSTANCE_SECRET));
assertNull(config.get("ignored.property"));
@@ -56,7 +56,7 @@ public class SiteConfigurationTest {
@Test
public void testDefault() {
- var conf = SiteConfiguration.auto();
+ var conf = SiteConfiguration.empty().build();
assertEquals("localhost:2181", conf.get(Property.INSTANCE_ZK_HOST));
assertEquals("DEFAULT", conf.get(Property.INSTANCE_SECRET));
assertEquals("", conf.get(Property.INSTANCE_VOLUMES));
@@ -84,10 +84,10 @@ public class SiteConfigurationTest {
@Test
public void testConfigOverrides() {
- var conf = SiteConfiguration.auto();
+ var conf = SiteConfiguration.empty().build();
assertEquals("localhost:2181", conf.get(Property.INSTANCE_ZK_HOST));
- conf = new SiteConfiguration.Builder().noFile()
+ conf = SiteConfiguration.empty()
.withOverrides(Map.of(Property.INSTANCE_ZK_HOST.getKey(), "myhost:2181")).build();
assertEquals("myhost:2181", conf.get(Property.INSTANCE_ZK_HOST));
diff --git a/core/src/test/java/org/apache/accumulo/core/spi/balancer/HostRegexTableLoadBalancerReconfigurationTest.java b/core/src/test/java/org/apache/accumulo/core/spi/balancer/HostRegexTableLoadBalancerReconfigurationTest.java
index 3878a41a3f..8b47f0e9c6 100644
--- a/core/src/test/java/org/apache/accumulo/core/spi/balancer/HostRegexTableLoadBalancerReconfigurationTest.java
+++ b/core/src/test/java/org/apache/accumulo/core/spi/balancer/HostRegexTableLoadBalancerReconfigurationTest.java
@@ -63,7 +63,7 @@ public class HostRegexTableLoadBalancerReconfigurationTest
tables.put(BAR.getTableName(), BAR.getId());
tables.put(BAZ.getTableName(), BAZ.getId());
- ConfigurationCopy config = new ConfigurationCopy(SiteConfiguration.auto());
+ ConfigurationCopy config = new ConfigurationCopy(SiteConfiguration.empty().build());
DEFAULT_TABLE_PROPERTIES.forEach(config::set);
ConfigurationImpl configImpl = new ConfigurationImpl(config);
BalancerEnvironment environment = createMock(BalancerEnvironment.class);
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 9a96af1a8e..1147c220ae 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
@@ -65,7 +65,7 @@ public class HostRegexTableLoadBalancerTest extends BaseHostRegexTableLoadBalanc
tables.put(BAR.getTableName(), BAR.getId());
tables.put(BAZ.getTableName(), BAZ.getId());
- ConfigurationCopy config = new ConfigurationCopy(SiteConfiguration.auto());
+ ConfigurationCopy config = new ConfigurationCopy(SiteConfiguration.empty().build());
tableProperties.forEach(config::set);
ConfigurationImpl configImpl = new ConfigurationImpl(config);
BalancerEnvironment environment = createMock(BalancerEnvironment.class);
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 398f73a01a..0ed173195a 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
@@ -19,6 +19,7 @@
package org.apache.accumulo.server;
import static com.google.common.base.Preconditions.checkArgument;
+import static com.google.common.base.Suppliers.memoize;
import static java.nio.charset.StandardCharsets.UTF_8;
import static java.util.concurrent.TimeUnit.MINUTES;
import static java.util.concurrent.TimeUnit.SECONDS;
@@ -37,6 +38,7 @@ import java.util.TreeMap;
import java.util.concurrent.ScheduledFuture;
import java.util.concurrent.ScheduledThreadPoolExecutor;
import java.util.concurrent.TimeUnit;
+import java.util.function.Supplier;
import org.apache.accumulo.core.Constants;
import org.apache.accumulo.core.clientImpl.ClientContext;
@@ -91,15 +93,15 @@ public class ServerContext extends ClientContext {
private final ZooReaderWriter zooReaderWriter;
private final ServerDirs serverDirs;
- private TableManager tableManager;
- private UniqueNameAllocator nameAllocator;
- private ServerConfigurationFactory serverConfFactory = null;
- private DefaultConfiguration defaultConfig = null;
- private AccumuloConfiguration systemConfig = null;
- private AuthenticationTokenSecretManager secretManager;
- private CryptoService cryptoService = null;
- private ScheduledThreadPoolExecutor sharedScheduledThreadPool = null;
- private AuditedSecurityOperation securityOperation = null;
+ // lazily loaded resources, only loaded when needed
+ private final Supplier<TableManager> tableManager;
+ private final Supplier<UniqueNameAllocator> nameAllocator;
+ private final Supplier<ServerConfigurationFactory> serverConfFactory;
+ private final Supplier<AccumuloConfiguration> systemConfig;
+ private final Supplier<AuthenticationTokenSecretManager> secretManager;
+ private final Supplier<CryptoService> cryptoService;
+ private final Supplier<ScheduledThreadPoolExecutor> sharedScheduledThreadPool;
+ private final Supplier<AuditedSecurityOperation> securityOperation;
public ServerContext(SiteConfiguration siteConfig) {
this(new ServerInfo(siteConfig));
@@ -110,6 +112,23 @@ public class ServerContext extends ClientContext {
this.info = info;
zooReaderWriter = new ZooReaderWriter(info.getSiteConfiguration());
serverDirs = info.getServerDirs();
+
+ tableManager = memoize(() -> new TableManager(this));
+ nameAllocator = memoize(() -> new UniqueNameAllocator(this));
+ serverConfFactory = memoize(() -> new ServerConfigurationFactory(this, getSiteConfiguration()));
+ // system configuration uses its own instance of ZooCache
+ // this could be useful to keep its update counter independent
+ systemConfig = memoize(() -> new ZooConfiguration(this, new ZooCache(getZooReader(), null),
+ getSiteConfiguration()));
+ secretManager = memoize(() -> new AuthenticationTokenSecretManager(getInstanceID(),
+ getConfiguration().getTimeInMillis(Property.GENERAL_DELEGATION_TOKEN_LIFETIME)));
+ cryptoService = memoize(
+ () -> CryptoServiceFactory.newInstance(getConfiguration(), ClassloaderType.ACCUMULO));
+ sharedScheduledThreadPool = memoize(() -> ThreadPools.getServerThreadPools()
+ .createGeneralScheduledExecutorService(getConfiguration()));
+ securityOperation =
+ memoize(() -> new AuditedSecurityOperation(this, SecurityOperation.getAuthorizor(this),
+ SecurityOperation.getAuthenticator(this), SecurityOperation.getPermHandler(this)));
}
/**
@@ -134,54 +153,25 @@ public class ServerContext extends ClientContext {
return info.getInstanceID();
}
- /**
- * Should only be called by the Tablet server
- */
- public synchronized void setupCrypto() throws CryptoService.CryptoException {
- if (cryptoService != null) {
- throw new CryptoService.CryptoException("Crypto Service " + cryptoService.getClass().getName()
- + " already exists and cannot be setup again");
- }
-
- AccumuloConfiguration acuConf = getConfiguration();
- 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());
- }
- return serverConfFactory;
- }
-
@Override
public AccumuloConfiguration getConfiguration() {
- if (systemConfig == null) {
- // system configuration uses its own instance of ZooCache
- // this could be useful to keep its update counter independent
- ZooCache propCache = new ZooCache(getZooReader(), null);
- systemConfig = new ZooConfiguration(this, propCache, getSiteConfiguration());
- }
- return systemConfig;
+ return systemConfig.get();
}
public TableConfiguration getTableConfiguration(TableId id) {
- return getServerConfFactory().getTableConfiguration(id);
+ return serverConfFactory.get().getTableConfiguration(id);
}
public NamespaceConfiguration getNamespaceConfiguration(NamespaceId namespaceId) {
- return getServerConfFactory().getNamespaceConfiguration(namespaceId);
+ return serverConfFactory.get().getNamespaceConfiguration(namespaceId);
}
public DefaultConfiguration getDefaultConfiguration() {
- if (defaultConfig == null) {
- defaultConfig = DefaultConfiguration.getInstance();
- }
- return defaultConfig;
+ return DefaultConfiguration.getInstance();
}
public ServerDirs getServerDirs() {
@@ -194,7 +184,7 @@ public class ServerContext extends ClientContext {
*/
// Should be private, but package-protected so EasyMock will work
void enforceKerberosLogin() {
- final AccumuloConfiguration conf = getServerConfFactory().getSiteConfiguration();
+ final AccumuloConfiguration conf = getSiteConfiguration();
// Unwrap _HOST into the FQDN to make the kerberos principal we'll compare against
final String kerberosPrincipal =
SecurityUtil.getServerPrincipal(conf.get(Property.GENERAL_KERBEROS_PRINCIPAL));
@@ -234,11 +224,11 @@ public class ServerContext extends ClientContext {
@Override
public SaslServerConnectionParams getSaslParams() {
- AccumuloConfiguration conf = getServerConfFactory().getSiteConfiguration();
+ AccumuloConfiguration conf = getSiteConfiguration();
if (!conf.getBoolean(Property.INSTANCE_RPC_SASL_ENABLED)) {
return null;
}
- return new SaslServerConnectionParams(conf, getCredentials().getToken(), secretManager);
+ return new SaslServerConnectionParams(conf, getCredentials().getToken(), getSecretManager());
}
/**
@@ -269,33 +259,20 @@ public class ServerContext extends ClientContext {
}
}
- public void setSecretManager(AuthenticationTokenSecretManager secretManager) {
- this.secretManager = secretManager;
- }
-
public AuthenticationTokenSecretManager getSecretManager() {
- return secretManager;
+ return secretManager.get();
}
- public synchronized TableManager getTableManager() {
- if (tableManager == null) {
- tableManager = new TableManager(this);
- }
- return tableManager;
+ public TableManager getTableManager() {
+ return tableManager.get();
}
- public synchronized UniqueNameAllocator getUniqueNameAllocator() {
- if (nameAllocator == null) {
- nameAllocator = new UniqueNameAllocator(this);
- }
- return nameAllocator;
+ public UniqueNameAllocator getUniqueNameAllocator() {
+ return nameAllocator.get();
}
public CryptoService getCryptoService() {
- if (cryptoService == null) {
- throw new CryptoService.CryptoException("Crypto service not initialized.");
- }
- return cryptoService;
+ return cryptoService.get();
}
@Override
@@ -453,15 +430,8 @@ public class ServerContext extends ClientContext {
ThreadPools.watchNonCriticalScheduledTask(future);
}
- /**
- * return a shared scheduled executor
- */
- public synchronized ScheduledThreadPoolExecutor getScheduledExecutor() {
- if (sharedScheduledThreadPool == null) {
- sharedScheduledThreadPool = ThreadPools.getServerThreadPools()
- .createGeneralScheduledExecutorService(getConfiguration());
- }
- return sharedScheduledThreadPool;
+ public ScheduledThreadPoolExecutor getScheduledExecutor() {
+ return sharedScheduledThreadPool.get();
}
@Override
@@ -470,11 +440,7 @@ public class ServerContext extends ClientContext {
}
public AuditedSecurityOperation getSecurityOperation() {
- if (securityOperation == null) {
- securityOperation = new AuditedSecurityOperation(this, SecurityOperation.getAuthorizor(this),
- SecurityOperation.getAuthenticator(this), SecurityOperation.getPermHandler(this));
- }
- return securityOperation;
+ return securityOperation.get();
}
}
diff --git a/server/base/src/main/java/org/apache/accumulo/server/conf/ServerConfigurationFactory.java b/server/base/src/main/java/org/apache/accumulo/server/conf/ServerConfigurationFactory.java
index 7b32e747b7..d968af064e 100644
--- a/server/base/src/main/java/org/apache/accumulo/server/conf/ServerConfigurationFactory.java
+++ b/server/base/src/main/java/org/apache/accumulo/server/conf/ServerConfigurationFactory.java
@@ -18,67 +18,50 @@
*/
package org.apache.accumulo.server.conf;
-import java.util.HashMap;
import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.function.Supplier;
import org.apache.accumulo.core.client.TableNotFoundException;
import org.apache.accumulo.core.conf.AccumuloConfiguration;
import org.apache.accumulo.core.conf.ConfigCheckUtil;
import org.apache.accumulo.core.conf.DefaultConfiguration;
import org.apache.accumulo.core.conf.SiteConfiguration;
-import org.apache.accumulo.core.data.InstanceId;
import org.apache.accumulo.core.data.NamespaceId;
import org.apache.accumulo.core.data.TableId;
import org.apache.accumulo.fate.zookeeper.ZooCache;
import org.apache.accumulo.fate.zookeeper.ZooCacheFactory;
import org.apache.accumulo.server.ServerContext;
+import com.google.common.base.Suppliers;
+
/**
* A factor for configurations used by a server process. Instance of this class are thread-safe.
*/
public class ServerConfigurationFactory extends ServerConfiguration {
- private static final Map<InstanceId,Map<TableId,TableConfiguration>> tableConfigs =
- new HashMap<>(1);
- private static final Map<InstanceId,Map<NamespaceId,NamespaceConfiguration>> namespaceConfigs =
- new HashMap<>(1);
- private static final Map<InstanceId,Map<TableId,NamespaceConfiguration>> tableParentConfigs =
- new HashMap<>(1);
+ private final Map<TableId,NamespaceConfiguration> tableParentConfigs = new ConcurrentHashMap<>();
+ private final Map<TableId,TableConfiguration> tableConfigs = new ConcurrentHashMap<>();
+ private final Map<NamespaceId,NamespaceConfiguration> namespaceConfigs =
+ new ConcurrentHashMap<>();
- private static void addInstanceToCaches(InstanceId iid) {
- synchronized (tableConfigs) {
- tableConfigs.computeIfAbsent(iid, k -> new HashMap<>());
- }
- synchronized (namespaceConfigs) {
- namespaceConfigs.computeIfAbsent(iid, k -> new HashMap<>());
- }
- synchronized (tableParentConfigs) {
- tableParentConfigs.computeIfAbsent(iid, k -> new HashMap<>());
- }
- }
-
- static void clearCachedConfigurations() {
- synchronized (tableConfigs) {
- tableConfigs.clear();
- }
- synchronized (namespaceConfigs) {
- namespaceConfigs.clear();
- }
- synchronized (tableParentConfigs) {
- tableParentConfigs.clear();
- }
- }
+ private final Supplier<ZooConfiguration> systemConfig;
private final ServerContext context;
private final SiteConfiguration siteConfig;
- private final InstanceId instanceID;
private ZooCacheFactory zcf = new ZooCacheFactory();
public ServerConfigurationFactory(ServerContext context, SiteConfiguration siteConfig) {
this.context = context;
this.siteConfig = siteConfig;
- instanceID = context.getInstanceID();
- addInstanceToCaches(instanceID);
+ systemConfig = Suppliers.memoize(() -> {
+ // Force the creation of a new ZooCache instead of using a shared one.
+ // This is done so that the ZooCache will update less often, causing the
+ // configuration update count to increment more slowly.
+ ZooCache propCache =
+ zcf.getNewZooCache(context.getZooKeepers(), context.getZooKeepersSessionTimeOut());
+ return new ZooConfiguration(context, propCache, siteConfig);
+ });
}
public ServerContext getServerContext() {
@@ -89,109 +72,51 @@ public class ServerConfigurationFactory extends ServerConfiguration {
this.zcf = zcf;
}
- private DefaultConfiguration defaultConfig = null;
- private AccumuloConfiguration systemConfig = null;
-
public SiteConfiguration getSiteConfiguration() {
return siteConfig;
}
- public synchronized DefaultConfiguration getDefaultConfiguration() {
- if (defaultConfig == null) {
- defaultConfig = DefaultConfiguration.getInstance();
- }
- return defaultConfig;
+ public DefaultConfiguration getDefaultConfiguration() {
+ return DefaultConfiguration.getInstance();
}
@Override
- public synchronized AccumuloConfiguration getSystemConfiguration() {
- if (systemConfig == null) {
- // Force the creation of a new ZooCache instead of using a shared one.
- // This is done so that the ZooCache will update less often, causing the
- // configuration update count to increment more slowly.
- ZooCache propCache =
- zcf.getNewZooCache(context.getZooKeepers(), context.getZooKeepersSessionTimeOut());
- systemConfig = new ZooConfiguration(context, propCache, getSiteConfiguration());
- }
- return systemConfig;
+ public AccumuloConfiguration getSystemConfiguration() {
+ return systemConfig.get();
}
@Override
public TableConfiguration getTableConfiguration(TableId tableId) {
- TableConfiguration conf;
- synchronized (tableConfigs) {
- conf = tableConfigs.get(instanceID).get(tableId);
- }
-
- // Can't hold the lock during the construction and validation of the config,
- // which would result in creating multiple objects for the same id.
- //
- // ACCUMULO-3859 We _cannot_ allow multiple instances to be created for a table. If the
- // TableConfiguration
- // instance a Tablet holds is not the same as the one cached here, any ConfigurationObservers
- // that
- // Tablet sets will never see updates from ZooKeeper which means that things like constraints
- // and
- // default visibility labels will never be updated in a Tablet until it is reloaded.
- if (conf == null && context.tableNodeExists(tableId)) {
- conf = new TableConfiguration(context, tableId, getNamespaceConfigurationForTable(tableId));
- ConfigCheckUtil.validate(conf);
- synchronized (tableConfigs) {
- Map<TableId,TableConfiguration> configs = tableConfigs.get(instanceID);
- TableConfiguration existingConf = configs.get(tableId);
- if (existingConf == null) {
- // Configuration doesn't exist yet
- configs.put(tableId, conf);
- } else {
- // Someone beat us to the punch, reuse their instance instead of replacing it
- conf = existingConf;
- }
+ return tableConfigs.computeIfAbsent(tableId, key -> {
+ if (context.tableNodeExists(tableId)) {
+ var conf =
+ new TableConfiguration(context, tableId, getNamespaceConfigurationForTable(tableId));
+ ConfigCheckUtil.validate(conf);
+ return conf;
}
- }
- return conf;
+ return null;
+ });
}
public NamespaceConfiguration getNamespaceConfigurationForTable(TableId tableId) {
- NamespaceConfiguration conf;
- synchronized (tableParentConfigs) {
- conf = tableParentConfigs.get(instanceID).get(tableId);
- }
- // can't hold the lock during the construction and validation of the config,
- // which may result in creating multiple objects for the same id, but that's ok.
- if (conf == null) {
- NamespaceId namespaceId;
- try {
- namespaceId = context.getNamespaceId(tableId);
- } catch (TableNotFoundException e) {
- throw new RuntimeException(e);
- }
- conf = new NamespaceConfiguration(namespaceId, context, getSystemConfiguration());
- ConfigCheckUtil.validate(conf);
- synchronized (tableParentConfigs) {
- tableParentConfigs.get(instanceID).put(tableId, conf);
- }
+ NamespaceId namespaceId;
+ try {
+ namespaceId = context.getNamespaceId(tableId);
+ } catch (TableNotFoundException e) {
+ throw new RuntimeException(e);
}
- return conf;
+ return tableParentConfigs.computeIfAbsent(tableId,
+ key -> getNamespaceConfiguration(namespaceId));
}
@Override
public NamespaceConfiguration getNamespaceConfiguration(NamespaceId namespaceId) {
- NamespaceConfiguration conf;
- // can't hold the lock during the construction and validation of the config,
- // which may result in creating multiple objects for the same id, but that's ok.
- synchronized (namespaceConfigs) {
- conf = namespaceConfigs.get(instanceID).get(namespaceId);
- }
- if (conf == null) {
- // changed - include instance in constructor call
- conf = new NamespaceConfiguration(namespaceId, context, getSystemConfiguration());
+ return namespaceConfigs.computeIfAbsent(namespaceId, key -> {
+ var conf = new NamespaceConfiguration(namespaceId, context, getSystemConfiguration());
conf.setZooCacheFactory(zcf);
ConfigCheckUtil.validate(conf);
- synchronized (namespaceConfigs) {
- namespaceConfigs.get(instanceID).put(namespaceId, conf);
- }
- }
- return conf;
+ return conf;
+ });
}
}
diff --git a/server/base/src/test/java/org/apache/accumulo/server/ServerContextTest.java b/server/base/src/test/java/org/apache/accumulo/server/ServerContextTest.java
index ab1912952a..e216ddae35 100644
--- a/server/base/src/test/java/org/apache/accumulo/server/ServerContextTest.java
+++ b/server/base/src/test/java/org/apache/accumulo/server/ServerContextTest.java
@@ -18,6 +18,14 @@
*/
package org.apache.accumulo.server;
+import static org.easymock.EasyMock.anyObject;
+import static org.easymock.EasyMock.createMock;
+import static org.easymock.EasyMock.createMockBuilder;
+import static org.easymock.EasyMock.expect;
+import static org.easymock.EasyMock.expectLastCall;
+import static org.easymock.EasyMock.getCurrentArguments;
+import static org.easymock.EasyMock.replay;
+import static org.easymock.EasyMock.verify;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertThrows;
@@ -27,6 +35,8 @@ import java.io.DataInputStream;
import java.io.DataOutputStream;
import java.security.PrivilegedExceptionAction;
import java.util.Properties;
+import java.util.function.IntConsumer;
+import java.util.stream.IntStream;
import org.apache.accumulo.core.client.security.tokens.PasswordToken;
import org.apache.accumulo.core.clientImpl.ClientConfConverter;
@@ -42,7 +52,6 @@ import org.apache.accumulo.server.security.SystemCredentials.SystemToken;
import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.fs.CommonConfigurationKeys;
import org.apache.hadoop.security.UserGroupInformation;
-import org.easymock.EasyMock;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
@@ -71,9 +80,9 @@ public class ServerContextTest {
clientProps.setProperty(ClientProperty.SASL_ENABLED.getKey(), "true");
clientProps.setProperty(ClientProperty.SASL_KERBEROS_SERVER_PRIMARY.getKey(), "accumulo");
final AccumuloConfiguration conf = ClientConfConverter.toAccumuloConf(clientProps);
- SiteConfiguration siteConfig = EasyMock.createMock(SiteConfiguration.class);
+ SiteConfiguration siteConfig = createMock(SiteConfiguration.class);
- EasyMock.expect(siteConfig.getBoolean(Property.INSTANCE_RPC_SASL_ENABLED)).andReturn(true);
+ expect(siteConfig.getBoolean(Property.INSTANCE_RPC_SASL_ENABLED)).andReturn(true);
// Deal with SystemToken being private
PasswordToken pw = new PasswordToken("fake");
@@ -82,38 +91,39 @@ public class ServerContextTest {
SystemToken token = new SystemToken();
token.readFields(new DataInputStream(new ByteArrayInputStream(baos.toByteArray())));
- ServerConfigurationFactory factory = EasyMock.createMock(ServerConfigurationFactory.class);
- EasyMock.expect(factory.getSystemConfiguration()).andReturn(conf).anyTimes();
- EasyMock.expect(factory.getSiteConfiguration()).andReturn(siteConfig).anyTimes();
+ ServerConfigurationFactory factory = createMock(ServerConfigurationFactory.class);
+ expect(factory.getSystemConfiguration()).andReturn(conf).anyTimes();
- ServerContext context = EasyMock.createMockBuilder(ServerContext.class)
- .addMockedMethod("enforceKerberosLogin").addMockedMethod("getConfiguration")
- .addMockedMethod("getServerConfFactory").addMockedMethod("getCredentials").createMock();
+ ServerContext context =
+ createMockBuilder(ServerContext.class).addMockedMethod("enforceKerberosLogin")
+ .addMockedMethod("getConfiguration").addMockedMethod("getSiteConfiguration")
+ .addMockedMethod("getSecretManager").addMockedMethod("getCredentials").createMock();
context.enforceKerberosLogin();
- EasyMock.expectLastCall().anyTimes();
- EasyMock.expect(context.getConfiguration()).andReturn(conf).anyTimes();
- EasyMock.expect(context.getServerConfFactory()).andReturn(factory).anyTimes();
- EasyMock.expect(context.getCredentials())
+ expectLastCall().anyTimes();
+ expect(context.getSiteConfiguration()).andReturn(siteConfig).anyTimes();
+ expect(context.getSecretManager()).andReturn(null).anyTimes();
+ expect(context.getConfiguration()).andReturn(conf).anyTimes();
+ expect(context.getCredentials())
.andReturn(new Credentials("accumulo/hostname@FAKE.COM", token)).once();
// Just make the SiteConfiguration delegate to our ClientConfiguration (by way of the
// AccumuloConfiguration)
// Presently, we only need get(Property) and iterator().
- EasyMock.expect(siteConfig.get(EasyMock.anyObject(Property.class))).andAnswer(() -> {
- Object[] args = EasyMock.getCurrentArguments();
+ expect(siteConfig.get(anyObject(Property.class))).andAnswer(() -> {
+ Object[] args = getCurrentArguments();
return conf.get((Property) args[0]);
}).anyTimes();
- EasyMock.expect(siteConfig.iterator()).andAnswer(conf::iterator).anyTimes();
+ expect(siteConfig.iterator()).andAnswer(conf::iterator).anyTimes();
- EasyMock.replay(factory, context, siteConfig);
+ replay(factory, context, siteConfig);
assertEquals(ThriftServerType.SASL, context.getThriftServerType());
SaslServerConnectionParams saslParams = context.getSaslParams();
assertEquals(new SaslServerConnectionParams(conf, token), saslParams);
assertEquals(username, saslParams.getPrincipal());
- EasyMock.verify(factory, context, siteConfig);
+ verify(factory, context, siteConfig);
return null;
});
@@ -121,8 +131,16 @@ public class ServerContextTest {
@Test
public void testCanRun() {
- // ensure this fails with older versions
- assertThrows(IllegalStateException.class, () -> ServerContext.ensureDataVersionCompatible(7));
+ // ensure this fails with older versions; the oldest supported version is hard-coded here
+ // to ensure we don't unintentionally break upgrade support; changing this should be a conscious
+ // decision and this check will ensure we don't overlook it
+ final int oldestSupported = 8;
+ final int currentVersion = AccumuloDataVersion.get();
+ IntConsumer shouldPass = v -> ServerContext.ensureDataVersionCompatible(v);
+ IntConsumer shouldFail = v -> assertThrows(IllegalStateException.class,
+ () -> ServerContext.ensureDataVersionCompatible(v));
+ IntStream.rangeClosed(oldestSupported, currentVersion).forEach(shouldPass);
+ IntStream.of(oldestSupported - 1, currentVersion + 1).forEach(shouldFail);
}
}
diff --git a/server/base/src/test/java/org/apache/accumulo/server/conf/ServerConfigurationFactoryTest.java b/server/base/src/test/java/org/apache/accumulo/server/conf/ServerConfigurationFactoryTest.java
index 1657734d10..1a7ad40410 100644
--- a/server/base/src/test/java/org/apache/accumulo/server/conf/ServerConfigurationFactoryTest.java
+++ b/server/base/src/test/java/org/apache/accumulo/server/conf/ServerConfigurationFactoryTest.java
@@ -18,13 +18,12 @@
*/
package org.apache.accumulo.server.conf;
-import static java.nio.charset.StandardCharsets.UTF_8;
import static org.easymock.EasyMock.anyObject;
import static org.easymock.EasyMock.createMock;
-import static org.easymock.EasyMock.endsWith;
import static org.easymock.EasyMock.eq;
import static org.easymock.EasyMock.expect;
import static org.easymock.EasyMock.replay;
+import static org.easymock.EasyMock.verify;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertSame;
@@ -38,6 +37,7 @@ import org.apache.accumulo.fate.zookeeper.ZooCache;
import org.apache.accumulo.fate.zookeeper.ZooCacheFactory;
import org.apache.accumulo.server.MockServerContext;
import org.apache.accumulo.server.ServerContext;
+import org.junit.jupiter.api.AfterAll;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.BeforeEach;
@@ -51,7 +51,7 @@ public class ServerConfigurationFactoryTest {
// use the same mock ZooCacheFactory and ZooCache for all tests
private static ZooCacheFactory zcf;
private static ZooCache zc;
- private static SiteConfiguration siteConfig = SiteConfiguration.auto();
+ private static SiteConfiguration siteConfig = SiteConfiguration.empty().build();
@BeforeAll
public static void setUpClass() {
@@ -62,47 +62,44 @@ public class ServerConfigurationFactoryTest {
replay(zcf);
expect(zc.getChildren(anyObject(String.class))).andReturn(null).anyTimes();
- // CheckServerConfig looks at timeout
- expect(zc.get(endsWith("timeout"))).andReturn(("" + ZK_TIMEOUT + "ms").getBytes(UTF_8));
replay(zc);
}
+ @AfterAll
+ public static void verifyClassMocks() {
+ verify(zcf, zc);
+ }
+
private ServerContext context;
private ServerConfigurationFactory scf;
@BeforeEach
public void setUp() {
context = MockServerContext.getWithZK(IID, ZK_HOST, ZK_TIMEOUT);
- }
-
- @AfterEach
- public void tearDown() {
- ServerConfigurationFactory.clearCachedConfigurations();
- }
-
- private void ready() {
replay(context);
scf = new ServerConfigurationFactory(context, siteConfig);
scf.setZooCacheFactory(zcf);
}
+ @AfterEach
+ public void verifyMocks() {
+ verify(context);
+ }
+
@Test
public void testGetDefaultConfiguration() {
- ready();
DefaultConfiguration c = scf.getDefaultConfiguration();
assertNotNull(c);
}
@Test
public void testGetSiteConfiguration() {
- ready();
var c = scf.getSiteConfiguration();
assertNotNull(c);
}
@Test
public void testGetConfiguration() {
- ready();
AccumuloConfiguration c = scf.getSystemConfiguration();
assertNotNull(c);
}
@@ -111,7 +108,6 @@ public class ServerConfigurationFactoryTest {
@Test
public void testGetNamespaceConfiguration() {
- ready();
NamespaceConfiguration c = scf.getNamespaceConfiguration(NSID);
assertEquals(NSID, c.getNamespaceId());
assertSame(c, scf.getNamespaceConfiguration(NSID));
diff --git a/server/base/src/test/java/org/apache/accumulo/server/master/balancer/BaseHostRegexTableLoadBalancerTest.java b/server/base/src/test/java/org/apache/accumulo/server/master/balancer/BaseHostRegexTableLoadBalancerTest.java
index 01ca53703e..cf767c54c5 100644
--- a/server/base/src/test/java/org/apache/accumulo/server/master/balancer/BaseHostRegexTableLoadBalancerTest.java
+++ b/server/base/src/test/java/org/apache/accumulo/server/master/balancer/BaseHostRegexTableLoadBalancerTest.java
@@ -92,7 +92,7 @@ public abstract class BaseHostRegexTableLoadBalancerTest extends HostRegexTableL
TestDefaultBalancer.class.getName());
}
- private static SiteConfiguration siteConfg = SiteConfiguration.auto();
+ private static SiteConfiguration siteConfg = SiteConfiguration.empty().build();
protected static class TestServerConfigurationFactory extends ServerConfigurationFactory {
diff --git a/server/base/src/test/java/org/apache/accumulo/server/security/SystemCredentialsTest.java b/server/base/src/test/java/org/apache/accumulo/server/security/SystemCredentialsTest.java
index 7a3ecc19ad..3b8f0b1a86 100644
--- a/server/base/src/test/java/org/apache/accumulo/server/security/SystemCredentialsTest.java
+++ b/server/base/src/test/java/org/apache/accumulo/server/security/SystemCredentialsTest.java
@@ -39,7 +39,7 @@ import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
public class SystemCredentialsTest {
- private static SiteConfiguration siteConfig = SiteConfiguration.auto();
+ private static SiteConfiguration siteConfig = SiteConfiguration.empty().build();
private InstanceId instanceId =
InstanceId.of(UUID.nameUUIDFromBytes(new byte[] {1, 2, 3, 4, 5, 6, 7, 8, 9, 0}));
diff --git a/server/compaction-coordinator/src/main/java/org/apache/accumulo/coordinator/CompactionCoordinator.java b/server/compaction-coordinator/src/main/java/org/apache/accumulo/coordinator/CompactionCoordinator.java
index 1dc1a01f85..87766f3bb1 100644
--- a/server/compaction-coordinator/src/main/java/org/apache/accumulo/coordinator/CompactionCoordinator.java
+++ b/server/compaction-coordinator/src/main/java/org/apache/accumulo/coordinator/CompactionCoordinator.java
@@ -157,7 +157,6 @@ public class CompactionCoordinator extends AbstractServer
}
protected void setupSecurity() {
- getContext().setupCrypto();
security = getContext().getSecurityOperation();
}
diff --git a/server/compactor/src/main/java/org/apache/accumulo/compactor/Compactor.java b/server/compactor/src/main/java/org/apache/accumulo/compactor/Compactor.java
index 4646f1613d..4d7da8b79f 100644
--- a/server/compactor/src/main/java/org/apache/accumulo/compactor/Compactor.java
+++ b/server/compactor/src/main/java/org/apache/accumulo/compactor/Compactor.java
@@ -191,7 +191,6 @@ public class Compactor extends AbstractServer implements MetricsProducer, Compac
}
protected void setupSecurity() {
- getContext().setupCrypto();
security = getContext().getSecurityOperation();
}
diff --git a/server/gc/src/test/java/org/apache/accumulo/gc/SimpleGarbageCollectorTest.java b/server/gc/src/test/java/org/apache/accumulo/gc/SimpleGarbageCollectorTest.java
index 9c7541f639..337a816ee0 100644
--- a/server/gc/src/test/java/org/apache/accumulo/gc/SimpleGarbageCollectorTest.java
+++ b/server/gc/src/test/java/org/apache/accumulo/gc/SimpleGarbageCollectorTest.java
@@ -60,7 +60,7 @@ public class SimpleGarbageCollectorTest {
private Credentials credentials;
private SimpleGarbageCollector gc;
private ConfigurationCopy systemConfig;
- private static SiteConfiguration siteConfig = SiteConfiguration.auto();
+ private static SiteConfiguration siteConfig = SiteConfiguration.empty().build();
@BeforeEach
public void setUp() {
diff --git a/server/manager/src/main/java/org/apache/accumulo/manager/Manager.java b/server/manager/src/main/java/org/apache/accumulo/manager/Manager.java
index da26715ff9..6bb41f0be4 100644
--- a/server/manager/src/main/java/org/apache/accumulo/manager/Manager.java
+++ b/server/manager/src/main/java/org/apache/accumulo/manager/Manager.java
@@ -130,7 +130,6 @@ import org.apache.accumulo.server.rpc.TServerUtils;
import org.apache.accumulo.server.rpc.ThriftProcessorTypes;
import org.apache.accumulo.server.security.SecurityOperation;
import org.apache.accumulo.server.security.delegation.AuthenticationTokenKeyManager;
-import org.apache.accumulo.server.security.delegation.AuthenticationTokenSecretManager;
import org.apache.accumulo.server.security.delegation.ZooAuthenticationKeyDistributor;
import org.apache.accumulo.server.tables.TableManager;
import org.apache.accumulo.server.tables.TableObserver;
@@ -386,9 +385,7 @@ public class Manager extends AbstractServer
this.security = context.getSecurityOperation();
- // Create the secret manager (can generate and verify delegation tokens)
final long tokenLifetime = aconf.getTimeInMillis(Property.GENERAL_DELEGATION_TOKEN_LIFETIME);
- context.setSecretManager(new AuthenticationTokenSecretManager(getInstanceID(), tokenLifetime));
authenticationTokenKeyManager = null;
keyDistributor = null;
diff --git a/server/manager/src/main/java/org/apache/accumulo/manager/upgrade/Upgrader9to10.java b/server/manager/src/main/java/org/apache/accumulo/manager/upgrade/Upgrader9to10.java
index 6cfed37ec3..20136e7fad 100644
--- a/server/manager/src/main/java/org/apache/accumulo/manager/upgrade/Upgrader9to10.java
+++ b/server/manager/src/main/java/org/apache/accumulo/manager/upgrade/Upgrader9to10.java
@@ -376,8 +376,6 @@ public class Upgrader9to10 implements Upgrader {
MetadataTime computeRootTabletTime(ServerContext context, Collection<String> goodPaths) {
try {
- context.setupCrypto();
-
long rtime = Long.MIN_VALUE;
for (String good : goodPaths) {
Path path = new Path(good);
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 9820f3e165..910c4e9303 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
@@ -124,7 +124,6 @@ import org.apache.accumulo.server.rpc.TServerUtils;
import org.apache.accumulo.server.rpc.ThriftProcessorTypes;
import org.apache.accumulo.server.security.SecurityOperation;
import org.apache.accumulo.server.security.SecurityUtil;
-import org.apache.accumulo.server.security.delegation.AuthenticationTokenSecretManager;
import org.apache.accumulo.server.security.delegation.ZooAuthenticationKeyWatcher;
import org.apache.accumulo.server.util.FileSystemMonitor;
import org.apache.accumulo.server.util.ServerBulkImportStatus;
@@ -247,7 +246,6 @@ public class TabletServer extends AbstractServer {
protected TabletServer(ServerOpts opts, String[] args) {
super("tserver", opts, args);
context = super.getContext();
- context.setupCrypto();
this.managerLockCache = new ZooCache(context.getZooReader(), null);
final AccumuloConfiguration aconf = getConfiguration();
log.info("Version " + Constants.VERSION);
@@ -363,9 +361,6 @@ public class TabletServer extends AbstractServer {
TabletLocator::clearLocators, jitter(), jitter(), TimeUnit.MILLISECONDS));
walMarker = new WalStateManager(context);
- // Create the secret manager
- context.setSecretManager(new AuthenticationTokenSecretManager(context.getInstanceID(),
- aconf.getTimeInMillis(Property.GENERAL_DELEGATION_TOKEN_LIFETIME)));
if (aconf.getBoolean(Property.INSTANCE_RPC_SASL_ENABLED)) {
log.info("SASL is enabled, creating ZooKeeper watcher for AuthenticationKeys");
// Watcher to notice new AuthenticationKeys which enable delegation tokens