keith-turner commented on code in PR #4807:
URL: https://github.com/apache/accumulo/pull/4807#discussion_r1722424505


##########
server/base/src/main/java/org/apache/accumulo/server/util/Admin.java:
##########
@@ -907,4 +1003,129 @@ private EnumSet<ReadOnlyTStore.TStatus> 
getCmdLineStatusFilters(List<String> sta
     }
     return statusFilter;
   }
+
+  @VisibleForTesting
+  public static void executeCheckCommand(ServerContext context, CheckCommand 
cmd) {
+    List<CheckCommand.Check> checks;
+
+    validateAndTransformCheckCommand(cmd);
+
+    if (cmd.list) {
+      listChecks();
+    } else if (cmd.run) {
+      checks = 
cmd.checks.stream().map(CheckCommand.Check::valueOf).collect(Collectors.toList());
+      executeRunCheckCommand(checks);
+    }
+  }
+
+  private static void validateAndTransformCheckCommand(CheckCommand cmd) {
+    Preconditions.checkArgument(cmd.list != cmd.run, "Must use either 'list' 
or 'run'");
+    if (cmd.list) {
+      Preconditions.checkArgument(cmd.checks == null,
+          "'list' does not expect any further arguments");
+    } else if (cmd.useRegex) {
+      // run with regex provided
+      Preconditions.checkArgument(cmd.checks != null, "Expected a regex 
pattern to be provided");
+      Preconditions.checkArgument(cmd.checks.size() == 1,
+          "Expected one argument (the regex pattern)");
+      String regex = cmd.checks.get(0).toUpperCase();
+      List<String> matchingChecks = new ArrayList<>();
+      var pattern = Pattern.compile(regex);
+      for (CheckCommand.Check check : CheckCommand.Check.values()) {
+        if (pattern.matcher(check.name()).matches()) {
+          matchingChecks.add(check.name());
+        }
+      }
+      Preconditions.checkArgument(!matchingChecks.isEmpty(),
+          "No checks matched the given pattern: " + regex);
+      cmd.checks = matchingChecks;
+    } else {
+      // run without regex provided
+      if (cmd.checks == null) {
+        cmd.checks = 
EnumSet.allOf(CheckCommand.Check.class).stream().map(Enum::name)
+            .collect(Collectors.toList());
+      }
+    }
+  }
+
+  private static void listChecks() {
+    System.out.println();
+    System.out.printf("%-20s | %-80s | %-20s%n", "Check Name", "Description", 
"Depends on");
+    System.out.println("-".repeat(120));
+    for (CheckCommand.Check check : CheckCommand.Check.values()) {
+      System.out.printf("%-20s | %-80s | %-20s%n", check.name(),
+          CheckCommand.Check.CHECK_DESCRIPTION.get(check), 
check.getDependencies().stream()
+              .map(CheckCommand.Check::name).collect(Collectors.joining(", 
")));
+    }
+    System.out.println("-".repeat(120));
+    System.out.println();
+  }
+
+  private static void executeRunCheckCommand(List<CheckCommand.Check> checks) {
+    final List<CheckCommand.Check> allChecksToRun = new ArrayList<>();
+    final TreeMap<CheckCommand.Check,CheckCommand.CheckStatus> checkStatus = 
new TreeMap<>();
+
+    // From the given list of checks to run, we need to compute all the checks 
that need to be
+    // run (the given checks and all the checks the given checks depend on)
+    computeAllChecksToRun(checks, allChecksToRun);

Review Comment:
   When checks were filtered I was thinking it would not run their 
dependencies.  Was thinking when a user wants to run specific checks that they 
may not want to wait on others.



##########
server/base/src/main/java/org/apache/accumulo/server/util/Admin.java:
##########
@@ -114,6 +129,82 @@ static class PingCommand {
     List<String> args = new ArrayList<>();
   }
 
+  @Parameters(commandNames = "check",
+      commandDescription = "Performs checks for problems in Accumulo.")
+  public static class CheckCommand {
+    @Parameter(names = "list",
+        description = "Lists the different checks that can be run, the 
description of each check, and the other check(s) each check depends on.")
+    boolean list;
+
+    @Parameter(names = "run",
+        description = "Runs the provided check(s) (explicit list or regex 
pattern specified following '-p'), beginning with their dependencies, or all 
checks if none are provided.")
+    boolean run;
+
+    @Parameter(names = {"--name_pattern", "-p"},
+        description = "Runs all checks that match the provided regex pattern.")
+    boolean useRegex;
+
+    @Parameter(description = "[<Check>...]")
+    List<String> checks;
+
+    public enum Check {
+      // Caution should be taken when changing or adding any new checks: order 
is important
+      SYSTEM_CONFIG, ROOT_METADATA, ROOT_TABLE, METADATA_TABLE, SYSTEM_FILES, 
USER_FILES;
+
+      private static final Map<Check,String> CHECK_DESCRIPTION = new 
EnumMap<>(Check.class);
+      private static final Map<Check,List<Check>> CHECK_DEPENDENCIES = new 
EnumMap<>(Check.class);
+      private static final Map<Check,Supplier<CheckRunner>> CHECK_RUNNERS =
+          new EnumMap<>(Check.class);
+
+      static {
+        CHECK_DESCRIPTION.put(SYSTEM_CONFIG, "Validate the system config 
stored in ZooKeeper");

Review Comment:
   Enums can have constructors.  So could use those instead of the static maps. 
 MAy be able to do something like the following.
   
   ```java
   public enum Check {
       SYSTEM_CONFIG(SystemConfigCheckRunner::new, "Validate the system config 
stored in ZooKeeper", List.of()),
       ROOT_METADATA(RootMetadataCheckRunner::new, "Checks integrity of the 
root tablet metadata stored in ZooKeeper", List.of(SYSTEM_CONFIG));
   
   
       private final Supplier<CheckRunner> checkRunner;
       private final String description;
       private final List<Check> dependencies;
   
       Check(Supplier<CheckRunner> checkRunner, String description, List<Check> 
deps) {
           this.checkRunner = Objects.requireNonNull(checkRunner);
           this.description = Objects.requireNonNull(description);
           this.dependencies = Objects.requireNonNull(deps);
       }
   
       /**
        * @return the list of other checks the check depends on
        */
       private List<Check> getDependencies() {
           return dependencies;
       }
   
       /**
        * @return the interface for running a check
        */
       private CheckRunner getCheckRunner() {
           return checkRunner.get();
       }
   }
   ```



-- 
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]

Reply via email to