HDDS-13286. Fail stream write when the volume is full. #8644

sumitagrawl · 2025-06-17T06:57:35Z

What changes were proposed in this pull request?

When volume if full, means no space available for data write, need to make write failure as Disk out of space.

This needs to be done for write operation during stream write.

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-13286

How was this patch tested?

Unit test is added

Copilot

Pull Request Overview

This PR introduces a check for volume full conditions during stream write operations, ensuring that a write failure is triggered with a proper error message and exception when disk space is exhausted.

Added a new method assertSpaceAvailability in the base stream data channel class
Updated the KeyValueStreamDataChannel to perform a space availability check before writing
Introduced unit tests to validate the disk full failure scenario

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File	Description
TestKeyValueStreamDataChannel.java	Added unit tests to simulate and assert behavior when the volume is full
StreamDataChannelBase.java	Implemented assertSpaceAvailability method to throw StorageContainerException on insufficient space
KeyValueStreamDataChannel.java	Invoked assertSpaceAvailability before writing to the channel

...test/java/org/apache/hadoop/ozone/container/keyvalue/impl/TestKeyValueStreamDataChannel.java

szetszwo

@sumitagrawl , thanks for working on this! Please see the comments inlined.

szetszwo · 2025-06-17T23:07:45Z

...src/main/java/org/apache/hadoop/ozone/container/keyvalue/impl/KeyValueStreamDataChannel.java

      throws IOException {
    getMetrics().incContainerOpsMetrics(getType());
    assertOpen();
+    assertSpaceAvailability();


It should account for referenceCounted.get().remaining().

szetszwo · 2025-06-17T23:08:26Z

...ice/src/main/java/org/apache/hadoop/ozone/container/keyvalue/impl/StreamDataChannelBase.java

+    if (containerData.getVolume().isVolumeFull()) {
+      throw new StorageContainerException("write failed for container " + containerData.getContainerID()
+          + " due to volume " + containerData.getVolume().getStorageID() + " out of space "
+          + containerData.getVolume().getCurrentUsage(), DISK_OUT_OF_SPACE);


We should print also the reserved space and the required space; otherwise, the message will be confusing since it will show space available.

Please move method to HddsVolume as assertSpaceForWrite.

Accept required size in parameter, as @szetszwo mentioned.

In the implementation, add local variables for the values (currentUsage, minFreeSpace), and use them in both calculation and log message. Do not call boolean isVolumeFull(), because then the message needs to obtain the same values again.

@adoroszlai others are done, but unable to move to HddsVolume as log message needs containerId which is not present as part of HddsVolume. For log completeness, it should be done at outer layer, or need encapsulate exception again adding more details at outer layer.

Pass container-specific part of the message as Supplier<String>.

Also, please rename to assertSpaceForWrite for clarity and simplicity, and rename parameter remaining (which is in terms of the buffer) to required.

szetszwo

@sumitagrawl , thanks for the update! Please see the comment inlined.

...ice/src/main/java/org/apache/hadoop/ozone/container/keyvalue/impl/StreamDataChannelBase.java

adoroszlai · 2025-06-19T06:05:12Z

On a related note: I don't think it's good practice to have two or more pull requests in parallel with similar code change handling different case (stream vs. non-stream write path).

You inevitably get duplication in one or more areas:

review effort
your effort (e.g. to tweak the exception message in HDDS-12151. Fail write when volume is full considering min free space #8642 based on comment made here)
code
CI

Depending on the size of the changes, please either:

create one PR at a time, wait for it to be merged, then your next PR starts in a better shape, OR
if the patches are small and changes are related, submit a single combined PR

szetszwo · 2025-06-19T19:05:34Z

...r-service/src/main/java/org/apache/hadoop/ozone/container/common/helpers/ContainerUtils.java

+    final SpaceUsageSource currentUsage = volume.getCurrentUsage();
+    final long spared = volume.getFreeSpaceToSpare(currentUsage.getCapacity());
+
+    if (currentUsage.getAvailable() - spared <= sizeRequested) {


@sumitagrawl , It should be < since == means that the space is enough. Sorry that I did not see it earlier.

szetszwo

+1 the change looks good.

szetszwo · 2025-06-20T17:53:57Z

@sumitagrawl , thanks for working on this!

@adoroszlai , thanks for reviewing this!

…239-container-reconciliation Commits: 9 05567e6 HDDS-13317. Table should support empty array/String (apache#8676) 803fa31 HDDS-13313. Bump maven-javadoc-plugin to 3.11.2 (apache#8672) 774b3ee HDDS-13315. Bump log4j2 to 2.25.0 (apache#8670) b86fad8 HDDS-13316. Bump commons-text to 1.13.1 (apache#8669) c043be7 HDDS-13312. Bump maven-patch-plugin to 1.3 (apache#8673) db6ed84 HDDS-13294. Remove the Table.close() method. (apache#8658) e1365a7 HDDS-13286. Fail stream write when the volume is full. (apache#8644) af3a0f0 HDDS-13307. integration (flaky) fails with all tests passing (apache#8667) d207c18 HDDS-13301. Fix TestSchemaOneBackwardsCompatibility (apache#8666) Conflicts: hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/helpers/ContainerUtils.java

…pache#8644) (cherry picked from commit e1365a7) Conflicts: ContainerUtils: Had to use volume.getVolumeInfo() for getting SpaceUsageSource. Added volume.getVolumeInfo().isPresent() check. Used ReferenceCountedObject.wrap(putBlockBuf, () -> {}, () -> {}) instead of ReferenceCountedObject.wrap(putBlockBuf) as the single arg method is not present in this version. hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/helpers/ContainerUtils.java hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/impl/KeyValueStreamDataChannel.java hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/impl/StreamDataChannelBase.java hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/impl/TestKeyValueStreamDataChannel.java

sumitagrawl and others added 5 commits May 20, 2025 11:00

HDDS-12926. remove *.tmp.* exclusion in DU

c1d94aa

Merge branch 'apache:master' into master

0eabb6e

Merge branch 'apache:master' into master

d39a809

Merge branch 'apache:master' into master

2f8040c

HDDS-13286. write fail for volume full during stream write

cec3094

sumitagrawl requested review from ChenSammi, Copilot, siddhantsangwan and szetszwo June 17, 2025 06:57

Copilot AI reviewed Jun 17, 2025

View reviewed changes

...test/java/org/apache/hadoop/ozone/container/keyvalue/impl/TestKeyValueStreamDataChannel.java Show resolved Hide resolved

sumitagrawl marked this pull request as ready for review June 17, 2025 10:27

szetszwo reviewed Jun 17, 2025

View reviewed changes

review fix

d64d590

sumitagrawl requested a review from szetszwo June 18, 2025 08:36

fix test case

fc903d2

sumitagrawl requested a review from adoroszlai June 18, 2025 08:53

adoroszlai mentioned this pull request Jun 18, 2025

HDDS-12151. Fail write when volume is full considering min free space #8642

Merged

szetszwo reviewed Jun 18, 2025

View reviewed changes

...ice/src/main/java/org/apache/hadoop/ozone/container/keyvalue/impl/StreamDataChannelBase.java Outdated Show resolved Hide resolved

fix review comment

39e82b5

adoroszlai reviewed Jun 19, 2025

View reviewed changes

...ice/src/main/java/org/apache/hadoop/ozone/container/keyvalue/impl/StreamDataChannelBase.java Show resolved Hide resolved

fix review comment

0e6d464

sumitagrawl requested review from adoroszlai and szetszwo June 19, 2025 13:00

szetszwo reviewed Jun 19, 2025

View reviewed changes

review fix

1e259ac

sumitagrawl requested a review from szetszwo June 20, 2025 04:44

Merge remote-tracking branch 'origin/master' into HDDS-13286

52350aa

adoroszlai marked this pull request as draft June 20, 2025 10:11

adoroszlai marked this pull request as ready for review June 20, 2025 11:43

adoroszlai approved these changes Jun 20, 2025

View reviewed changes

szetszwo approved these changes Jun 20, 2025

View reviewed changes

szetszwo merged commit e1365a7 into apache:master Jun 20, 2025
52 checks passed

szetszwo changed the title ~~HDDS-13286. write fail for volume full during stream write~~ HDDS-13286. Fail stream write when the volume is full. Jun 20, 2025

HDDS-13286. Fail stream write when the volume is full. #8644

HDDS-13286. Fail stream write when the volume is full. #8644

Uh oh!

Conversation

sumitagrawl commented Jun 17, 2025

What changes were proposed in this pull request?

What is the link to the Apache JIRA

How was this patch tested?

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

szetszwo left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

szetszwo left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

adoroszlai commented Jun 19, 2025

Uh oh!

szetszwo Jun 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

szetszwo left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

szetszwo commented Jun 20, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

szetszwo Jun 19, 2025 •

edited

Loading