You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@accumulo.apache.org by cs...@apache.org on 2022/09/30 14:01:36 UTC

[accumulo] 01/03: Re-add getProperties() to InstanceOperations

This is an automated email from the ASF dual-hosted git repository.

cshannon pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/accumulo.git

commit eb1b875cb969a8ac20df57186d4a5189b6e520bc
Author: Christopher L. Shannon (cshannon) <ch...@gmail.com>
AuthorDate: Fri Sep 30 07:08:17 2022 -0400

    Re-add getProperties() to InstanceOperations
    
    This method was removed in #2985 because it returned a Thrift object and
    appeared to be return the same properties as getSystemConfiguration().
    This commit readds the method back (but removes the Thrift object)
    as it only returns the properties set in ZK which is different than
    getSystemConfiguration() which also returns defaults.
---
 .../accumulo/core/client/admin/InstanceOperations.java    |  9 +++++++++
 .../accumulo/core/clientImpl/InstanceOperationsImpl.java  | 15 +++++++++++++--
 .../org/apache/accumulo/test/conf/PropStoreConfigIT.java  | 15 ++++++++++++---
 3 files changed, 34 insertions(+), 5 deletions(-)

diff --git a/core/src/main/java/org/apache/accumulo/core/client/admin/InstanceOperations.java b/core/src/main/java/org/apache/accumulo/core/client/admin/InstanceOperations.java
index 00b8f14c0d..1ea5d4d1b3 100644
--- a/core/src/main/java/org/apache/accumulo/core/client/admin/InstanceOperations.java
+++ b/core/src/main/java/org/apache/accumulo/core/client/admin/InstanceOperations.java
@@ -93,6 +93,15 @@ public interface InstanceOperations {
    */
   Map<String,String> getSystemConfiguration() throws AccumuloException, AccumuloSecurityException;
 
+  /**
+   * Retrieves the configured System properties from zookeeper. This method is different from
+   * {@link #getSystemConfiguration()} as it will only return the configured properties in zookeeper
+   * and not properties from accumulo.properties or defaults.
+   *
+   * @return a map of system properties set in zookeeper
+   */
+  Map<String,String> getSystemProperties() throws AccumuloException, AccumuloSecurityException;
+
   /**
    * Retrieve the site configuration (that is set in the server configuration file).
    *
diff --git a/core/src/main/java/org/apache/accumulo/core/clientImpl/InstanceOperationsImpl.java b/core/src/main/java/org/apache/accumulo/core/clientImpl/InstanceOperationsImpl.java
index d4b0892935..d6e7c243d4 100644
--- a/core/src/main/java/org/apache/accumulo/core/clientImpl/InstanceOperationsImpl.java
+++ b/core/src/main/java/org/apache/accumulo/core/clientImpl/InstanceOperationsImpl.java
@@ -99,8 +99,7 @@ public class InstanceOperationsImpl implements InstanceOperations {
       ConcurrentModificationException {
     checkArgument(mapMutator != null, "mapMutator is null");
 
-    final TVersionedProperties vProperties = ThriftClientTypes.CLIENT.execute(context,
-        client -> client.getVersionedSystemProperties(TraceUtil.traceInfo(), context.rpcCreds()));
+    final TVersionedProperties vProperties = getVersionedSystemProperties();
     mapMutator.accept(vProperties.getProperties());
 
     for (Map.Entry<String,String> entry : vProperties.getProperties().entrySet()) {
@@ -158,6 +157,18 @@ public class InstanceOperationsImpl implements InstanceOperations {
         .getConfiguration(TraceUtil.traceInfo(), context.rpcCreds(), ConfigurationType.CURRENT));
   }
 
+  @Override
+  public Map<String,String> getSystemProperties()
+      throws AccumuloException, AccumuloSecurityException {
+    return getVersionedSystemProperties().getProperties();
+  }
+
+  private TVersionedProperties getVersionedSystemProperties()
+      throws AccumuloException, AccumuloSecurityException {
+    return ThriftClientTypes.CLIENT.execute(context,
+        client -> client.getVersionedSystemProperties(TraceUtil.traceInfo(), context.rpcCreds()));
+  }
+
   @Override
   public Map<String,String> getSiteConfiguration()
       throws AccumuloException, AccumuloSecurityException {
diff --git a/test/src/main/java/org/apache/accumulo/test/conf/PropStoreConfigIT.java b/test/src/main/java/org/apache/accumulo/test/conf/PropStoreConfigIT.java
index ce4ac9948e..26735d98c6 100644
--- a/test/src/main/java/org/apache/accumulo/test/conf/PropStoreConfigIT.java
+++ b/test/src/main/java/org/apache/accumulo/test/conf/PropStoreConfigIT.java
@@ -193,6 +193,10 @@ public class PropStoreConfigIT extends AccumuloClusterHarness {
     try (var client = Accumulo.newClient().from(getClientProps()).build()) {
       // Grab original default config
       Map<String,String> config = client.instanceOperations().getSystemConfiguration();
+      Map<String,String> properties = client.instanceOperations().getSystemProperties();
+
+      // should be empty to start
+      assertEquals(0, properties.size());
 
       final String originalClientPort = config.get(Property.TSERV_CLIENTPORT.getKey());
       final String originalMaxMem = config.get(Property.TSERV_MAXMEM.getKey());
@@ -204,9 +208,14 @@ public class PropStoreConfigIT extends AccumuloClusterHarness {
       });
 
       // Verify system properties added
-      assertTrue(Wait.waitFor(() -> client.instanceOperations().getSystemConfiguration().size() > 0,
+      assertTrue(Wait.waitFor(() -> client.instanceOperations().getSystemProperties().size() > 0,
           5000, 500));
 
+      // verify properties updated
+      properties = client.instanceOperations().getSystemProperties();
+      assertEquals("9998", properties.get(Property.TSERV_CLIENTPORT.getKey()));
+      assertEquals("35%", properties.get(Property.TSERV_MAXMEM.getKey()));
+
       // verify properties updated in config as well
       config = client.instanceOperations().getSystemConfiguration();
       assertEquals("9998", config.get(Property.TSERV_CLIENTPORT.getKey()));
@@ -216,8 +225,8 @@ public class PropStoreConfigIT extends AccumuloClusterHarness {
       // should be restored
       client.instanceOperations().modifyProperties(Map::clear);
 
-      assertTrue(Wait.waitFor(
-          () -> client.instanceOperations().getSystemConfiguration().size() == 0, 5000, 500));
+      assertTrue(Wait.waitFor(() -> client.instanceOperations().getSystemProperties().size() == 0,
+          5000, 500));
 
       // verify default system config restored
       config = client.instanceOperations().getSystemConfiguration();