Siyao Meng created HDDS-17367:
---------------------------------
Summary: MappedBufferManager cleanup returns quota of the other
chunk layout's cached mappings because the mapping map is static
Key: HDDS-17367
URL: https://issues.apache.org/jira/browse/HDDS-17367
Project: Apache Ozone
Issue Type: Bug
Reporter: Siyao Meng
Attachments: MC-3-per-instance-mapped-buffer-map.patch,
TestBugMC3CleanupCreditsOtherManager.java
h3. Mechanism
{{MappedBufferManager.mappedBuffers}} is a static field, while the quota
{{Semaphore}} belongs to each instance. Each datanode builds two instances, one
in {{FilePerChunkStrategy}} and one in {{FilePerBlockStrategy}}, each with the
full {{ozone.chunk.read.mapped.buffer.max.count}}. The cleanup in
{{MappedBufferManager.getQuota}} walks the shared map and returns the permit of
every cleared entry to the instance that ran it, including entries the other
instance created. The two strategies read different files, so the shared map
gives no cache reuse.
h3. Trigger
A datanode with mmap reads enabled
({{ozone.chunk.read.mapped.buffer.max.count}} > 0, default 0) that serves
memory mapped reads for both FILE_PER_CHUNK and FILE_PER_BLOCK containers.
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.
FILE_PER_BLOCK is the default container layout, so this needs a datanode that
still holds FILE_PER_CHUNK containers. A mapping of one strategy is cleared by
GC, and the other strategy runs out of quota and runs its cleanup.
h3. Impact
The strategy that ran the cleanup can hold more live mappings than its limit,
while the other can be left with neither quota nor mappings and serves its
reads from heap buffers until permits move back. The total over both instances
stays within twice {{max.count}}, which the datanode already allows today, so
the limit is not exceeded for the datanode as a whole. The effect is on
performance only. The static map is also shared by all datanodes in one JVM,
for example in mini cluster tests.
h3. Reproduction
PASS on unmodified source, which means the defect is present.
[^TestBugMC3CleanupCreditsOtherManager.java]
({{hadoop-hdds/container-service}}) builds two managers of capacity 1, one per
strategy, and reads through {{ChunkUtils.readData}}: after the chunk manager's
mapping is cleared, the block manager's cleanup reclaims its permit, the block
manager then holds 2 live mappings with limit 1, and the chunk manager has
quota 0 with no live mapping. With the patch the block manager's cleanup
reclaims nothing and the test fails ({{expected: 1 but was: 0}}).
h3. Patch
[^MC-3-per-instance-mapped-buffer-map.patch], against
5fed99dab3c3204cd7bf1516074969a38d5eb00d. It also applies to master at
76b5c6947a2.
{{mappedBuffers}} becomes an instance field, so each cleanup reclaims only the
entries of its own manager. Datanodes that serve one layout behave as before.
One shared manager per datanode would match the description of {{max.count}}
("allowed for a DN") but would halve the effective limit for datanodes that
serve both layouts, so it is left as a separate question.
Covered by {{TestMappedBufferManager#testCleanupReturnsOnlyOwnQuota}}, which
fails without the change ({{expected: <1> but was: <2>}}). 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]
