[
https://issues.apache.org/jira/browse/HDDS-17025?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Siyao Meng updated HDDS-17025:
------------------------------
Attachment: TestBugCR12NegativeImportSizeReservation.java
> Container push with a negative size reserves a negative amount on the
> receiving volume that is never released
> -------------------------------------------------------------------------------------------------------------
>
> Key: HDDS-17025
> URL: https://issues.apache.org/jira/browse/HDDS-17025
> Project: Apache Ozone
> Issue Type: Bug
> Reporter: Siyao Meng
> Priority: Minor
> Attachments:
> CR-12-reserve-default-space-for-negative-import-size.patch,
> TestBugCR12NegativeImportSizeReservation.java
>
>
> h3. Mechanism
> On the first message of a push, {{SendContainerRequestHandler.onNext}}
> reserves {{ContainerImporter.getSpaceToReserve(size)}}, which is {{2 * size}}
> for any size the sender gives. For a negative size this is a negative
> reservation: {{AvailableSpaceFilter}} accepts any volume for it and the
> volume's committed bytes go down. Both releases (in {{onError}} and
> {{onCompleted}}) only release a positive reservation, so the negative amount
> stays when the stream ends, whether the import succeeds, fails or the sender
> aborts.
> h3. Trigger
> The source sends {{getBytesUsed()}} of its container as {{size}}
> ({{GrpcContainerUploader.startUpload}}, since HDDS-12998 in 2.1.0). On 2.1.0
> to 2.1.2 that value is negative as soon as block deletion drives the in
> memory counter below zero, the case HDDS-15110 addresses; its in memory clamp
> is only in 2.2.0 and later. On 2.2.x and master the persisted
> {{CONTAINER_BYTES_USED}} counter is still written as {{bytes -
> releasedBytes}} without a lower bound in
> {{KeyValueContainerData.updateAndCommitDBCounters}} and loaded back without
> one, so after a restart the source sends a negative size again. HDDS-15110's
> {{TestContainerReplication.pushSucceedsWhenSourceBytesUsedIsNegative}} pushes
> such a size and only checks that the import succeeds. I did not drive a real
> counter negative.
> h3. Impact
> Each such push lowers the receiving volume's committed bytes by {{2 *
> |size|}} until restart. The volume then reports less committed space than it
> holds, also to SCM, and admits containers and imports it has no room for
> (argued from {{AvailableSpaceFilter}}, not run here).
> h3. Reproduction
> [^TestBugCR12NegativeImportSizeReservation.java]
> ({{hadoop-hdds/container-service}}) feeds two streams with {{size =
> -1234567890}} into {{SendContainerRequestHandler}}, the observer the
> replication service hands each message to, with a real {{ContainerImporter}},
> {{MutableVolumeSet}} and volume choosing policy. After the first stream
> (failed import) {{committed=-2469135780}}, after the second (aborted by the
> sender) {{committed=-4938271560 reported to SCM=-4938271560}}. A passing test
> means the defect is present. The negative size is supplied directly, not
> produced by a real source.
> h3. Patch
> [^CR-12-reserve-default-space-for-negative-import-size.patch], against
> cce1ca28944886218132f01c0e271c1fd0700078. It also applies to master at
> 2536fe9cbc3. Released 2.1.0 to 2.2.1 are affected.
> {{ContainerImporter.getSpaceToReserve}} uses the given size only when it is
> not negative and {{2 * size}} does not overflow, and otherwise reserves the
> default replication space, as for a source that sends no size. The
> reservation is then never negative and the existing releases free it.
> Rejecting the push instead would make the container permanently unreplicable
> from that source, since its counter stays wrong until repaired, so the
> receiver falls back to the reservation it already uses when no size is sent.
> A push with a negative size can now be refused with {{DISK_OUT_OF_SPACE}} on
> a volume without the default free space, where it is always accepted today.
> Covered by the new
> {{TestSendContainerRequestHandler.testInvalidSizeReservesDefaultSpace}} for a
> negative size and for {{Long.MAX_VALUE}}, which fails without the change (for
> example {{expected: <10737418240> but was: <-2469135780>}}). With it the
> replication package passes (133 tests) and checkstyle is clean. Clamping the
> persisted counter on the source is a separate change and is not included.
> 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]
