You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by GitBox <gi...@apache.org> on 2022/09/21 05:40:56 UTC
[GitHub] [ozone] swamirishi opened a new pull request, #3768: HDDS-7208. Erasure coding and encryption are not flagged on FileStatus
swamirishi opened a new pull request, #3768:
URL: https://github.com/apache/ozone/pull/3768
## 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
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] jojochuang commented on a diff in pull request #3768: HDDS-7208. Erasure coding and encryption are not flagged on FileStatus
Posted by GitBox <gi...@apache.org>.
jojochuang commented on code in PR #3768:
URL: https://github.com/apache/ozone/pull/3768#discussion_r980408938
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java:
##########
@@ -1690,7 +1691,8 @@ private void findKeyInDbWithIterator(boolean recursive, String startKey,
if (!entryKeyName.equals(immediateChild)) {
OmKeyInfo fakeDirEntry = createDirectoryKey(
omKeyInfo.getVolumeName(), omKeyInfo.getBucketName(),
- immediateChild, omKeyInfo.getAcls());
+ immediateChild, omKeyInfo.getAcls(),
+ omKeyInfo.getReplicationConfig());
Review 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.
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestDataUtil.java:
##########
@@ -63,7 +61,8 @@ public static OzoneBucket createVolumeAndBucket(MiniOzoneCluster cluster,
if (bucketLayout != null) {
builder.setBucketLayout(bucketLayout);
}
- omBucketArgs = builder.build();
+ omBucketArgs =
+ builder.setDefaultReplicationConfig(new DefaultReplicationConfig(new ECReplicationConfig(3,2))).build();
Review 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.
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestOzoneFileSystem.java:
##########
@@ -610,12 +602,68 @@ public void testListStatusWithIntermediateDir() throws Exception {
}
FileStatus[] fileStatuses = fs.listStatus(parent);
-
// the number of immediate children of root is 1
Assert.assertEquals(1, fileStatuses.length);
writeClient.deleteKey(keyArgs);
}
+ @Test
+ public void testListStatusWithIntermediateDirWithECEnabled() throws Exception {
+ System.out.println("Executing");
+ String keyName = "object-dir/object-name1";
+ OmKeyArgs keyArgs = new OmKeyArgs.Builder()
+ .setVolumeName(volumeName)
+ .setBucketName(bucketName)
+ .setKeyName(keyName)
+ .setAcls(Collections.emptyList())
+ .setReplicationConfig(new ECReplicationConfig(3, 2))
+ .setLocationInfoList(new ArrayList<>())
+ .build();
+ OpenKeySession session = writeClient.openKey(keyArgs);
+
+ writeClient.commitKey(keyArgs, session.getId());
+
+
+
+
+
+
Review Comment:
```suggestion
```
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestOzoneFileSystem.java:
##########
@@ -209,6 +200,7 @@ public static void teardown() {
@After
public void cleanup() {
try {
+ System.out.println("Cleaning Up");
Review Comment:
```suggestion
LOG.info("Cleaning Up");
```
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestDataUtil.java:
##########
@@ -23,9 +23,7 @@
import java.util.HashMap;
import java.util.Scanner;
-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.*;
Review Comment:
This change is not needed.
You can consider updating IntelliJ settings to not make wildcard imports.
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestOzoneFileSystem.java:
##########
@@ -610,12 +602,68 @@ public void testListStatusWithIntermediateDir() throws Exception {
}
FileStatus[] fileStatuses = fs.listStatus(parent);
-
// the number of immediate children of root is 1
Assert.assertEquals(1, fileStatuses.length);
writeClient.deleteKey(keyArgs);
}
+ @Test
+ public void testListStatusWithIntermediateDirWithECEnabled() throws Exception {
+ System.out.println("Executing");
+ String keyName = "object-dir/object-name1";
+ OmKeyArgs keyArgs = new OmKeyArgs.Builder()
+ .setVolumeName(volumeName)
+ .setBucketName(bucketName)
+ .setKeyName(keyName)
+ .setAcls(Collections.emptyList())
+ .setReplicationConfig(new ECReplicationConfig(3, 2))
+ .setLocationInfoList(new ArrayList<>())
+ .build();
+ OpenKeySession session = writeClient.openKey(keyArgs);
+
+ writeClient.commitKey(keyArgs, session.getId());
+
+
+
+
+
+
+
+ keyName = "object-dir/object-name2";
+ keyArgs = new OmKeyArgs.Builder()
+ .setVolumeName(volumeName)
+ .setBucketName(bucketName)
+ .setKeyName(keyName)
+ .setAcls(Collections.emptyList())
+ .setReplicationConfig(new ECReplicationConfig(3, 2))
+ .setLocationInfoList(new ArrayList<>())
+ .build();
+ session = writeClient.openKey(keyArgs);
+
+ writeClient.commitKey(keyArgs, session.getId());
+
+ Path parent = new Path("/");
+
+ // Wait until the filestatus is updated
+ if (!enabledFileSystemPaths) {
+ GenericTestUtils.waitFor(() -> {
+ try {
+ return fs.listStatus(parent).length != 0;
+ } catch (IOException e) {
+ LOG.error("listStatus() Failed", e);
+ Assert.fail("listStatus() Failed");
+ return false;
+ }
+ }, 1000, 120000);
+ }
Review Comment:
is this necessary? I am under the impression commitKey() is synchronous and listStatus() should reflect the change immediately?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] jojochuang commented on pull request #3768: HDDS-7208. Erasure coding and encryption are not flagged on FileStatus
Posted by GitBox <gi...@apache.org>.
jojochuang commented on PR #3768:
URL: https://github.com/apache/ozone/pull/3768#issuecomment-1272046492
+1 merging now.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] swamirishi commented on pull request #3768: HDDS-7208. Erasure coding and encryption are not flagged on FileStatus
Posted by GitBox <gi...@apache.org>.
swamirishi commented on PR #3768:
URL: https://github.com/apache/ozone/pull/3768#issuecomment-1264052520
> 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?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] jojochuang commented on a diff in pull request #3768: HDDS-7208. Erasure coding and encryption are not flagged on FileStatus
Posted by GitBox <gi...@apache.org>.
jojochuang commented on code in PR #3768:
URL: https://github.com/apache/ozone/pull/3768#discussion_r976982379
##########
hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicOzoneClientAdapterImpl.java:
##########
@@ -534,7 +536,10 @@ private FileStatusAdapter toFileStatusAdapter(OzoneFileStatus status,
owner,
owner,
null,
- getBlockLocations(status)
+ getBlockLocations(status),
+ !Objects.isNull(keyInfo.getFileEncryptionInfo()),
+ keyInfo.getReplicationConfig().getReplicationType() ==
+ HddsProtos.ReplicationType.EC
Review 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?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] adoroszlai commented on pull request #3768: HDDS-7208. Erasure coding and encryption are not flagged on FileStatus
Posted by GitBox <gi...@apache.org>.
adoroszlai commented on PR #3768:
URL: https://github.com/apache/ozone/pull/3768#issuecomment-1253240290
Thanks @swamirishi for working on this. Before opening a PR, please wait for successful [CI run](https://github.com/swamirishi/ozone/actions/runs/3095449451) 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?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] swamirishi commented on pull request #3768: HDDS-7208. Erasure coding and encryption are not flagged on FileStatus
Posted by GitBox <gi...@apache.org>.
swamirishi commented on PR #3768:
URL: https://github.com/apache/ozone/pull/3768#issuecomment-1264052870
I am fixing the test failures
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] jojochuang merged pull request #3768: HDDS-7208. Erasure coding and encryption are not flagged on FileStatus
Posted by GitBox <gi...@apache.org>.
jojochuang merged PR #3768:
URL: https://github.com/apache/ozone/pull/3768
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] swamirishi closed pull request #3768: HDDS-7208. Erasure coding and encryption are not flagged on FileStatus
Posted by GitBox <gi...@apache.org>.
swamirishi closed pull request #3768: HDDS-7208. Erasure coding and encryption are not flagged on FileStatus
URL: https://github.com/apache/ozone/pull/3768
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] jojochuang commented on a diff in pull request #3768: HDDS-7208. Erasure coding and encryption are not flagged on FileStatus
Posted by GitBox <gi...@apache.org>.
jojochuang commented on code in PR #3768:
URL: https://github.com/apache/ozone/pull/3768#discussion_r976993653
##########
hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneClientAdapterImpl.java:
##########
@@ -1051,8 +1056,7 @@ private static FileStatusAdapter getFileStatusAdapterForVolume(
return new FileStatusAdapter(0L, path, true, (short)0, 0L,
ozoneVolume.getCreationTime().getEpochSecond() * 1000, 0L,
FsPermission.getDirDefault().toShort(),
- owner, group, path,
- new BlockLocation[0]
+ owner, group, path, new BlockLocation[0], false, false
Review Comment:
Ok. So an Ozone volume does not have the properties of either encryption nor EC. So they should be false. Makes sense.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] sodonnel commented on pull request #3768: HDDS-7208. Erasure coding and encryption are not flagged on FileStatus
Posted by GitBox <gi...@apache.org>.
sodonnel commented on PR #3768:
URL: https://github.com/apache/ozone/pull/3768#issuecomment-1263961829
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?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] swamirishi commented on pull request #3768: HDDS-7208. Erasure coding and encryption are not flagged on FileStatus
Posted by GitBox <gi...@apache.org>.
swamirishi commented on PR #3768:
URL: https://github.com/apache/ozone/pull/3768#issuecomment-1269137420
@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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org