[
https://issues.apache.org/jira/browse/HDDS-16815?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Siyao Meng updated HDDS-16815:
------------------------------
Attachment:
CR-5-report-invalid-stored-bytes-per-checksum-in-container-scanner.patch
> 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
> Priority: Major
> 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]
