[accumulo] branch main updated: Hold system config in ServerConfigurationFactory (#2936)

[ct...@apache.org]

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 518fd214ca Hold system config in ServerConfigurationFactory (#2936)
518fd214ca is described below

commit 518fd214ca5a850eff621e99b1b6ffb52c8b6159
Author: Christopher Tubbs <ct...@apache.org>
AuthorDate: Sun Sep 18 20:33:40 2022 -0400

    Hold system config in ServerConfigurationFactory (#2936)
    
    * Hold instance of SystemConfiguration in ServerConfigurationFactory
      alongside other ZooBasedConfiguration instances (TableConfiguration
      and NamespaceConfiguration), and server it to ServerContext from
      there, rather than indirectly accessing it via an instance maintained
      in ServerContext
    * Also fix warning about resource leak in PerTableCryptoIT by
      surrounding the scanner in a try-with-resources block
---
 .../src/main/java/org/apache/accumulo/server/ServerContext.java    | 7 +------
 .../apache/accumulo/server/conf/ServerConfigurationFactory.java    | 7 ++++++-
 .../java/org/apache/accumulo/test/functional/PerTableCryptoIT.java | 7 ++++---
 3 files changed, 11 insertions(+), 10 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 d74d4e0a7d..c63b078a1f 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
@@ -62,10 +62,8 @@ import org.apache.accumulo.fate.zookeeper.ZooReader;
 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.SystemConfiguration;
 import org.apache.accumulo.server.conf.TableConfiguration;
 import org.apache.accumulo.server.conf.store.PropStore;
-import org.apache.accumulo.server.conf.store.SystemPropKey;
 import org.apache.accumulo.server.conf.store.impl.ZooPropStore;
 import org.apache.accumulo.server.fs.VolumeManager;
 import org.apache.accumulo.server.metadata.ServerAmpleImpl;
@@ -99,7 +97,6 @@ public class ServerContext extends ClientContext {
   private final Supplier<TableManager> tableManager;
   private final Supplier<UniqueNameAllocator> nameAllocator;
   private final Supplier<ServerConfigurationFactory> serverConfFactory;
-  private final Supplier<SystemConfiguration> systemConfig;
   private final Supplier<AuthenticationTokenSecretManager> secretManager;
   private final Supplier<ScheduledThreadPoolExecutor> sharedScheduledThreadPool;
   private final Supplier<AuditedSecurityOperation> securityOperation;
@@ -120,8 +117,6 @@ public class ServerContext extends ClientContext {
     tableManager = memoize(() -> new TableManager(this));
     nameAllocator = memoize(() -> new UniqueNameAllocator(this));
     serverConfFactory = memoize(() -> new ServerConfigurationFactory(this, getSiteConfiguration()));
-    systemConfig = memoize(() -> new SystemConfiguration(this, SystemPropKey.of(getInstanceID()),
-        getSiteConfiguration()));
     secretManager = memoize(() -> new AuthenticationTokenSecretManager(getInstanceID(),
         getConfiguration().getTimeInMillis(Property.GENERAL_DELEGATION_TOKEN_LIFETIME)));
     cryptoFactorySupplier = memoize(() -> CryptoFactoryLoader.newInstance(getConfiguration()));
@@ -160,7 +155,7 @@ public class ServerContext extends ClientContext {
 
   @Override
   public AccumuloConfiguration getConfiguration() {
-    return systemConfig.get();
+    return serverConfFactory.get().getSystemConfiguration();
   }
 
   public TableConfiguration getTableConfiguration(TableId id) {
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 f3ea0d8f0c..287e9bca44 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,6 +18,7 @@
  */
 package org.apache.accumulo.server.conf;
 
+import static com.google.common.base.Suppliers.memoize;
 import static java.util.concurrent.TimeUnit.MINUTES;
 import static java.util.concurrent.TimeUnit.NANOSECONDS;
 
@@ -26,6 +27,7 @@ import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ScheduledFuture;
 import java.util.concurrent.ScheduledThreadPoolExecutor;
 import java.util.concurrent.ThreadLocalRandom;
+import java.util.function.Supplier;
 
 import org.apache.accumulo.core.client.TableNotFoundException;
 import org.apache.accumulo.core.conf.AccumuloConfiguration;
@@ -54,6 +56,7 @@ import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 public class ServerConfigurationFactory extends ServerConfiguration {
   private final static Logger log = LoggerFactory.getLogger(ServerConfigurationFactory.class);
 
+  private final Supplier<SystemConfiguration> systemConfig;
   private final Map<TableId,NamespaceConfiguration> tableParentConfigs = new ConcurrentHashMap<>();
   private final Map<TableId,TableConfiguration> tableConfigs = new ConcurrentHashMap<>();
   private final Map<NamespaceId,NamespaceConfiguration> namespaceConfigs =
@@ -70,6 +73,8 @@ public class ServerConfigurationFactory extends ServerConfiguration {
   public ServerConfigurationFactory(ServerContext context, SiteConfiguration siteConfig) {
     this.context = context;
     this.siteConfig = siteConfig;
+    this.systemConfig = memoize(() -> new SystemConfiguration(context,
+        SystemPropKey.of(context.getInstanceID()), siteConfig));
 
     refresher = new ConfigRefreshRunner();
     Runtime.getRuntime()
@@ -90,7 +95,7 @@ public class ServerConfigurationFactory extends ServerConfiguration {
 
   @Override
   public AccumuloConfiguration getSystemConfiguration() {
-    return context.getConfiguration();
+    return systemConfig.get();
   }
 
   @Override
diff --git a/test/src/main/java/org/apache/accumulo/test/functional/PerTableCryptoIT.java b/test/src/main/java/org/apache/accumulo/test/functional/PerTableCryptoIT.java
index 4210463000..ad8a03fdd4 100644
--- a/test/src/main/java/org/apache/accumulo/test/functional/PerTableCryptoIT.java
+++ b/test/src/main/java/org/apache/accumulo/test/functional/PerTableCryptoIT.java
@@ -153,9 +153,10 @@ public class PerTableCryptoIT extends AccumuloClusterHarness {
       TableId tableId = TableId.of(c.tableOperations().tableIdMap().get(tableName));
       c.tableOperations().offline(tableName, true);
 
-      var oScanner = new OfflineScanner((ClientContext) c, tableId, Authorizations.EMPTY);
-      long count = oScanner.stream().count();
-      assertEquals(count, 100_000);
+      try (var oScanner = new OfflineScanner((ClientContext) c, tableId, Authorizations.EMPTY)) {
+        long count = oScanner.stream().count();
+        assertEquals(count, 100_000);
+      }
     }
   }