Siyao Meng created HDDS-16815:
---------------------------------
Summary: Container data scanner thread ends when a chunk's stored
bytes per checksum is invalid, and a large stored value sizes the scan's read
buffer
Key: HDDS-16815
URL: https://issues.apache.org/jira/browse/HDDS-16815
Project: Apache Ozone
Issue Type: Bug
Reporter: Siyao Meng
Attachments:
CR-5-report-invalid-stored-bytes-per-checksum-in-container-scanner.patch,
TestBugCR5ScannerStoredBytesPerChecksum.java
h3. Mechanism
The container data scan reads the bytes per checksum stored in a chunk's
checksum metadata and trusts it. {{KeyValueContainerCheck.scanBlock}} passes it
to {{BUFFER_POOL.getBuffer(bytesPerChecksum)}} to size the read buffer, and
{{verifyChecksum}} uses it to walk the chunk and, for the last interval, as the
divisor in {{chunk.getLen() % bytesPerChecksum}}.
For a chunk whose stored checksum type is not NONE and whose stored bytes per
checksum is zero or negative, the scan throws an unchecked exception: zero
gives an {{IndexOutOfBoundsException}} fetching the first computed checksum
from an empty list (or an {{ArithmeticException}} divide by zero, depending on
the chunk), and a negative value gives an {{IllegalArgumentException}}
allocating the buffer. The data scan runs on CLOSED, QUASI_CLOSED and UNHEALTHY
containers, so the exception can be thrown for any such container.
The background data scanner does not survive this.
{{AbstractBackgroundContainerScanner.scanContainers}} catches only
{{InterruptedException}} and {{IOException}} per container, and {{run()}}
catches the escaping exception, logs "exiting because of exception" and
returns. There is one background data scanner thread per volume, and nothing
restarts it, so one such chunk on a volume ends that volume's data scanning
until the datanode restarts, and the scanner's metrics are unregistered. The
on-demand scanner catches the exception in the scan future but leaves the
container in {{containerRescheduleCheckSet}}, so later on-demand scans of that
container are never queued.
A large stored bytes per checksum (up to {{Integer.MAX_VALUE}}) is used to
reserve a direct read buffer of that size, so one such chunk makes each scan of
its container try to reserve up to about 2 GB of direct memory.
h3. Trigger
A closed (or quasi-closed or unhealthy) container holds a chunk whose stored
checksum metadata has a checksum type other than NONE and a bytes per checksum
that is zero, negative, or far larger than the chunk. A data scan of that
container (background or on demand) reads the stored value.
h3. Impact
* A stored bytes per checksum of zero or less: the data scan throws an
unchecked exception. The volume's background data scanner thread ends and does
not restart until the datanode restarts, and the on-demand scanner stops
scanning that container. Other volumes' scanner threads and the metadata
scanner are not affected.
* A very large stored bytes per checksum: each data scan of the container
reserves a direct buffer of that size, which can exhaust the datanode's direct
memory.
h3. Reproduction
PASS on the unmodified pin, deterministic.
[^TestBugCR5ScannerStoredBytesPerChecksum.java]
({{hadoop-hdds/container-service}}) builds real closed {{KeyValueContainer}}s
on disk and drives the real {{KeyValueContainerCheck}},
{{BackgroundContainerDataScanner}} and {{OnDemandContainerScanner}}; only the
{{ContainerController}} is a mock. The stored value is put in place by
rewriting the block metadata in the container DB, the way the existing scanner
tests set up their containers. Five cases: the data scan throws for a stored
value of 0 and of -1; the background scanner thread ends although the volume
has not failed and the scanner was not shut down; the on-demand scanner never
schedules the container again after the scan fails; a stored value of
{{Integer.MAX_VALUE}} makes the scan reserve over 1 GB of direct memory. A
passing run means the defect is present. With the patch all five fail (4
failures, 1 error): the two scan cases and the on-demand case fail on their
assertion because the scan returns a result instead of throwing and the
container is scheduled again, the buffer case fails on its assertion because
the read buffer stays small, and the background scanner case ends with a
timeout waiting for the thread to end, because the thread stays alive.
h3. Patch
[^CR-5-report-invalid-stored-bytes-per-checksum-in-container-scanner.patch],
against 1e528b97aed3eef1be6846f1f0767c1ef98a883a. It also applies to master at
8ca44914771.
{{KeyValueContainerCheck.scanBlock}} now treats a stored bytes per checksum of
zero or less as a chunk it cannot verify: it records a {{CORRUPT_CHUNK}} scan
error and adds the chunk to the merkle tree as not matching, instead of reading
it, and keeps scanning the rest of the container. A new {{readBufferSize}}
helper caps the direct read buffer, so an interval larger than
{{OZONE_SCM_CHUNK_MAX_SIZE}} (the largest chunk a datanode reads at once) does
not allocate a buffer larger than the chunk file, and the last-interval
{{buffer.limit(...)}} is clamped to the buffer capacity.
{{AbstractBackgroundContainerScanner.scanContainers}} and
{{OnDemandContainerScanner.performOnDemandScan}} now also catch
{{RuntimeException}} per container, so an unexpected failure scanning one
container is logged and skipped instead of ending the background scanner thread
or leaving the container scheduled. {{InterruptedException}} handling is
unchanged, so shutdown still works.
A container that already holds such a chunk is, after the patch, scanned to
completion and the chunk is reported as corrupt, so the container is marked
UNHEALTHY. The chunk's data on disk is not touched. Containers whose chunks all
have a valid stored bytes per checksum are unaffected.
The patch covers the container data scan only: other readers of a chunk with
such stored metadata (chunk and block reads, reconciliation, file checksum
computation) still fail for that read or task (from reading the code, not run
with the patch).
Covered by new tests in existing suites:
{{TestKeyValueContainerCheck.testInvalidBytesPerChecksumReportedAsCorruptChunk}}
and {{testBytesPerChecksumLargerThanChunkFile}},
{{TestBackgroundContainerDataScanner.testUnexpectedScanFailureDoesNotStopIteration}},
and
{{TestOnDemandContainerScanner.testContainerScannedAgainAfterUnexpectedScanFailure}}.
Without the patch the first, third and fourth of these fail with the unchecked
exception escaping the scan, and the second does not compile because it also
checks the new {{readBufferSize}} helper; with it the unit suites of the
{{keyvalue}}, {{ozoneimpl}} and {{checksum}} packages in
{{hadoop-hdds/container-service}} pass (931 tests, 10 skipped by the suites),
the scanner integration suites pass (21 tests), checkstyle is clean, and the
reproduction's five cases fail as described above. Not run on a released
version.
Found by code review of the container reconciliation paths in the datanode and
SCM, as part of the TLA+ verification effort under HDDS-15926, on commit
1e528b97aed3eef1be6846f1f0767c1ef98a883a. Checked against HDDS issues and
apache/ozone pull requests for duplicates before filing. The attached patch is
a proposal for review. Generated with Specula (Claude Opus 5.5).
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
