This is an automated email from the ASF dual-hosted git repository.
dlmarion pushed a commit to branch 2.1
in repository https://gitbox.apache.org/repos/asf/accumulo.git
The following commit(s) were added to refs/heads/2.1 by this push:
new af50af77ce Added validation of configuration during upgrade (#4289)
af50af77ce is described below
commit af50af77cea24a8c4c8cfe62c09b4238577e1034
Author: Dave Marion <dlmar...@apache.org>
AuthorDate: Thu Feb 22 12:19:36 2024 -0500
Added validation of configuration during upgrade (#4289)
Backported second parameter to CheckConfigUtil.validate and updated
all call locations to supply the second parameter. Added method to
UpgradeCoordinator to call CheckConfigUtil.validate after the
upgradeMetadata method is called on all Upgraders
Fixes #3972
---
core/pom.xml | 4 +++
.../apache/accumulo/core/conf/ConfigCheckUtil.java | 11 ++++---
.../accumulo/core/conf/SiteConfiguration.java | 2 +-
.../accumulo/core/conf/ConfigCheckUtilTest.java | 16 +++++-----
.../core/conf/DefaultConfigurationTest.java | 2 +-
.../server/conf/ServerConfigurationFactory.java | 4 +--
.../manager/upgrade/UpgradeCoordinator.java | 37 +++++++++++++++++++++-
7 files changed, 58 insertions(+), 18 deletions(-)
diff --git a/core/pom.xml b/core/pom.xml
index 43dcd6de30..d8cc050b33 100644
--- a/core/pom.xml
+++ b/core/pom.xml
@@ -135,6 +135,10 @@
<groupId>org.apache.zookeeper</groupId>
<artifactId>zookeeper-jute</artifactId>
</dependency>
+ <dependency>
+ <groupId>org.checkerframework</groupId>
+ <artifactId>checker-qual</artifactId>
+ </dependency>
<dependency>
<groupId>org.slf4j</groupId>
<artifactId>slf4j-api</artifactId>
diff --git
a/core/src/main/java/org/apache/accumulo/core/conf/ConfigCheckUtil.java
b/core/src/main/java/org/apache/accumulo/core/conf/ConfigCheckUtil.java
index b9e7ecfff3..ba3e7e543f 100644
--- a/core/src/main/java/org/apache/accumulo/core/conf/ConfigCheckUtil.java
+++ b/core/src/main/java/org/apache/accumulo/core/conf/ConfigCheckUtil.java
@@ -23,6 +23,7 @@ import java.util.Map.Entry;
import java.util.Objects;
import org.apache.accumulo.core.spi.crypto.CryptoServiceFactory;
+import org.checkerframework.checker.nullness.qual.NonNull;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -45,7 +46,7 @@ public class ConfigCheckUtil {
* @param entries iterable through configuration keys and values
* @throws ConfigCheckException if a fatal configuration error is found
*/
- public static void validate(Iterable<Entry<String,String>> entries) {
+ public static void validate(Iterable<Entry<String,String>> entries, @NonNull
String source) {
String instanceZkTimeoutValue = null;
for (Entry<String,String> entry : entries) {
String key = entry.getKey();
@@ -54,12 +55,12 @@ public class ConfigCheckUtil {
if (prop == null && Property.isValidPropertyKey(key)) {
continue; // unknown valid property (i.e. has proper prefix)
} else if (prop == null) {
- log.warn(PREFIX + "unrecognized property key (" + key + ")");
+ log.warn(PREFIX + "unrecognized property key ({}) for {}", key,
source);
} else if (prop.getType() == PropertyType.PREFIX) {
- fatal(PREFIX + "incomplete property key (" + key + ")");
+ fatal(PREFIX + "incomplete property key (" + key + ") for {}" +
source);
} else if (!prop.getType().isValidFormat(value)) {
fatal(PREFIX + "improperly formatted value for key (" + key + ",
type=" + prop.getType()
- + ") : " + value);
+ + ") : " + value + " for " + source);
}
if (key.equals(Property.INSTANCE_ZK_TIMEOUT.getKey())) {
@@ -128,7 +129,7 @@ public class ConfigCheckUtil {
}
/**
- * The exception thrown when {@link ConfigCheckUtil#validate(Iterable)}
fails.
+ * The exception thrown when {@link ConfigCheckUtil#validate(Iterable,
String)} fails.
*/
public static class ConfigCheckException extends RuntimeException {
private static final long serialVersionUID = 1L;
diff --git
a/core/src/main/java/org/apache/accumulo/core/conf/SiteConfiguration.java
b/core/src/main/java/org/apache/accumulo/core/conf/SiteConfiguration.java
index c20aab006b..196486f205 100644
--- a/core/src/main/java/org/apache/accumulo/core/conf/SiteConfiguration.java
+++ b/core/src/main/java/org/apache/accumulo/core/conf/SiteConfiguration.java
@@ -212,7 +212,7 @@ public class SiteConfiguration extends
AccumuloConfiguration {
private final Map<String,String> config;
private SiteConfiguration(Map<String,String> config) {
- ConfigCheckUtil.validate(config.entrySet());
+ ConfigCheckUtil.validate(config.entrySet(), "site config");
this.config = config;
}
diff --git
a/core/src/test/java/org/apache/accumulo/core/conf/ConfigCheckUtilTest.java
b/core/src/test/java/org/apache/accumulo/core/conf/ConfigCheckUtilTest.java
index 258e38075f..e204316c50 100644
--- a/core/src/test/java/org/apache/accumulo/core/conf/ConfigCheckUtilTest.java
+++ b/core/src/test/java/org/apache/accumulo/core/conf/ConfigCheckUtilTest.java
@@ -40,51 +40,51 @@ public class ConfigCheckUtilTest {
m.put(Property.MANAGER_TABLET_BALANCER.getKey(),
"org.apache.accumulo.server.manager.balancer.TableLoadBalancer");
m.put(Property.MANAGER_BULK_RETRIES.getKey(), "3");
- ConfigCheckUtil.validate(m.entrySet());
+ ConfigCheckUtil.validate(m.entrySet(), "test");
}
@Test
public void testPass_Empty() {
- ConfigCheckUtil.validate(m.entrySet());
+ ConfigCheckUtil.validate(m.entrySet(), "test");
}
@Test
public void testPass_UnrecognizedValidProperty() {
m.put(Property.MANAGER_CLIENTPORT.getKey(), "9999");
m.put(Property.MANAGER_PREFIX.getKey() + "something", "abcdefg");
- ConfigCheckUtil.validate(m.entrySet());
+ ConfigCheckUtil.validate(m.entrySet(), "test");
}
@Test
public void testPass_UnrecognizedProperty() {
m.put(Property.MANAGER_CLIENTPORT.getKey(), "9999");
m.put("invalid.prefix.value", "abcdefg");
- ConfigCheckUtil.validate(m.entrySet());
+ ConfigCheckUtil.validate(m.entrySet(), "test");
}
@Test
public void testFail_Prefix() {
m.put(Property.MANAGER_CLIENTPORT.getKey(), "9999");
m.put(Property.MANAGER_PREFIX.getKey(), "oops");
- assertThrows(ConfigCheckException.class, () ->
ConfigCheckUtil.validate(m.entrySet()));
+ assertThrows(ConfigCheckException.class, () ->
ConfigCheckUtil.validate(m.entrySet(), "test"));
}
@Test
public void testFail_InstanceZkTimeoutOutOfRange() {
m.put(Property.INSTANCE_ZK_TIMEOUT.getKey(), "10ms");
- assertThrows(ConfigCheckException.class, () ->
ConfigCheckUtil.validate(m.entrySet()));
+ assertThrows(ConfigCheckException.class, () ->
ConfigCheckUtil.validate(m.entrySet(), "test"));
}
@Test
public void testFail_badCryptoFactory() {
m.put(Property.INSTANCE_CRYPTO_FACTORY.getKey(),
"DoesNotExistCryptoFactory");
- assertThrows(ConfigCheckException.class, () ->
ConfigCheckUtil.validate(m.entrySet()));
+ assertThrows(ConfigCheckException.class, () ->
ConfigCheckUtil.validate(m.entrySet(), "test"));
}
@Test
public void testPass_defaultCryptoFactory() {
m.put(Property.INSTANCE_CRYPTO_FACTORY.getKey(),
Property.INSTANCE_CRYPTO_FACTORY.getDefaultValue());
- ConfigCheckUtil.validate(m.entrySet());
+ ConfigCheckUtil.validate(m.entrySet(), "test");
}
}
diff --git
a/core/src/test/java/org/apache/accumulo/core/conf/DefaultConfigurationTest.java
b/core/src/test/java/org/apache/accumulo/core/conf/DefaultConfigurationTest.java
index 123b5bc326..8ddecfbdf3 100644
---
a/core/src/test/java/org/apache/accumulo/core/conf/DefaultConfigurationTest.java
+++
b/core/src/test/java/org/apache/accumulo/core/conf/DefaultConfigurationTest.java
@@ -52,6 +52,6 @@ public class DefaultConfigurationTest {
@Test
public void testSanityCheck() {
- ConfigCheckUtil.validate(c);
+ ConfigCheckUtil.validate(c, "test");
}
}
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 21f7822461..e5ac4945a3 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
@@ -115,7 +115,7 @@ public class ServerConfigurationFactory extends
ServerConfiguration {
context.getPropStore().registerAsListener(TablePropKey.of(context,
tableId), changeWatcher);
var conf =
new TableConfiguration(context, tableId,
getNamespaceConfigurationForTable(tableId));
- ConfigCheckUtil.validate(conf);
+ ConfigCheckUtil.validate(conf, "table id: " + tableId.toString());
return conf;
}
return null;
@@ -138,7 +138,7 @@ public class ServerConfigurationFactory extends
ServerConfiguration {
context.getPropStore().registerAsListener(NamespacePropKey.of(context,
namespaceId),
changeWatcher);
var conf = new NamespaceConfiguration(context, namespaceId,
getSystemConfiguration());
- ConfigCheckUtil.validate(conf);
+ ConfigCheckUtil.validate(conf, "namespace id: " +
namespaceId.toString());
return conf;
});
}
diff --git
a/server/manager/src/main/java/org/apache/accumulo/manager/upgrade/UpgradeCoordinator.java
b/server/manager/src/main/java/org/apache/accumulo/manager/upgrade/UpgradeCoordinator.java
index d6494356ec..d62e824e05 100644
---
a/server/manager/src/main/java/org/apache/accumulo/manager/upgrade/UpgradeCoordinator.java
+++
b/server/manager/src/main/java/org/apache/accumulo/manager/upgrade/UpgradeCoordinator.java
@@ -29,6 +29,10 @@ import java.util.concurrent.TimeUnit;
import org.apache.accumulo.core.Constants;
import org.apache.accumulo.core.client.AccumuloException;
+import org.apache.accumulo.core.client.AccumuloSecurityException;
+import org.apache.accumulo.core.client.NamespaceNotFoundException;
+import org.apache.accumulo.core.client.TableNotFoundException;
+import org.apache.accumulo.core.conf.ConfigCheckUtil;
import org.apache.accumulo.core.dataImpl.KeyExtent;
import org.apache.accumulo.core.fate.ReadOnlyTStore;
import org.apache.accumulo.core.fate.ZooStore;
@@ -78,6 +82,15 @@ public class UpgradeCoordinator {
return extent.isMeta();
}
},
+ /**
+ * This signifies that zookeeper and the root and metadata tables have
been upgraded so far.
+ */
+ UPGRADED_METADATA {
+ @Override
+ public boolean isParentLevelUpgraded(KeyExtent extent) {
+ return extent.isMeta();
+ }
+ },
/**
* This signifies that everything (zookeeper, root table, metadata table)
is upgraded.
*/
@@ -188,13 +201,16 @@ public class UpgradeCoordinator {
log.info("Upgrading Root from data version {}", v);
upgraders.get(v).upgradeRoot(context);
}
-
setStatus(UpgradeStatus.UPGRADED_ROOT, eventCoordinator);
for (int v = currentVersion; v < AccumuloDataVersion.get(); v++)
{
log.info("Upgrading Metadata from data version {}", v);
upgraders.get(v).upgradeMetadata(context);
}
+ setStatus(UpgradeStatus.UPGRADED_METADATA, eventCoordinator);
+
+ log.info("Validating configuration properties.");
+ validateProperties(context);
log.info("Updating persistent data version.");
updateAccumuloVersion(context.getServerDirs(),
context.getVolumeManager(),
@@ -211,6 +227,25 @@ public class UpgradeCoordinator {
}
}
+ private void validateProperties(ServerContext context) {
+ ConfigCheckUtil.validate(context.getSiteConfiguration(), "site
configuration");
+ ConfigCheckUtil.validate(context.getConfiguration(), "system
configuration");
+ try {
+ for (String ns : context.namespaceOperations().list()) {
+ ConfigCheckUtil.validate(
+
context.namespaceOperations().getNamespaceProperties(ns).entrySet(),
+ ns + " namespace configuration");
+ }
+ for (String table : context.tableOperations().list()) {
+
ConfigCheckUtil.validate(context.tableOperations().getTableProperties(table).entrySet(),
+ table + " table configuration");
+ }
+ } catch (AccumuloException | AccumuloSecurityException |
NamespaceNotFoundException
+ | TableNotFoundException e) {
+ throw new IllegalStateException("Error checking properties", e);
+ }
+ }
+
// visible for testing
synchronized void updateAccumuloVersion(ServerDirs serverDirs, VolumeManager
fs, int oldVersion) {
for (Volume volume : fs.getVolumes()) {
