This is an automated email from the ASF dual-hosted git repository.
jmark99 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 6f5c2f07a4 Implement consistent Namespace and Table property handling
(#2771)
6f5c2f07a4 is described below
commit 6f5c2f07a44bbbda1c33fb094654fc5644dab461
Author: Mark Owens <jmar...@apache.org>
AuthorDate: Mon Jun 13 12:12:02 2022 -0400
Implement consistent Namespace and Table property handling (#2771)
TablePropUtil and NamespacePropUtil handle invalid properties differently.
This update updates the two classes to handle properties in a similar manner.
They both will now throw an exception on an invalid property value.
This is the first step in what may be a multi-step process. With this
change, both TablePropUtil and NamespacePropUtil are very similar and could
possibly be combined into one method. I will create a follow-on ticket to look
into that possibility. But I wanted to get the initial task of making them
consistent completed first.
Closes #2633
---
.../accumulo/server/util/NamespacePropUtil.java | 12 ++-
.../apache/accumulo/server/util/TablePropUtil.java | 18 ++--
.../accumulo/test/ZooKeeperPropertiesIT.java | 116 +++++++++++++++++++++
3 files changed, 136 insertions(+), 10 deletions(-)
diff --git
a/server/base/src/main/java/org/apache/accumulo/server/util/NamespacePropUtil.java
b/server/base/src/main/java/org/apache/accumulo/server/util/NamespacePropUtil.java
index f141513126..da1f13f28c 100644
---
a/server/base/src/main/java/org/apache/accumulo/server/util/NamespacePropUtil.java
+++
b/server/base/src/main/java/org/apache/accumulo/server/util/NamespacePropUtil.java
@@ -32,12 +32,20 @@ public class NamespacePropUtil extends
PropUtil<NamespaceId> {
super(context);
}
+ /**
+ * Helper method to set provided properties for the provided namespace.
+ *
+ * @throws IllegalStateException
+ * if an underlying exception (KeeperException,
InterruptException) or other failure to
+ * read properties from the cache / backend store
+ * @throws IllegalArgumentException
+ * if a provided property is not valid
+ */
@Override
public void setProperties(NamespaceId namespaceId, Map<String,String>
properties) {
for (Map.Entry<String,String> prop : properties.entrySet()) {
- // TODO reconcile with TablePropUtil see
https://github.com/apache/accumulo/issues/2633
if (!Property.isTablePropertyValid(prop.getKey(), prop.getValue())) {
- throw new IllegalArgumentException("Invalid table property for
namespace: " + namespaceId
+ throw new IllegalArgumentException("Invalid property for namespace: "
+ namespaceId
+ " name: " + prop.getKey() + ", value: " + prop.getValue());
}
}
diff --git
a/server/base/src/main/java/org/apache/accumulo/server/util/TablePropUtil.java
b/server/base/src/main/java/org/apache/accumulo/server/util/TablePropUtil.java
index b3fb54be3b..642af9af00 100644
---
a/server/base/src/main/java/org/apache/accumulo/server/util/TablePropUtil.java
+++
b/server/base/src/main/java/org/apache/accumulo/server/util/TablePropUtil.java
@@ -19,7 +19,6 @@
package org.apache.accumulo.server.util;
import java.util.Collection;
-import java.util.HashMap;
import java.util.Map;
import org.apache.accumulo.core.conf.Property;
@@ -39,15 +38,18 @@ public class TablePropUtil extends PropUtil<TableId> {
* @throws IllegalStateException
* if an underlying exception (KeeperException,
InterruptException) or other failure to
* read properties from the cache / backend store
+ * @throws IllegalArgumentException
+ * if a provided property is not valid
*/
@Override
- public void setProperties(TableId tableId, Map<String,String> props) {
- Map<String,String> tempProps = new HashMap<>(props);
- // TODO reconcile with NamespacePropUtil see
https://github.com/apache/accumulo/issues/2633
- tempProps.entrySet().removeIf(e ->
!Property.isTablePropertyValid(e.getKey(), e.getValue()));
-
- context.getPropStore().putAll(TablePropKey.of(context, tableId), props);
-
+ public void setProperties(TableId tableId, Map<String,String> properties) {
+ for (Map.Entry<String,String> prop : properties.entrySet()) {
+ if (!Property.isTablePropertyValid(prop.getKey(), prop.getValue())) {
+ throw new IllegalArgumentException("Invalid property for table: " +
tableId + " name: "
+ + prop.getKey() + ", value: " + prop.getValue());
+ }
+ }
+ context.getPropStore().putAll(TablePropKey.of(context, tableId),
properties);
}
@Override
diff --git
a/test/src/main/java/org/apache/accumulo/test/ZooKeeperPropertiesIT.java
b/test/src/main/java/org/apache/accumulo/test/ZooKeeperPropertiesIT.java
index 5805341229..efb8f0a8d6 100644
--- a/test/src/main/java/org/apache/accumulo/test/ZooKeeperPropertiesIT.java
+++ b/test/src/main/java/org/apache/accumulo/test/ZooKeeperPropertiesIT.java
@@ -18,14 +18,28 @@
*/
package org.apache.accumulo.test;
+import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.fail;
+
+import java.util.List;
+import java.util.Map;
import org.apache.accumulo.core.client.Accumulo;
import org.apache.accumulo.core.client.AccumuloClient;
import org.apache.accumulo.core.client.AccumuloException;
+import org.apache.accumulo.core.client.AccumuloSecurityException;
+import org.apache.accumulo.core.client.NamespaceExistsException;
+import org.apache.accumulo.core.client.NamespaceNotFoundException;
+import org.apache.accumulo.core.client.TableExistsException;
+import org.apache.accumulo.core.client.TableNotFoundException;
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.harness.AccumuloClusterHarness;
+import org.apache.accumulo.server.ServerContext;
import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.Timeout;
public class ZooKeeperPropertiesIT extends AccumuloClusterHarness {
@@ -38,4 +52,106 @@ public class ZooKeeperPropertiesIT extends
AccumuloClusterHarness {
}
}
+ @Test
+ @Timeout(30)
+ public void testTablePropUtils() throws AccumuloException,
TableExistsException,
+ AccumuloSecurityException, TableNotFoundException {
+ ServerContext context = getServerContext();
+
+ try (AccumuloClient client =
Accumulo.newClient().from(getClientProps()).build()) {
+
+ final String tableName = getUniqueNames(1)[0];
+ client.tableOperations().create(tableName);
+ Map<String,String> idMap = client.tableOperations().tableIdMap();
+ String tid = idMap.get(tableName);
+
+ Map<String,String> properties =
client.tableOperations().getConfiguration(tableName);
+ assertEquals("false",
properties.get(Property.TABLE_BLOOM_ENABLED.getKey()));
+
+ context.tablePropUtil().setProperties(TableId.of(tid),
+ Map.of(Property.TABLE_BLOOM_ENABLED.getKey(), "true"));
+
+ // add a sleep to give the property change time to propagate
+ properties = client.tableOperations().getConfiguration(tableName);
+ while
(properties.get(Property.TABLE_BLOOM_ENABLED.getKey()).equals("false")) {
+ try {
+ Thread.sleep(250);
+ } catch (InterruptedException e) {
+ fail("Thread interrupted while waiting for tablePropUtil update");
+ }
+ properties = client.tableOperations().getConfiguration(tableName);
+ }
+
+ context.tablePropUtil().removeProperties(TableId.of(tid),
+ List.of(Property.TABLE_BLOOM_ENABLED.getKey()));
+
+ properties = client.tableOperations().getConfiguration(tableName);
+ while
(properties.get(Property.TABLE_BLOOM_ENABLED.getKey()).equals("true")) {
+ try {
+ Thread.sleep(250);
+ } catch (InterruptedException e) {
+ fail("Thread interrupted while waiting for tablePropUtil update");
+ }
+ properties = client.tableOperations().getConfiguration(tableName);
+ }
+
+ // Add invalid property
+ assertThrows(IllegalArgumentException.class,
+ () -> context.tablePropUtil().setProperties(TableId.of(tid),
+ Map.of("NOT_A_PROPERTY", "not_a_value")),
+ "Expected IllegalArgumentException to be thrown.");
+ }
+ }
+
+ @Test
+ @Timeout(30)
+ public void testNamespacePropUtils() throws AccumuloException,
AccumuloSecurityException,
+ NamespaceExistsException, NamespaceNotFoundException {
+ ServerContext context = getServerContext();
+
+ try (AccumuloClient client =
Accumulo.newClient().from(getClientProps()).build()) {
+
+ final String namespace = getUniqueNames(1)[0];
+ client.namespaceOperations().create(namespace);
+ Map<String,String> nsMap = client.namespaceOperations().namespaceIdMap();
+ String nid = nsMap.get(namespace);
+
+ Map<String,String> properties =
client.namespaceOperations().getConfiguration(namespace);
+ assertEquals("15", properties.get(Property.TABLE_FILE_MAX.getKey()));
+
+ context.namespacePropUtil().setProperties(NamespaceId.of(nid),
+ Map.of(Property.TABLE_FILE_MAX.getKey(), "31"));
+
+ // add a sleep to give the property change time to propagate
+ properties = client.namespaceOperations().getConfiguration(namespace);
+ while (!properties.get(Property.TABLE_FILE_MAX.getKey()).equals("31")) {
+ try {
+ Thread.sleep(250);
+ } catch (InterruptedException e) {
+ fail("Thread interrupted while waiting for namespacePropUtil
update");
+ }
+ properties = client.namespaceOperations().getConfiguration(namespace);
+ }
+
+ context.namespacePropUtil().removeProperties(NamespaceId.of(nid),
+ List.of(Property.TABLE_FILE_MAX.getKey()));
+
+ properties = client.namespaceOperations().getConfiguration(namespace);
+ while (!properties.get(Property.TABLE_FILE_MAX.getKey()).equals("15")) {
+ try {
+ Thread.sleep(250);
+ } catch (InterruptedException e) {
+ fail("Thread interrupted while waiting for namespacePropUtil
update");
+ }
+ properties = client.namespaceOperations().getConfiguration(namespace);
+ }
+
+ // Add invalid property
+ assertThrows(IllegalArgumentException.class,
+ () -> context.namespacePropUtil().setProperties(NamespaceId.of(nid),
+ Map.of("NOT_A_PROPERTY", "not_a_value")),
+ "Expected IllegalArgumentException to be thrown.");
+ }
+ }
+
}
