HDDS-7208. Erasure coding and encryption are not flagged on FileStatus#3768
HDDS-7208. Erasure coding and encryption are not flagged on FileStatus#3768jojochuang merged 10 commits intoapache:masterfrom
Conversation
|
Thanks @swamirishi for working on this. Before opening a PR, please wait for successful CI run in your fork. This helps save resources for Apache project. In this case checkstyle is failing and needs a fix. Also, can you please explain what "Changing Getter Setters" means in terms of testing? |
| keyInfo.getReplicationConfig().getReplicationType() == | ||
| HddsProtos.ReplicationType.EC |
There was a problem hiding this comment.
how about creating a helper method isErasureCoded() in OzoneClientAdpter and move this code there so that this code can be shared by both BasicOzoneClientAdapterImpl and BasicOzoneRootedOzoneClientAdapterImpl?
| FsPermission.getDirDefault().toShort(), | ||
| owner, group, path, | ||
| new BlockLocation[0] | ||
| owner, group, path, new BlockLocation[0], false, false |
There was a problem hiding this comment.
Ok. So an Ozone volume does not have the properties of either encryption nor EC. So they should be false. Makes sense.
| omKeyInfo.getVolumeName(), omKeyInfo.getBucketName(), | ||
| immediateChild, omKeyInfo.getAcls()); | ||
| immediateChild, omKeyInfo.getAcls(), | ||
| omKeyInfo.getReplicationConfig()); |
There was a problem hiding this comment.
This is fine.
But if most of the parameters are taken from omKeyInfo, we can consider making createDirectoryKey() to take omKeyInfo and immediateChild only. That way if OmKeyInfo adds new fields in the future, we don't need to keep updating it and its caller.
| import org.apache.hadoop.hdds.client.ReplicationConfig; | ||
| import org.apache.hadoop.hdds.client.ReplicationFactor; | ||
| import org.apache.hadoop.hdds.client.ReplicationType; | ||
| import org.apache.hadoop.hdds.client.*; |
There was a problem hiding this comment.
This change is not needed.
You can consider updating IntelliJ settings to not make wildcard imports.
| } | ||
| omBucketArgs = builder.build(); | ||
| omBucketArgs = | ||
| builder.setDefaultReplicationConfig(new DefaultReplicationConfig(new ECReplicationConfig(3,2))).build(); |
There was a problem hiding this comment.
Unrelated, but it would be nice to have ECReplicationConfig objects as singletons.
I suspect we'll end up seeing a lot of duplicate ECReplicationConfig objects in the heap in the future.
| @After | ||
| public void cleanup() { | ||
| try { | ||
| System.out.println("Cleaning Up"); |
There was a problem hiding this comment.
| System.out.println("Cleaning Up"); | |
| LOG.info("Cleaning Up"); |
|
|
||
|
|
||
|
|
||
|
|
||
|
|
||
|
|
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestOzoneFileSystem.java
Show resolved
Hide resolved
|
I tried re-running the tests a few times and it looks like the integration test failures might be related to this change. Can you have a check please? |
|
I am fixing the test failures |
|
@sodonnel @jojochuang I have fixed issues related to Hadoop 2 Compatability with FileStatus class. There were failures for Hadoop 2 Acceptance tests as Erasure Coding is not supported in Hadoop 2. Thus the class FileStatus also does not support it. |
|
+1 merging now. |
4 participants
What changes were proposed in this pull request?
Hadoop FileStatus accepts flags for erasure coding and encryption on creation. Impala relies on FileStatus.isErasureCoded. Ozone doesn't use this signature, so isErasureCoded and isEncrypted always return false.
What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-7208
How was this patch tested?
Changing Getter Setters