[
https://issues.apache.org/jira/browse/HDDS-17008?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Siyao Meng updated HDDS-17008:
------------------------------
Attachment: CR-15-reserve-before-container-visible-at-load.patch
> Duplicate container resolved by another volume's ContainerReader at datanode
> start can leave a reservation for the deleted copy
> -------------------------------------------------------------------------------------------------------------------------------
>
> Key: HDDS-17008
> URL: https://issues.apache.org/jira/browse/HDDS-17008
> Project: Apache Ozone
> Issue Type: Bug
> Reporter: Siyao Meng
> Priority: Minor
> Attachments: CR-15-reserve-before-container-visible-at-load.patch,
> TestBugCR15DuplicateSwapReservationRace.java
>
>
> h3. Mechanism
> At start, {{OzoneContainer.buildContainerSet}} runs one {{ContainerReader}}
> per volume on a shared {{ContainerSet}}.
> {{ContainerReader.verifyAndFixupContainerData}} adds a container to the set
> and only then reserves its space ({{commitSpace}}). Nothing orders these two
> steps against the other readers. If the same OPEN container is on another
> volume with a higher BCSID (or CLOSED), that volume's reader can find the
> first copy in the set, swap it out, delete it from disk and release its
> reservation in between. That release does nothing, because nothing is
> reserved yet. The first reader then reserves space for the copy that was just
> deleted.
> h3. Trigger
> The same container on two volumes at datanode start, the stale copy OPEN, and
> the stale copy's reader descheduled between {{addContainer}} and
> {{commitSpace}} for as long as the other reader needs for the whole duplicate
> handling, including removing the stale copy from disk. The precondition is
> rare: the disk balancer refuses to move OPEN containers, so it does not
> create such a duplicate. One possible route (argued, not run) is a volume
> that fails with an OPEN copy, the container being replicated as CLOSED to
> another volume of the same datanode, and the failed volume coming back at the
> next start. The window is narrow as well: a run of 400 duplicate pairs
> without timing help did not hit it.
> h3. Impact
> The volume of the deleted copy keeps one container size (minus the copy's
> used bytes) committed until the next restart, refuses that much new data, and
> reports it in the storage report. No data is lost.
> h3. Reproduction
> [^TestBugCR15DuplicateSwapReservationRace.java]
> ({{hadoop-hdds/container-service}}) uses two real volumes, the same container
> with BCSID 5 and 10, and real {{ContainerReader}}s. The only help is timing:
> a {{ContainerSet}} spy runs the real {{addContainer}} and then holds the
> stale copy's reader on a latch until the other reader has finished. Output:
> {{kept copy on fresh volume=true, stale copy dir exists=false, stale volume
> committed=5368709120, fresh volume committed=5368709120}}. A passing test
> means the defect is present.
> h3. Patch
> [^CR-15-reserve-before-container-visible-at-load.patch], against
> cce1ca28944886218132f01c0e271c1fd0700078. It also applies to master at
> 2536fe9cbc3. Released 2.1.0 and later have the same code.
> The reader reserves before it adds the container to the set, so a copy is
> reserved from the moment another reader can see it, and a {{finally}}
> releases the copy's own reservation when it is not kept (duplicate not
> swapped, or the add or duplicate resolution failed before the copy entered
> the set). A failure after the copy entered the set (removing the swapped out
> copy, or the container ID table update) leaves the reservations as they are
> today: the copy in the set unreserved, the swapped out copy still reserved.
> When the new copy wins, the existing copy is released as before. Covered by
> the new {{TestContainerReader.testDuplicateSwappedWhileFirstCopyIsLoading}},
> which fails without the change ({{expected: 0L but was: 5368698880L}}), and
> by a new check in the existing {{testMultipleContainerReader}} that each
> volume's committed bytes match the OPEN containers kept on it. With the
> change the reader, container, persistence and handler suites pass (332 tests)
> and checkstyle is clean.
> Not covered: {{ContainerSet.addContainer}} also adds the container ID to the
> volume's list after the map insert, and the swap removes it from there. A
> slightly shifted interleaving could leave the ID of the deleted copy in the
> stale volume's list (argued, not run).
> Found by code review of the datanode volume and per-disk RocksDB lifecycle
> paths, as part of the TLA+ verification effort under HDDS-15926, on commit
> cce1ca28944886218132f01c0e271c1fd0700078. 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]
