accumulo icon indicating copy to clipboard operation
accumulo copied to clipboard

Create an admin command that performs a comprehensive check for problems

Open keith-turner opened this issue 1 year ago • 11 comments

Accumulo has at least two admin command that check for some problems. The accumulo admin checkTablets will check for a few problems related to tablets. The accumulo admin fate command can look for table locks that reference a non existent fate operation. There may be other admin command that look for problem.

Having to know which admin command to run to look for problems is a bit problematic if one does not know what problems may exists. These existing checks could be consolidated under a new command like accumulo admin check that will check as many things as possible in accumulo looking for problems. Then this single command could be run to find any problems that may exists and would be a useful thing to run periodically.

The following is a list of things this new command could check. Some of these checks already exists in different places in the accumulo code base.

  • Look for malformed data like :
    • Zookeeper node that contains an unexpected value (like if a positive integer is expected in the value and it contains something else).
    • Tablet metadata column that contains an unexpected value
    • Configuration property that contains an unexpected value
  • Look for extraneous data like :
    • Unexpected columns in the metadata table
    • Unexpected nodes in zookeeper
    • Unexpected files in HDFS (could be unreferenced tablet files or just junk files)
    • Unexpected config properties
  • Look for missing data like :
    • Nodes that are expected to exists in zookeeper but are not present
    • Columns that are expected to exists in tablets metadata that are not present (there are a few columns every tablet is expected to have like prev row, dir, availability, etc)
    • Required configuration properties that are absent, like instance.volumes
  • Check references like :
    • Ensure table locks point to a valid table and fate operation
    • Ensure tablets prev row is valid
    • Ensure tablets reference files that actually exist
    • Ensure tablets reference valid fate operations
  • Check consistency like :
    • A small amount of metadata is written to DFS for each volume in Accumulo, its expected this metadata is consistent across volumes.
    • Server processes that have inconsistent essential configuration.

This command could potentially find a lot of problems. Some of these problems may have automated fixes. The command could optionally automatically fix those that can be fixed. The user would need a way to control what is automatically fixed. May not want to automatically fix all problems found.

keith-turner avatar Jun 20 '24 18:06 keith-turner

Opened this issue after working on #4686 and trying to determine where that new functionality should go. The functionality added in #4686 could have gone under the existing admin checkTablets or admin fate command.

keith-turner avatar Jun 20 '24 18:06 keith-turner

This looks like it would be a really good feature to add. It would be nice to also optionally be able to configure the checks to run on some schedule and generate a report or something without having to always manually run the command. I'm not sure how you would report problems other than logs (maybe in the monitor?) but that could be useful. Of course another option is someone could just configure the command to run periodically with cron or something like that so we may not need to build in the automated running portion.

I could also see this command expanding with a few flags to make it more powerful such as being able to define which things to check, whether to automatically fix things, and probably plenty of other options that we would think of.

cshannon avatar Jun 21 '24 11:06 cshannon

If run on a schedule, and similar to FATEs, there could be some simple metrics - maybe error count and run-time would be enough. A non-zero error count could then be used to prompt / alert to investigate further. The run time could allow for trending over time as a proxy for a performance measurement of the subsystems that are used in the check(s)

Metrics for timing sub-system checks would could add more fidelity if there are some discrete subsystems that are being used. (time to scan fate table, time to scan metadata table ...)

EdColeman avatar Jun 22 '24 12:06 EdColeman

I could also see this command expanding with a few flags to make it more powerful such as being able to define which things to check

I suspect that would be needed because some checks will take a bit of time to run. If someone wants to run a specific check then they may not want to wait on everything to run.

keith-turner avatar Jun 22 '24 18:06 keith-turner

This looks like a very interesting ticket. I would be interested in working on this, if no one else has started @cshannon @keith-turner Were either of you planning or started working on this?

kevinrr888 avatar Jul 26 '24 15:07 kevinrr888

I was not planning to anytime soon as i'm going to likely be working on gRPC stuff for a while so feel free to take a look

cshannon avatar Jul 26 '24 15:07 cshannon

This looks like a very interesting ticket. I would be interested in working on this, if no one else has started @cshannon @keith-turner Were either of you planning or started working on this?

No I was not, but I had some half baked thoughts about it that I will try to write up in case they are useful.

keith-turner avatar Jul 26 '24 15:07 keith-turner

That would be good, the more info the better

kevinrr888 avatar Jul 26 '24 15:07 kevinrr888

@kevinrr888 My suggestions would be:

  1. Don't duplicate other tooling, like metrics, log collection/analysis, etc.
  2. Stay under the Accumulo scope (don't try to identify problems with the whole system environment)
  3. Focus on detection, not automatic solutions: "Observe and Report"
  4. Use consistent terminology, and formatting
  5. Ensure output is machine-readable
  6. Chunk the reports into discrete and organized topics

ctubbsii avatar Jul 26 '24 16:07 ctubbsii

@kevinrr888 here are some of the things I was thinking about.

Each check could have a well defined name that allows only that check to be run and that a user could list all possible checks. Some checks may take a long time to run and if a user wants to run a specific check then they would not want to wait on everything.

Was also thinking that internally the checks could all implement a similar interface and declare dependencies on other checks, allowing the dependencies to be run first.

So maybe we could have something like the following to list checks that could be run.

accumulo admin check list
  Check Name        Description                                                                      Depends on

  system_config     Validate the system config stored in zookeeper
  root_metadata     Checks integrity of the root tablet metadata stored in zookeeper                  system_config 
  root_table        Scans all the tablet metadata stored in the root table and checks integrity      root_metadata 
  metadata_table    Scans all the tablet metadata stored in the metadata table and checks integrity  root_table
  system_files      Checks that files in system tablet metadata exists in DFS                        root_table
  user_files        Checks that files in user tablet metadata exists in DFS                          metadata_table

Then to run checks could do something like the following that starts running all checks following the dependency graph and does not run checks if their dependency check failed. Was also thinking the command could print high level status to stdout and details about failures to stderr (so thinking details about the failure would go to check.log in example below). Ideally what ends up in check.log would be some machine readable format like json, but not really sure how this would play out.

accumulo admin check run 2> check.log

  Check Name       Status

  system_config    OK
  root_table       OK
  root_metadata    OK
  metadata_table   FAILED
  system_files     OK
  user_files       SKIPPED_DEPENDENCY_FAILED

Could somehow support a regex pattern for selecting checks to run using their well known names.

accumulo admin check run --name_pattern "*.files"  2>check.log

  Check Name       Status

  system_files     OK
  user_files       OK

In the code set of checks will be static, so we could use an enum to specify the top level set of all checks. Then the list and run commands could just operate on the all the enums. The enum could offer method to get check objects and to get dependencies. Thinking the enum will make specifying the dependcy graph (which is also static) in code really easy.

// Not sure what to name thjs
enum Check {
   SYSTEM_CONFIG,
   ROOT_TABLE,
   ROOT_METADATA,
   METADATA_TABLE,
   SYSTEM_FILES,
   USER_FILES;

   // retunrs the list of other checks the check depends on
    List<Check> getDependicies();

    // returns a well defined interface for running a check
    CheckRunner getCheckRunner();
}

Not sure if this overall structure is workable. If we can get a structure for the command laid down along with a few initial checks, then we can start adding more checks as individual PRs.

keith-turner avatar Jul 26 '24 16:07 keith-turner

Thank you for this info As you suggested, I can work on getting the structure laid out with the existing checks now done through this new check cmd as an initial PR, then go from there

kevinrr888 avatar Jul 26 '24 16:07 kevinrr888

Closing this as completed by https://github.com/apache/accumulo/pull/4807. Essentially just replacing this ticket with https://github.com/apache/accumulo/issues/4892 so we don't have two tickets tracking the same thing. That ticket has some added info and links to this ticket as well.

kevinrr888 avatar Sep 16 '24 15:09 kevinrr888