[
https://issues.apache.org/jira/browse/HDDS-17369?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Meng-Shuan Tsai reassigned HDDS-17369:
--------------------------------------
Assignee: Meng-Shuan Tsai
> Mapped chunk read that fails on a later piece releases the quota of earlier
> pieces a second time, raising the effective mmap limit until restart
> ------------------------------------------------------------------------------------------------------------------------------------------------
>
> Key: HDDS-17369
> URL: https://issues.apache.org/jira/browse/HDDS-17369
> 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
> Assignee: Meng-Shuan Tsai
> Priority: Major
> Attachments:
> MC-2-release-only-unsettled-quota-on-failed-mapped-read.patch
>
>
> h3. Mechanism
> The mapped branch of {{ChunkUtils.readData}} reserves one permit per piece
> with {{MappedBufferManager.getQuota(bufferNum)}}. Each piece then settles its
> own permit in {{MappedBufferManager.computeIfAbsent}}: a cache hit releases
> it at once, a miss hands it to the new cache entry, which the cleanup in
> {{getQuota}} reclaims after GC has cleared it. When a later piece fails to
> map, the failure handler calls {{releaseQuota(bufferNum)}} and so releases
> the permits of the pieces that already settled a second time. The quota
> {{Semaphore}} has no ceiling, so the extra permits stay.
> h3. Trigger
> # Mapped reads are enabled ({{ozone.chunk.read.mapped.buffer.max.count}} > 0,
> default 0).
> # A read is served from the mapped path, which needs a buffer capacity above
> {{ozone.chunk.read.mapped.buffer.threshold}} (32KB). The default 16KB per
> checksum does not reach it.
> # The read is split into several pieces, and a later piece fails to map after
> an earlier one has settled, for example because the block file on disk is
> shorter than the chunk being read, or because the map call fails.
> h3. Impact
> * Each such failure raises the effective mmap limit of the manager by the
> number of pieces that settled before it, and the limit stays raised until the
> datanode restarts. The configured value of
> {{ozone.chunk.read.mapped.buffer.max.count}} does not change.
> * More mappings can then be live at the same time than {{max.count}} allows:
> at once for pieces that were cache hits, and for newly mapped pieces after GC
> and the cleanup have reclaimed their entries.
> * That limit was added in HDDS-10488 to keep the datanode from running out of
> map areas. What running out would do here is argued from that issue, not run.
> h3. Reproduction
> No standalone reproduction is attached. The regression test in the patch,
> {{TestChunkUtils#testFailedMappedReadReleasesOnlyUnsettledQuota}}, covers it:
> it calls {{ChunkUtils.readData}} directly with a buffer capacity of 64KB,
> above the threshold, on a block file that holds one piece, and checks the
> quota after a failed two piece read in three cases: first piece a cache hit,
> no piece mapped, first piece newly mapped. On unmodified source it fails in
> the cache hit case with {{expected: <3> but was: <4>}}.
> h3. Patch
> [^MC-2-release-only-unsettled-quota-on-failed-mapped-read.patch], against
> 5fed99dab3c3204cd7bf1516074969a38d5eb00d. It also applies to master at
> 76b5c6947a2.
> The list of mapped pieces is declared before the {{try}}, and the failure
> handler releases only {{bufferNum - buffers.size()}} permits. A piece is
> added to that list only after {{computeIfAbsent}} returned it, that is after
> it settled its permit. A failure before the first piece still releases all
> permits, a failure after the last piece releases none, and successful reads
> are unchanged.
> With the patch {{TestMappedBufferManager}} (1 test), {{TestChunkUtils}} (7),
> {{TestFilePerBlockStrategy}} (21), {{TestFilePerChunkStrategy}} (17),
> {{TestChunkManagerDummyImpl}} (2), {{TestKeyValueHandler}} (43) and
> {{TestKeyValueHandlerWithUnhealthyContainer}} (68) pass, 159 tests in total,
> and checkstyle on {{hadoop-hdds/container-service}} is clean. It does not
> conflict with the patches for the two related quota accounting issues in
> {{MappedBufferManager}} found in the same review (HDDS-17368, a replaced
> cleared cache entry, and HDDS-17367, the cache map shared by both chunk
> layouts).
> The defect is in every release from 2.0.0 to 2.2.1. It came in with
> HDDS-10488, which added the mapped read path.
> 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 PRs for duplicates before filing. Related but not duplicates:
> HDDS-10488 (introduced the mapped read path and its limit), HDDS-15357 (other
> quota races in {{MappedBufferManager}}). 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]
