Completed functionality for admin check command

server/base/src/main/java/org/apache/accumulo/server/conf/CheckAccumuloProperties.java

@@ -24,6 +24,7 @@
 import org.apache.accumulo.core.conf.SiteConfiguration;
 import org.apache.accumulo.server.ServerDirs;
 import org.apache.accumulo.server.fs.VolumeManagerImpl;
+import org.apache.accumulo.server.util.Admin;
 import org.apache.accumulo.start.spi.KeywordExecutable;
 import org.apache.hadoop.conf.Configuration;
 
@@ -45,7 +46,7 @@ public String description() {
     return "Checks the provided Accumulo configuration file for errors. "
         + "This only checks the contents of the file and not any running Accumulo system, "
         + "so it can be used prior to init, but only performs a subset of the checks done by "
-        + (new CheckServerConfig().keyword());
+        + "'admin check run " + Admin.CheckCommand.Check.SERVER_CONFIG + "'";
   }
 
   @SuppressFBWarnings(value = "PATH_TRAVERSAL_IN", justification = "intentional user-provided path")

server/base/src/main/java/org/apache/accumulo/server/log/WalStateManager.java

@@ -144,7 +144,7 @@ public void walUnreferenced(TServerInstance tsi, Path path) throws WalMarkerExce
     updateState(tsi, path, WalState.UNREFERENCED);
   }
 
-  private static Pair<WalState,Path> parse(byte[] data) {
+  public static Pair<WalState,Path> parse(byte[] data) {
     String[] parts = new String(data, UTF_8).split(",");
     return new Pair<>(WalState.valueOf(parts[0]), new Path(parts[1]));
   }

server/base/src/main/java/org/apache/accumulo/server/util/Admin.java

@@ -101,6 +101,7 @@
 import org.apache.accumulo.server.util.checkCommand.MetadataTableCheckRunner;
 import org.apache.accumulo.server.util.checkCommand.RootMetadataCheckRunner;
 import org.apache.accumulo.server.util.checkCommand.RootTableCheckRunner;
+import org.apache.accumulo.server.util.checkCommand.ServerConfigCheckRunner;
 import org.apache.accumulo.server.util.checkCommand.SystemConfigCheckRunner;
 import org.apache.accumulo.server.util.checkCommand.SystemFilesCheckRunner;
 import org.apache.accumulo.server.util.checkCommand.TableLocksCheckRunner;
@@ -191,6 +192,8 @@ public enum Check {
       // Caution should be taken when changing or adding any new checks: order is important
       SYSTEM_CONFIG(SystemConfigCheckRunner::new, "Validate the system config stored in ZooKeeper",
           Collections.emptyList()),
+      SERVER_CONFIG(ServerConfigCheckRunner::new, "Validate the server configuration",
+          Collections.singletonList(SYSTEM_CONFIG)),
       TABLE_LOCKS(TableLocksCheckRunner::new,
           "Ensures that table and namespace locks are valid and are associated with a FATE op",
           Collections.singletonList(SYSTEM_CONFIG)),

server/base/src/main/java/org/apache/accumulo/server/util/checkCommand/MetadataCheckRunner.java

@@ -27,7 +27,6 @@
 
 import org.apache.accumulo.core.client.IteratorSetting;
 import org.apache.accumulo.core.client.Scanner;
-import org.apache.accumulo.core.client.TableNotFoundException;
 import org.apache.accumulo.core.data.Key;
 import org.apache.accumulo.core.data.Mutation;
 import org.apache.accumulo.core.data.TableId;
@@ -43,17 +42,22 @@
 import org.apache.accumulo.server.constraints.SystemEnvironment;
 import org.apache.accumulo.server.util.Admin;
 import org.apache.hadoop.io.Text;
-import org.apache.zookeeper.KeeperException;
 
 public interface MetadataCheckRunner extends CheckRunner {
 
   String tableName();
 
   TableId tableId();
 
-  Set<ColumnFQ> requiredColFQs();
+  default Set<ColumnFQ> requiredColFQs() {
+    return Set.of(MetadataSchema.TabletsSection.TabletColumnFamily.PREV_ROW_COLUMN,
+        MetadataSchema.TabletsSection.ServerColumnFamily.DIRECTORY_COLUMN,
+        MetadataSchema.TabletsSection.ServerColumnFamily.TIME_COLUMN);
+  }
 
-  Set<Text> requiredColFams();
+  default Set<Text> requiredColFams() {
+    return Set.of(MetadataSchema.TabletsSection.CurrentLocationColumnFamily.NAME);
+  }
 
   default String scanning() {
     return String.format("%s (%s) table", tableName(), tableId());
@@ -64,8 +68,7 @@ default String scanning() {
    * that are expected. For the root metadata, ensures that the expected "columns" exist in ZK.
    */
   default Admin.CheckCommand.CheckStatus checkRequiredColumns(ServerContext context,
-      Admin.CheckCommand.CheckStatus status)
-      throws TableNotFoundException, InterruptedException, KeeperException {
+      Admin.CheckCommand.CheckStatus status) throws Exception {
     Set<ColumnFQ> requiredColFQs;
     Set<Text> requiredColFams;
     boolean missingReqCol = false;

server/base/src/main/java/org/apache/accumulo/server/util/checkCommand/MetadataTableCheckRunner.java

@@ -24,9 +24,7 @@
 import org.apache.accumulo.core.client.Scanner;
 import org.apache.accumulo.core.data.TableId;
 import org.apache.accumulo.core.metadata.SystemTables;
-import org.apache.accumulo.core.metadata.schema.MetadataSchema;
 import org.apache.accumulo.core.security.Authorizations;
-import org.apache.accumulo.core.util.ColumnFQ;
 import org.apache.accumulo.server.ServerContext;
 import org.apache.accumulo.server.cli.ServerUtilOpts;
 import org.apache.accumulo.server.util.Admin;
@@ -47,13 +45,6 @@ public TableId tableId() {
     return SystemTables.METADATA.tableId();
   }
 
-  @Override
-  public Set<ColumnFQ> requiredColFQs() {
-    return Set.of(MetadataSchema.TabletsSection.TabletColumnFamily.PREV_ROW_COLUMN,
-        MetadataSchema.TabletsSection.ServerColumnFamily.DIRECTORY_COLUMN,
-        MetadataSchema.TabletsSection.ServerColumnFamily.TIME_COLUMN);
-  }
-
   @Override
   public Set<Text> requiredColFams() {
     return Set.of();

server/base/src/main/java/org/apache/accumulo/server/util/checkCommand/RootMetadataCheckRunner.java

@@ -23,19 +23,16 @@
 import java.util.HashSet;
 import java.util.Set;
 
-import org.apache.accumulo.core.client.TableNotFoundException;
 import org.apache.accumulo.core.data.TableId;
 import org.apache.accumulo.core.metadata.RootTable;
 import org.apache.accumulo.core.metadata.SystemTables;
-import org.apache.accumulo.core.metadata.schema.MetadataSchema;
 import org.apache.accumulo.core.metadata.schema.RootTabletMetadata;
 import org.apache.accumulo.core.util.ColumnFQ;
 import org.apache.accumulo.server.ServerContext;
 import org.apache.accumulo.server.cli.ServerUtilOpts;
 import org.apache.accumulo.server.util.Admin;
 import org.apache.accumulo.server.util.FindOfflineTablets;
 import org.apache.hadoop.io.Text;
-import org.apache.zookeeper.KeeperException;
 
 public class RootMetadataCheckRunner implements MetadataCheckRunner {
   private static final Admin.CheckCommand.Check check = Admin.CheckCommand.Check.ROOT_METADATA;
@@ -50,27 +47,14 @@ public TableId tableId() {
     throw new UnsupportedOperationException();
   }
 
-  @Override
-  public Set<ColumnFQ> requiredColFQs() {
-    return Set.of(MetadataSchema.TabletsSection.TabletColumnFamily.PREV_ROW_COLUMN,
-        MetadataSchema.TabletsSection.ServerColumnFamily.DIRECTORY_COLUMN,
-        MetadataSchema.TabletsSection.ServerColumnFamily.TIME_COLUMN,
-        MetadataSchema.TabletsSection.ServerColumnFamily.LOCK_COLUMN);
-  }
-
-  @Override
-  public Set<Text> requiredColFams() {
-    return Set.of(MetadataSchema.TabletsSection.CurrentLocationColumnFamily.NAME);
-  }
-
   @Override
   public String scanning() {
     return "root tablet metadata in ZooKeeper";
   }
 
   @Override
   public Admin.CheckCommand.CheckStatus runCheck(ServerContext context, ServerUtilOpts opts,
-      boolean fixFiles) throws TableNotFoundException, InterruptedException, KeeperException {
+      boolean fixFiles) throws Exception {
     Admin.CheckCommand.CheckStatus status = Admin.CheckCommand.CheckStatus.OK;
     printRunning();
 
@@ -97,7 +81,7 @@ public Admin.CheckCommand.CheckStatus runCheck(ServerContext context, ServerUtil
 
   @Override
   public Admin.CheckCommand.CheckStatus checkRequiredColumns(ServerContext context,
-      Admin.CheckCommand.CheckStatus status) throws InterruptedException, KeeperException {
+      Admin.CheckCommand.CheckStatus status) throws Exception {
     final String json =
         new String(context.getZooSession().asReader().getData(RootTable.ZROOT_TABLET), UTF_8);
     final var rtm = new RootTabletMetadata(json);

server/base/src/main/java/org/apache/accumulo/server/util/checkCommand/RootTableCheckRunner.java

@@ -19,20 +19,16 @@
 package org.apache.accumulo.server.util.checkCommand;
 
 import java.util.AbstractMap;
-import java.util.Set;
 
 import org.apache.accumulo.core.client.Scanner;
 import org.apache.accumulo.core.data.TableId;
 import org.apache.accumulo.core.metadata.SystemTables;
-import org.apache.accumulo.core.metadata.schema.MetadataSchema;
 import org.apache.accumulo.core.security.Authorizations;
-import org.apache.accumulo.core.util.ColumnFQ;
 import org.apache.accumulo.server.ServerContext;
 import org.apache.accumulo.server.cli.ServerUtilOpts;
 import org.apache.accumulo.server.util.Admin;
 import org.apache.accumulo.server.util.CheckForMetadataProblems;
 import org.apache.accumulo.server.util.FindOfflineTablets;
-import org.apache.hadoop.io.Text;
 
 public class RootTableCheckRunner implements MetadataCheckRunner {
   private static final Admin.CheckCommand.Check check = Admin.CheckCommand.Check.ROOT_TABLE;
@@ -47,19 +43,6 @@ public TableId tableId() {
     return SystemTables.ROOT.tableId();
   }
 
-  @Override
-  public Set<ColumnFQ> requiredColFQs() {
-    return Set.of(MetadataSchema.TabletsSection.TabletColumnFamily.PREV_ROW_COLUMN,
-        MetadataSchema.TabletsSection.ServerColumnFamily.DIRECTORY_COLUMN,
-        MetadataSchema.TabletsSection.ServerColumnFamily.TIME_COLUMN,
-        MetadataSchema.TabletsSection.ServerColumnFamily.LOCK_COLUMN);
-  }
-
-  @Override
-  public Set<Text> requiredColFams() {
-    return Set.of(MetadataSchema.TabletsSection.CurrentLocationColumnFamily.NAME);
-  }
-
   @Override
   public Admin.CheckCommand.CheckStatus runCheck(ServerContext context, ServerUtilOpts opts,
       boolean fixFiles) throws Exception {

server/base/src/main/java/org/apache/accumulo/server/util/checkCommand/ServerConfigCheckRunner.java

@@ -0,0 +1,86 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.accumulo.server.util.checkCommand;
+
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Set;
+
+import org.apache.accumulo.core.conf.Property;
+import org.apache.accumulo.server.ServerContext;
+import org.apache.accumulo.server.cli.ServerUtilOpts;
+import org.apache.accumulo.server.util.Admin;
+
+public class ServerConfigCheckRunner implements CheckRunner {
+  private static final Admin.CheckCommand.Check check = Admin.CheckCommand.Check.SERVER_CONFIG;
+
+  @Override
+  public Admin.CheckCommand.CheckStatus runCheck(ServerContext context, ServerUtilOpts opts,
+      boolean fixFiles) throws Exception {
+    Admin.CheckCommand.CheckStatus status = Admin.CheckCommand.CheckStatus.OK;
+    printRunning();
+
+    log.trace("********** Checking server configuration **********");
+
+    log.trace("Checking that all configured properties are valid (valid key and value)");
+    final Map<String,String> definedProps = new HashMap<>();
+    final var config = context.getConfiguration();
+    config.getProperties(definedProps, s -> true);
+    for (var entry : definedProps.entrySet()) {
+      var key = entry.getKey();
+      var val = entry.getValue();
+      if (!Property.isValidProperty(key, val)) {
+        log.warn("Invalid property (key={} val={}) found in the config", key, val);
+        status = Admin.CheckCommand.CheckStatus.FAILED;
+      }
+    }
+
+    log.trace("Checking that all required config properties are present");
+    // there are many properties that should be set (default value or user set), identifying them
+    // all and checking them here is unrealistic. Some property that is not set but is expected
+    // will likely result in some sort of failure eventually anyway. We will just check a few
+    // obvious required properties here.
+    Set<Property> requiredProps = Set.of(Property.INSTANCE_ZK_HOST, Property.INSTANCE_ZK_TIMEOUT,
+        Property.INSTANCE_SECRET, Property.INSTANCE_VOLUMES, Property.GENERAL_THREADPOOL_SIZE,
+        Property.GENERAL_DELEGATION_TOKEN_LIFETIME,
+        Property.GENERAL_DELEGATION_TOKEN_UPDATE_INTERVAL, Property.GENERAL_IDLE_PROCESS_INTERVAL,
+        Property.GENERAL_LOW_MEM_DETECTOR_INTERVAL, Property.GENERAL_LOW_MEM_DETECTOR_THRESHOLD,
+        Property.GENERAL_SERVER_LOCK_VERIFICATION_INTERVAL, Property.MANAGER_CLIENTPORT,
+        Property.TSERV_CLIENTPORT, Property.GC_CYCLE_START, Property.GC_CYCLE_DELAY,
+        Property.GC_PORT, Property.MONITOR_PORT, Property.TABLE_MAJC_RATIO,
+        Property.TABLE_SPLIT_THRESHOLD);
+    for (var reqProp : requiredProps) {
+      var confPropVal = config.get(reqProp);
+      // already checked that all set properties are valid, just check that it is set then we know
+      // it's valid
+      if (confPropVal == null || confPropVal.isEmpty()) {
+        log.warn("Required property {} is not set!", reqProp);
+        status = Admin.CheckCommand.CheckStatus.FAILED;
+      }
+    }
+
+    printCompleted(status);
+    return status;
+  }
+
+  @Override
+  public Admin.CheckCommand.Check getCheck() {
+    return check;
+  }
+}