kevinrr888 opened a new pull request, #5348:
URL: https://github.com/apache/accumulo/pull/5348
New checks for `admin check` command
- Implemented SYSTEM_CONFIG check:
- Checks ZooKeeper locks for Accumulo server processes
- Checks ZooKeeper table nodes
- Checks that the WAL metadata in ZooKeeper is valid
- Added new SERVER_CONFIG check:
- Checks that all configured properties are valid
- Checks that required properties are present in the config
- Added new tests in `AdminCheckIT` for SYSTEM_CONFIG and SERVER_CONFIG.
Added failing test cases for all checks.
- Updated description for #4892. This describes all the checks thus far for
the new `accumulo admin check` command. See this for the complete functionality
of the command.
- Considered adding check to MetadataConstraints to ensure tablets reference
files that actually exist (i.e., in
`MetadataConstraints.validateDataFileMetadata`, check that `stf.getPath()`
exists in HDFS). This would make sure the file exists before the mutation is
ever written and would also check that it exists when we run the admin check
command for metadata (root metadata, root table, and metadata table): see
`MetadataCheckRunner.checkColumns()`. However, this led to a ComprehensiveIT
test failure, so I assume there's a reason we're not checking its existance in
MetadataConstraints. Would this be something we want? Or is there a reason we
are not checking for this?
- Deleted CheckServerConfig (run via `accumulo check-server-config`) as the
new `accumulo admin check run SERVER_CONFIG` will inherently do the same
checks. Maybe we don't want to completely get rid of this check yet. Maybe it
would be better to have the check just output that the check has moved under
`accumulo admin check`?
- There were a couple other existing check commands that I originally
considered moving under this new admin command (CheckAccumuloProperties and
CheckCompactionConfig), however, their usage is specifically to check a file
irrespective of any running Accumulo instance, so I did not feel it would make
sense to move these under the new admin command.
This PR along with #4957 closes #4892
--
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]
