Siyao Meng created HDDS-17368:
---------------------------------
Summary: 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
Reporter: Siyao Meng
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]
