[
https://issues.apache.org/jira/browse/HDDS-17368?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Siyao Meng updated HDDS-17368:
------------------------------
Attachment: MC-1-return-quota-of-replaced-cleared-mapped-buffer.patch
> MappedBufferManager loses one mmap quota permit each time a chunk range is
> re-read after GC cleared its cached mapping
> ----------------------------------------------------------------------------------------------------------------------
>
> Key: HDDS-17368
> URL: https://issues.apache.org/jira/browse/HDDS-17368
> Project: Apache Ozone
> Issue Type: Bug
> Affects Versions: 2.0.0, 2.1.0, 2.2.0, 2.1.1, 2.1.2, 2.2.1
> Reporter: Siyao Meng
> Priority: Minor
> Attachments:
> MC-1-return-quota-of-replaced-cleared-mapped-buffer.patch,
> TestBugMC1ReplacedClearedEntryLeaksQuota.java
>
>
> h3. Mechanism
> {{MappedBufferManager.computeIfAbsent}} treats an entry whose
> {{WeakReference}} has been cleared by GC like a missing entry: it maps the
> range again and stores the new reference with an unconditional
> {{mappedBuffers.put}}, discarding the cleared reference. The cleared entry
> still accounted for the permit taken when it was mapped, and only the
> asynchronous cleanup in {{MappedBufferManager.getQuota}} returns such
> permits, for references it removes itself with {{remove(key, ref)}}. After
> the {{put}} the cleared reference is no longer in the map, so its permit is
> never returned.
> h3. Trigger
> # A datanode with mmap reads enabled
> ({{ozone.chunk.read.mapped.buffer.max.count}} > 0, default 0) serves a memory
> mapped read of a chunk range, and the response is released. Reads are memory
> mapped only when the read buffer capacity is above
> {{ozone.chunk.read.mapped.buffer.threshold}} (32KB), so data written with the
> default 16KB per checksum is never mapped; chunks written with checksum type
> NONE or with more than 32KB per checksum, and reads into a single buffer, are.
> # GC clears the cached mapping. The entry stays in the map, because the
> cleanup only runs after {{getQuota}} fails.
> # The same range (same file, offset and length) is read again.
> For such data this is the ordinary pattern of repeated reads; no fault is
> needed.
> h3. Impact
> Each such re-read permanently lowers the mmap quota of the manager by one,
> until the datanode restarts. Each lost permit replaces one cache entry with
> another, so at least one entry is always left for the cleanup to reclaim, and
> repeated re-reads can wear the quota down to 1 of {{max.count}}. Reads that
> need more pieces than the remaining quota fall back to heap buffers. The
> reproduction below ended at 1 of 4. Data stays correct; the effect is that
> the configured mmap read capacity erodes.
> h3. Reproduction
> PASS on unmodified source, which means the defect is present.
> [^TestBugMC1ReplacedClearedEntryLeaksQuota.java]
> ({{hadoop-hdds/container-service}}) reads one range through
> {{ChunkUtils.readData}} with a manager of capacity 4, lets GC clear the
> mapping and re-reads it three times. Output: quota 3, 2, 1, 0 after the
> re-reads, quota 1 of 4 after the cleanup with no live mapping, and a two
> piece read then returns a heap buffer. With the patch the quota stays at 3
> and the test fails at its first re-read assertion.
> h3. Patch
> [^MC-1-return-quota-of-replaced-cleared-mapped-buffer.patch], against
> 5fed99dab3c3204cd7bf1516074969a38d5eb00d. It also applies to master at
> 76b5c6947a2.
> When the key still holds a cleared reference, the new reference is installed
> with {{mappedBuffers.replace(key, refer, newRef)}} and the cleared entry's
> permit is returned if the replace succeeds; otherwise the code falls back to
> {{put}} as before. This follows the {{remove(key, ref)}} approach of
> HDDS-15357: either the cleanup removes the cleared reference and returns its
> permit, or the replace does, never both. A failed mapping leaves the cleared
> entry to the cleanup as before.
> Covered by {{TestMappedBufferManager#testRemapClearedBufferReturnsItsQuota}},
> which fails without the change ({{expected: <1> but was: <0>}}). With it,
> {{TestMappedBufferManager}}, {{TestChunkUtils}},
> {{TestFilePerBlockStrategy}}, {{TestFilePerChunkStrategy}},
> {{TestChunkManagerDummyImpl}} and the {{TestKeyValueHandler}} suites pass
> (159 tests), checkstyle is clean.
> Found by TLA+ model checking and code review of the datanode memory mapped
> chunk read path under HDDS-15926, on commit
> 5fed99dab3c3204cd7bf1516074969a38d5eb00d. 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
(v10.3.26#10030026)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
