-
Notifications
You must be signed in to change notification settings - Fork 588
HDDS-12151. Fail write when volume is full considering min free space #8642
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR addresses the handling of write operations when a volume is full by returning a DISK_OUT_OF_SPACE error. Key changes include:
- Updating the test to expect DISK_OUT_OF_SPACE instead of SUCCESS.
- Modifying HddsDispatcher to wrap container close actions in a try-catch and to throw a StorageContainerException for write operations when the volume is full.
- Adding a new import for DISK_OUT_OF_SPACE and updating the logging for full volume cases.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| TestHddsDispatcher.java | Updated test assertion to expect DISK_OUT_OF_SPACE for write failures. |
| HddsDispatcher.java | Refactored close container action logic to handle exceptions and throw an error when volume is full. |
Comments suppressed due to low confidence (1)
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/impl/HddsDispatcher.java:625
- The error message indicates 'Container creation failed' even though this branch is handling write operations. Consider updating the message to clearly reflect that the write operation failed due to insufficient disk space.
throw new StorageContainerException("Container creation failed, due to volume out of space", DISK_OUT_OF_SPACE);
...iner-service/src/main/java/org/apache/hadoop/ozone/container/common/impl/HddsDispatcher.java
Outdated
Show resolved
Hide resolved
...iner-service/src/main/java/org/apache/hadoop/ozone/container/common/impl/HddsDispatcher.java
Outdated
Show resolved
Hide resolved
...iner-service/src/main/java/org/apache/hadoop/ozone/container/common/impl/HddsDispatcher.java
Outdated
Show resolved
Hide resolved
siddhantsangwan
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good other than a minor comment.
| ContainerUtils.assertSpaceAvailability(containerID, container.getContainerData().getVolume(), 0); | ||
| } | ||
| } catch (StorageContainerException e) { | ||
| LOG.warn(e.getMessage()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should add the actual path of the volume to the message being constructed in assertSpaceAvailability to help in figuring out which volume is full. Right now it just has the volume UUID.
|
Thanks @sumitagrawl , the last patch LGTM, +1. |
siddhantsangwan
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks for fixing this.
* master: (90 commits) HDDS-12468. Check for space availability for all dns while container creation in pipeline (apache#8663) HDDS-12151. Fail write when volume is full considering min free space (apache#8642) HDDS-13251. Update Byteman usage README license (apache#8700) HDDS-13251. Support dynamic Byteman scripts via bmsubmit in ozonesecure-ha (apache#8654) HDDS-12070. Bump Ratis to 3.2.0 (apache#8689) HDDS-12984. Use InodeID to identify the SST files inside the tarball. (apache#8477) HDDS-13319. Simplify KeyPrefixFilter (apache#8692) HDDS-13295. Remove jackson1 exclusions for hadoop-common (apache#8687) HDDS-13314. Remove unused maven-pdf-plugin (apache#8686) HDDS-13324. Optimize memory footprint for Recon listKeys API (apache#8680) HDDS-13240. Add newly added metrics into grafana dashboard. (apache#8656) HDDS-13309. Add keyIterator/valueIterator methods to Table. (apache#8675) HDDS-13325. Introduce OZONE_SERVER_OPTS for common options for server processes (apache#8685) HDDS-13318. Simplify the getRangeKVs methods in Table (apache#8683) HDDS-13322. Remove module auto-detection from flaky-test-check (apache#8679) HDDS-13289. Remove usage of Jetty StringUtil (apache#8684) HDDS-13270. Reduce getBucket API invocations in S3 bucket owner verification (apache#8653) HDDS-13288. Container checksum file proto changes to account for deleted blocks. (apache#8649) HDDS-13306. Intermittent failure in testDirectoryDeletingServiceIntervalReconfiguration (apache#8682) HDDS-13317. Table should support empty array/String (apache#8676) ...
* master: (170 commits) HDDS-12468. Check for space availability for all dns while container creation in pipeline (apache#8663) HDDS-12151. Fail write when volume is full considering min free space (apache#8642) HDDS-13251. Update Byteman usage README license (apache#8700) HDDS-13251. Support dynamic Byteman scripts via bmsubmit in ozonesecure-ha (apache#8654) HDDS-12070. Bump Ratis to 3.2.0 (apache#8689) HDDS-12984. Use InodeID to identify the SST files inside the tarball. (apache#8477) HDDS-13319. Simplify KeyPrefixFilter (apache#8692) HDDS-13295. Remove jackson1 exclusions for hadoop-common (apache#8687) HDDS-13314. Remove unused maven-pdf-plugin (apache#8686) HDDS-13324. Optimize memory footprint for Recon listKeys API (apache#8680) HDDS-13240. Add newly added metrics into grafana dashboard. (apache#8656) HDDS-13309. Add keyIterator/valueIterator methods to Table. (apache#8675) HDDS-13325. Introduce OZONE_SERVER_OPTS for common options for server processes (apache#8685) HDDS-13318. Simplify the getRangeKVs methods in Table (apache#8683) HDDS-13322. Remove module auto-detection from flaky-test-check (apache#8679) HDDS-13289. Remove usage of Jetty StringUtil (apache#8684) HDDS-13270. Reduce getBucket API invocations in S3 bucket owner verification (apache#8653) HDDS-13288. Container checksum file proto changes to account for deleted blocks. (apache#8649) HDDS-13306. Intermittent failure in testDirectoryDeletingServiceIntervalReconfiguration (apache#8682) HDDS-13317. Table should support empty array/String (apache#8676) ... Conflicts: hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/checksum/ContainerChecksumTreeManager.java hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/checksum/TestContainerChecksumTreeManager.java
…n free space (apache#8642) (cherry picked from commit 99c163a) Conflicts: HddsDispatcher: Used audit(action, eventType, params, AuditEventStatus.FAILURE, e) since audit(action, eventType, msg, dispatcherContext, AuditEventStatus.FAILURE, e) is not available. ContainerUtils: Changed volume.getStorageID() to volume for the first exception as well in assertSpaceAvailability. Other conflicts: hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/impl/HddsDispatcher.java hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/volume/HddsVolume.java hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/impl/TestHddsDispatcher.java hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/impl/TestKeyValueStreamDataChannel.java
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.
Here, available includes: available - reserved part available - min.free.space configured
This is to mandate that min.free.space is available for ozone work.
HDDS-13286 is created to handle ratis stream write flow.
What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-12151
How was this patch tested?