ozone
ozone copied to clipboard
HDDS-11290. Container scanner should keep scanning after non-fatal errors
What changes were proposed in this pull request?
Motivation
In order to build the merkle tree of all data in the container, the scanner should not exit after the first issue it encounters like it does currently. The scanner should track and return all errors that it sees, and only stop the scan on fatal errors that prevent further scanning of the container, like DB access errors.
This PR is a pre-requisite to HDDS-10374. It does not actually generate a merkle tree during the scan and is also not testing this functionality. It sets up HDDS-10374 to be an easy drop in to the scanner which will allow the focus of that change to be testing of merkle tree generation.
Primary Changes
Previously ScanResult
was an object that encapsulated a singe error. This was the first error the scanner saw which would abort the scan. This change decouples the ScanResult
from the errors, which are now represented by a list ofContainerScanError
s in the ScanResult
.
-
ScanResult
is a general interface that can represent a data or metadata scan. It can be logged by entities like theContainerLogger
which do not care where the unhealthy result came from. -
MetadataScanResult
is aScanResult
implementation produced from a container metadata scan.- This scan will not produce a merkle tree since it does not check data.
-
DataScanResult
extendsMetadataScanResult
by adding a merkle tree representing the data that was scanned.- All data scans begin with a metadata scan, and then proceed to scan the data only if the metadata scan succeeds.
- The datanode application log will only log the first error encountered unless debug logging is enabled, then all errors will be logged.
- The container log does not have this option to keep it from rolling off.
Secondary Changes
- General cleanup of
KeyValueContainerCheck
internals were done since this PR already required invasive changes in this area. - Fixed a bug from #5485/HDDS-9005 where the DB was being used to check for container deletion during a scan instead of the container state in memory.
- This would fail to detect a schema v2 container which is completely lost but not actually deleted. The container would remain in the datanode's memory as healthy.
- Since the memory state is updated first and our container object will still be valid even after being removed from the ContainerSet, we can use the
DELETED
state as the deletion check which is both safer and simpler.
- When a container is deleted during a scan, this is indicated with a different field in the
ScanResult
. It is no longer considered an error and will not cause theScanResult
to be unhealthy. - Container log checks are more stringent now making sure that the error logged is actually for the expected container ID.
- Since the tests re-use the same cluster, one could have passed if a previous test logged the failure type this test was expecting.
Notes to Reviewers
It is probably best to review the scan flow end-to-end instead in addition to just viewing the diff.
What is the link to the Apache JIRA
HDDS-11290
How was this patch tested?
The scanner has a lot of existing tests that should all pass ensuring no regressions:
-
TestKeyValueContainerCheck
-
TestKeyValueHandlerWithUnhealthyContainer
-
Test{Background,OnDemand}Container{Data,Metadata}Scanner
-
Test{Background,OnDemand}Container{Data,Metadata}ScannerIntegration
New tests added for detecting multiple errors will be added in HDDS-11471 since this already a large PR.