-
Notifications
You must be signed in to change notification settings - Fork 590
HDDS-10553. Add test case for creating file with EC replication config #6405
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
… config by ofs/o3fs
adoroszlai
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.
Thanks @YuanbenWang for the patch.
| Path root = new Path("/" + volumeName + "/" + bucketName); | ||
| Path testKeyPath = new Path(root, "testKey"); | ||
| conf.set(OZONE_REPLICATION, "rs-3-2-1024k"); | ||
| conf.set(OZONE_REPLICATION_TYPE, "EC"); | ||
| FileSystem fileSystem = FileSystem.get(conf); | ||
| ContractTestUtils.touch(fileSystem, testKeyPath); | ||
|
|
||
| OzoneKeyDetails key = getKey(testKeyPath, false); | ||
| assertEquals(HddsProtos.ReplicationType.EC, | ||
| key.getReplicationConfig().getReplicationType()); | ||
| assertEquals("rs-3-2-1024k", | ||
| key.getReplicationConfig().getReplication()); | ||
| fileSystem.close(); |
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.
Please move the implementation to a new method in OzoneFileSystemTests and call that from both test classes, to avoid code duplication.
| void init() throws Exception { | ||
| OzoneConfiguration conf = new OzoneConfiguration(); | ||
| conf = new OzoneConfiguration(); | ||
| conf.set("fs.o3fs.impl.disable.cache", "true"); |
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.
Please disable the cache only in the specific test case that needs new FileSystem with changed config, similar to:
Lines 63 to 65 in 264cbc6
| config.setBoolean( | |
| String.format("fs.%s.impl.disable.cache", uri.getScheme()), true); | |
| FileSystem subject = FileSystem.get(uri, config); |
|
@adoroszlai Thanks for your suggestions~! I have updated my code. Could you plz review it? |
adoroszlai
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.
Thanks @YuanbenWang for updating the patch.
...gration-test/src/test/java/org/apache/hadoop/fs/ozone/AbstractRootedOzoneFileSystemTest.java
Show resolved
Hide resolved
...op-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/OzoneFileSystemTests.java
Outdated
Show resolved
Hide resolved
...op-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/OzoneFileSystemTests.java
Outdated
Show resolved
Hide resolved
...op-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/OzoneFileSystemTests.java
Outdated
Show resolved
Hide resolved
|
@adoroszlai Thanks for the review~! I have updated my code again. Could you plz review it? |
This reverts commit c9ef5bd.
|
Thanks @YuanbenWang for the patch. |
|
@adoroszlai Thank you for the review. |
apache#6405) (cherry picked from commit b267a57)
What changes were proposed in this pull request?
ofs/o3fs is able to write keys/files with EC replication config, but there is no unit test and doc to explain how to do it. So this pr makes it.
What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-10553
How was this patch tested?
Unit Test
Tested locally.
hadoop client write:
key info: