kevinrr888 commented on code in PR #5781:
URL: https://github.com/apache/accumulo/pull/5781#discussion_r2299124283
##########
server/manager/src/main/java/org/apache/accumulo/manager/upgrade/Upgrader12to13.java:
##########
@@ -438,4 +447,51 @@ void addTableMappingsToZooKeeper(ServerContext context) {
"Could not read or write metadata in ZooKeeper because of ZooKeeper
exception", ex);
}
}
+
+ void moveTableProperties(ServerContext context) {
+ try {
+ final SystemPropKey spk = SystemPropKey.of();
+ final VersionedProperties sysProps = context.getPropStore().get(spk);
+ final Map<String,String> sysTableProps = new HashMap<>();
+ sysProps.asMap().entrySet().stream()
+ .filter(e -> e.getKey().startsWith(Property.TABLE_PREFIX.getKey()))
+ .forEach(e -> sysTableProps.put(e.getKey(), e.getValue()));
+ LOG.info("Adding the following table properties to namespaces unless
overridden:");
+ sysTableProps.forEach((k, v) -> LOG.info("{} -> {}", k, v));
+
+ for (String ns : context.namespaceOperations().list()) {
+ final NamespacePropKey nsk = NamespacePropKey.of(NamespaceId.of(ns));
+ final Map<String,String> nsProps =
context.getPropStore().get(nsk).asMap();
+ final Map<String,String> nsPropAdditions = new HashMap<>();
+
+ for (Entry<String,String> e : sysTableProps.entrySet()) {
+ final String nsVal = nsProps.get(e.getKey());
+ // If it's not set, then add the system table property
+ // to the namespace. If it is set, then it doesnt matter
+ // what the value is, we can ignore it.
+ if (nsVal == null) {
+ nsPropAdditions.put(e.getKey(), e.getValue());
+ }
+ }
+ try {
+ context.getPropStore().putAll(nsk, nsPropAdditions);
+ LOG.debug("Added table properties to namespace {}:", ns);
+ nsPropAdditions.forEach((k, v) -> LOG.debug("{} -> {}", k, v));
+ } catch (IllegalStateException e1) {
+ throw e1;
+ }
Review Comment:
```suggestion
context.getPropStore().putAll(nsk, nsPropAdditions);
LOG.debug("Added table properties to namespace {}:", ns);
nsPropAdditions.forEach((k, v) -> LOG.debug("{} -> {}", k, v));
```
##########
test/src/main/java/org/apache/accumulo/test/conf/PropStoreConfigIT_SimpleSuite.java:
##########
@@ -97,23 +99,32 @@ public void setTablePropTest() throws Exception {
try (var client = Accumulo.newClient().from(getClientProps()).build()) {
client.tableOperations().create(table);
+ NamespaceId nid = client.tableOperations().getNamespace(table);
log.debug("Tables: {}", client.tableOperations().list());
- // override default in sys, and then over-ride that for table prop
-
client.instanceOperations().setProperty(Property.TABLE_BLOOM_ENABLED.getKey(),
"true");
Review Comment:
How did you find which tests set a table prop at system level?
##########
test/src/main/java/org/apache/accumulo/test/conf/PropStoreConfigIT_SimpleSuite.java:
##########
@@ -97,23 +99,32 @@ public void setTablePropTest() throws Exception {
try (var client = Accumulo.newClient().from(getClientProps()).build()) {
client.tableOperations().create(table);
+ NamespaceId nid = client.tableOperations().getNamespace(table);
log.debug("Tables: {}", client.tableOperations().list());
- // override default in sys, and then over-ride that for table prop
-
client.instanceOperations().setProperty(Property.TABLE_BLOOM_ENABLED.getKey(),
"true");
+ AccumuloException ae = assertThrows(AccumuloException.class, () ->
client.instanceOperations()
+ .setProperty(Property.TABLE_BLOOM_ENABLED.getKey(), "true"));
+ assertTrue(ae.getMessage()
+ .contains("Table property " + Property.TABLE_BLOOM_ENABLED.getKey()
+ + " cannot be set at the system level."
+ + " Set table properties at the namespace or table level."));
+ // override default in namespace, and then over-ride that for table prop
+ client.namespaceOperations().setProperty(nid.canonical(),
+ Property.TABLE_BLOOM_ENABLED.getKey(), "true");
client.tableOperations().setProperty(table,
Property.TABLE_BLOOM_ENABLED.getKey(), "false");
- Wait.waitFor(() -> client.instanceOperations().getSystemConfiguration()
+ Wait.waitFor(() ->
client.namespaceOperations().getConfiguration(nid.canonical())
.get(Property.TABLE_BLOOM_ENABLED.getKey()).equals("true"), 5000,
500);
Wait.waitFor(() -> client.tableOperations().getConfiguration(table)
.get(Property.TABLE_BLOOM_ENABLED.getKey()).equals("false"), 5000,
500);
// revert sys, and then over-ride to true with table prop
Review Comment:
```suggestion
// revert namespace prop, and then over-ride to true with table prop
```
##########
test/src/main/java/org/apache/accumulo/test/ComprehensiveTableOperationsIT_SimpleSuite.java:
##########
@@ -746,6 +748,23 @@ public void
test_setTabletAvailability_getTabletInformation() throws Exception {
}
}
+ @Test
+ public void test_getNamespace() throws Exception {
+ assertEquals(NamespaceId.of(Namespace.ACCUMULO.name()),
+ ops.getNamespace(SystemTables.METADATA.tableName()));
Review Comment:
Good to test for all sys tables
```suggestion
for (var sysTable : SystemTables.tableNames()) {
assertEquals(NamespaceId.of(Namespace.ACCUMULO.name()),
ops.getNamespace(sysTable);
}
```
##########
server/manager/src/main/java/org/apache/accumulo/manager/upgrade/Upgrader12to13.java:
##########
@@ -438,4 +447,51 @@ void addTableMappingsToZooKeeper(ServerContext context) {
"Could not read or write metadata in ZooKeeper because of ZooKeeper
exception", ex);
}
}
+
+ void moveTableProperties(ServerContext context) {
+ try {
+ final SystemPropKey spk = SystemPropKey.of();
+ final VersionedProperties sysProps = context.getPropStore().get(spk);
+ final Map<String,String> sysTableProps = new HashMap<>();
+ sysProps.asMap().entrySet().stream()
+ .filter(e -> e.getKey().startsWith(Property.TABLE_PREFIX.getKey()))
+ .forEach(e -> sysTableProps.put(e.getKey(), e.getValue()));
+ LOG.info("Adding the following table properties to namespaces unless
overridden:");
+ sysTableProps.forEach((k, v) -> LOG.info("{} -> {}", k, v));
Review Comment:
Avoids 2 iterations
```suggestion
LOG.info("Adding the following table properties to namespaces unless
overridden:");
sysProps.asMap().entrySet().stream()
.filter(e -> e.getKey().startsWith(Property.TABLE_PREFIX.getKey()))
.forEach(e -> {
sysTableProps.put(e.getKey(), e.getValue());
LOG.info("{} -> {}", e.getKey(), e.getValue())
});
sysTableProps.forEach((k, v) -> LOG.info("{} -> {}", k, v));
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
