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:38 UTC
[accumulo] 03/03: Revert readding getSystemProperties() and instead just fix failed test
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 257deaa9fd689f9f6464820f6469ad5fbb161453
Author: Christopher L. Shannon (cshannon) <ch...@gmail.com>
AuthorDate: Fri Sep 30 09:16:09 2022 -0400
Revert readding getSystemProperties() and instead just fix failed test
Instead of exposing properties stored in ZK just update the test to use
the Thrift api to get the properties instead
---
.../core/client/admin/InstanceOperations.java | 9 ---------
.../core/clientImpl/InstanceOperationsImpl.java | 15 ++-------------
.../apache/accumulo/test/conf/PropStoreConfigIT.java | 20 ++++++++++++++------
3 files changed, 16 insertions(+), 28 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 c8202aac61..00b8f14c0d 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,15 +93,6 @@ public interface InstanceOperations {
*/
Map<String,String> getSystemConfiguration() throws AccumuloException, AccumuloSecurityException;
- /**
- * Retrieves the stored System properties from zookeeper. This method is different from
- * {@link #getSystemConfiguration()} as it will only return the stored properties in zookeeper and
- * not properties from accumulo.properties or default values..
- *
- * @return a map of stored system properties set in zookeeper
- */
- Map<String,String> getStoredProperties() 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 522b1a5e86..d4b0892935 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,7 +99,8 @@ public class InstanceOperationsImpl implements InstanceOperations {
ConcurrentModificationException {
checkArgument(mapMutator != null, "mapMutator is null");
- final TVersionedProperties vProperties = getVersionedSystemProperties();
+ final TVersionedProperties vProperties = ThriftClientTypes.CLIENT.execute(context,
+ client -> client.getVersionedSystemProperties(TraceUtil.traceInfo(), context.rpcCreds()));
mapMutator.accept(vProperties.getProperties());
for (Map.Entry<String,String> entry : vProperties.getProperties().entrySet()) {
@@ -157,18 +158,6 @@ public class InstanceOperationsImpl implements InstanceOperations {
.getConfiguration(TraceUtil.traceInfo(), context.rpcCreds(), ConfigurationType.CURRENT));
}
- @Override
- public Map<String,String> getStoredProperties()
- 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 91cb0e3686..2921afb138 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
@@ -35,6 +35,8 @@ import org.apache.accumulo.core.client.Accumulo;
import org.apache.accumulo.core.conf.Property;
import org.apache.accumulo.core.data.NamespaceId;
import org.apache.accumulo.core.data.TableId;
+import org.apache.accumulo.core.rpc.clients.ThriftClientTypes;
+import org.apache.accumulo.core.trace.TraceUtil;
import org.apache.accumulo.fate.zookeeper.ZooReaderWriter;
import org.apache.accumulo.fate.zookeeper.ZooUtil;
import org.apache.accumulo.harness.AccumuloClusterHarness;
@@ -193,7 +195,7 @@ 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().getStoredProperties();
+ Map<String,String> properties = getStoredConfiguration();
// should be empty to start
assertEquals(0, properties.size());
@@ -208,11 +210,10 @@ public class PropStoreConfigIT extends AccumuloClusterHarness {
});
// Verify system properties added
- assertTrue(Wait.waitFor(() -> client.instanceOperations().getStoredProperties().size() > 0,
- 5000, 500));
+ assertTrue(Wait.waitFor(() -> getStoredConfiguration().size() > 0, 5000, 500));
// verify properties updated
- properties = client.instanceOperations().getStoredProperties();
+ properties = getStoredConfiguration();
assertEquals("9998", properties.get(Property.TSERV_CLIENTPORT.getKey()));
assertEquals("35%", properties.get(Property.TSERV_MAXMEM.getKey()));
@@ -225,8 +226,7 @@ public class PropStoreConfigIT extends AccumuloClusterHarness {
// should be restored
client.instanceOperations().modifyProperties(Map::clear);
- assertTrue(Wait.waitFor(() -> client.instanceOperations().getStoredProperties().size() == 0,
- 5000, 500));
+ assertTrue(Wait.waitFor(() -> getStoredConfiguration().size() == 0, 5000, 500));
// verify default system config restored
config = client.instanceOperations().getSystemConfiguration();
@@ -346,4 +346,12 @@ public class PropStoreConfigIT extends AccumuloClusterHarness {
assertEquals(originalBloomEnabled, config.get(Property.TABLE_BLOOM_ENABLED.getKey()));
assertEquals(originalBloomSize, config.get(Property.TABLE_BLOOM_SIZE.getKey()));
}
+
+ private Map<String,String> getStoredConfiguration() throws Exception {
+ ServerContext ctx = getCluster().getServerContext();
+ return ThriftClientTypes.CLIENT
+ .execute(ctx,
+ client -> client.getVersionedSystemProperties(TraceUtil.traceInfo(), ctx.rpcCreds()))
+ .getProperties();
+ }
}