Open kevinrr888 opened 1 month ago
@kevinrr888 these changes look good. I was running the changes locally and I noticed at the top level the
accumulo check-server-config
command exists. Was wondering if that should be rolled into this check command, if it should could add that to the checklist on #4892.
Thanks, I had missed that. Added to #4892. Also noticed accumulo check-compaction-config
, which I also added to the checklist. Did you think this should be included as well or no?
Changes in 11f1f7643e51c7f0851a0220b4892e074ff86594:
System.out
-> log.trace/warn
to avoid flooding output with unnecessary/detailed info. The most important info (e.g., output of admin check list
command, and the final run status table from the admin check run
command) is still printed to stdout. Problems found are now logged at warn instead of stdout. Detailed, non-error info logged at trace.Check.TABLE_LOCKS
which ensures that table and namespace locks are valid and are associated with a FATE op. This depends on SYSTEM_CONFIG, which seemed like the most fitting location for it in the dependency tree.assertNoOtherChecksRan()
in AdminCheckIT
which ensures only the expected checks ranMetadataCheckRunner
code improved to only fetch required columns when scanning, object creation moved outside of a loopLet me know how these changes look. Something still left TODO is tests for failing checks. Tests should be added to ensure everything that is supposed to be checked correctly fails when it is expected to. I imagine this will take quite a bit of time to do and the changes are already pretty large, so thinking it might be best to leave for follow-on.
This PR:
checkTablets
and the fate check for dangling locks) into the appropriate newadmin check
commandThere are still quite a few checks that need to be added (mentioned in https://github.com/apache/accumulo/issues/4687) and probably more. This is a first/starting PR for these checks. More checks will be added in follow-ons. Something else still left todo are tests for these checks for FAILING cases.
Part of #4892