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/06/07 14:57:49 UTC
[GitHub] [ozone] umamaheswararao opened a new pull request, #3489: HDDS-6815: EC: getFileCheckSum should return null EC files until ECFileChecksum implemented.
umamaheswararao opened a new pull request, #3489:
URL: https://github.com/apache/ozone/pull/3489
## What changes were proposed in this pull request?
GetFileChecksum is implemented only for Ratis based file layout. For EC files layout, we have no implementation ready yet. This JIRA is to return null for EC files until we have the implementation ready. Otherwise, we will get the wrong CRC unknowingly for EC files.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-6815
## How was this patch tested?
Added a UT.
--
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] umamaheswararao commented on pull request #3489: HDDS-6815: EC: getFileCheckSum should return null EC files until ECFileChecksum implemented.
Posted by GitBox <gi...@apache.org>.
umamaheswararao commented on PR #3489:
URL: https://github.com/apache/ozone/pull/3489#issuecomment-1164739007
Thanks @jojochuang for the review!
--
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 #3489: HDDS-6815: EC: getFileCheckSum should return null EC files until ECFileChecksum implemented.
Posted by GitBox <gi...@apache.org>.
jojochuang commented on PR #3489:
URL: https://github.com/apache/ozone/pull/3489#issuecomment-1150407545
Merged. Thanks @umamaheswararao !
--
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 #3489: HDDS-6815: EC: getFileCheckSum should return null EC files until ECFileChecksum implemented.
Posted by GitBox <gi...@apache.org>.
jojochuang merged PR #3489:
URL: https://github.com/apache/ozone/pull/3489
--
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 #3489: HDDS-6815: EC: getFileCheckSum should return null EC files until ECFileChecksum implemented.
Posted by GitBox <gi...@apache.org>.
jojochuang commented on code in PR #3489:
URL: https://github.com/apache/ozone/pull/3489#discussion_r891662348
##########
hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/OzoneClientUtils.java:
##########
@@ -205,8 +207,19 @@ public static FileChecksum getFileChecksumWithCombineMode(OzoneVolume volume,
if (keyName.length() == 0) {
return null;
}
- BaseFileChecksumHelper helper = new ReplicatedFileChecksumHelper(
- volume, bucket, keyName, length, combineMode, rpcClient);
+ OmKeyArgs keyArgs = new OmKeyArgs.Builder().setVolumeName(volume.getName())
+ .setBucketName(bucket.getName()).setKeyName(keyName)
+ .setRefreshPipeline(true).setSortDatanodesInPipeline(true)
+ .setLatestVersionLocation(true).build();
+ OmKeyInfo keyInfo = rpcClient.getOzoneManagerClient().lookupKey(keyArgs);
+
+ if (keyInfo.getReplicationConfig()
Review Comment:
can you add a TODO comment here? Like:
```suggestion
// TODO: return null util ECFileChecksum is implemented
if (keyInfo.getReplicationConfig()
```
##########
hadoop-ozone/ozonefs-common/src/test/java/org/apache/hadoop/fs/ozone/TestOzoneClientUtils.java:
##########
@@ -72,6 +76,34 @@ public void testEmptyKeyName() throws IOException {
assertNull(checksum);
}
+ @Test
+ public void testGetFileChecksumForECFileShouldReturnNull()
+ throws IOException {
+ OzoneVolume volume = mock(OzoneVolume.class);
+ OzoneBucket bucket = mock(OzoneBucket.class);
+ ClientProtocol clientProtocol = mock(ClientProtocol.class);
+ OzoneManagerProtocol ozoneManagerProtocol =
+ mock(OzoneManagerProtocol.class);
+ try {
+ OmKeyInfo omKeyInfo = new OmKeyInfo.Builder()
+ .setReplicationConfig(new ECReplicationConfig("rs-3-2-1024K"))
+ .setVolumeName("vol").setBucketName("bucket").setKeyName("mykey")
+ .setDataSize(10).setCreationTime(1).setModificationTime(1).build();
+ Mockito.when(clientProtocol.getOzoneManagerClient())
+ .thenReturn(ozoneManagerProtocol);
+ Mockito.when(ozoneManagerProtocol.lookupKey(any())).thenReturn(omKeyInfo);
+ FileChecksum checksum = OzoneClientUtils
+ .getFileChecksumWithCombineMode(volume, bucket, "test", 1,
+ OzoneClientConfig.ChecksumCombineMode.MD5MD5CRC, clientProtocol);
+ assertNull(checksum);
+ } finally {
+ Mockito.reset(volume);
+ Mockito.reset(bucket);
+ Mockito.reset(volume);
+ Mockito.reset(volume);
Review Comment:
i'm not familiar with this. Is it required to reset volume multiple times?
--
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] umamaheswararao commented on a diff in pull request #3489: HDDS-6815: EC: getFileCheckSum should return null EC files until ECFileChecksum implemented.
Posted by GitBox <gi...@apache.org>.
umamaheswararao commented on code in PR #3489:
URL: https://github.com/apache/ozone/pull/3489#discussion_r891694752
##########
hadoop-ozone/ozonefs-common/src/test/java/org/apache/hadoop/fs/ozone/TestOzoneClientUtils.java:
##########
@@ -72,6 +76,34 @@ public void testEmptyKeyName() throws IOException {
assertNull(checksum);
}
+ @Test
+ public void testGetFileChecksumForECFileShouldReturnNull()
+ throws IOException {
+ OzoneVolume volume = mock(OzoneVolume.class);
+ OzoneBucket bucket = mock(OzoneBucket.class);
+ ClientProtocol clientProtocol = mock(ClientProtocol.class);
+ OzoneManagerProtocol ozoneManagerProtocol =
+ mock(OzoneManagerProtocol.class);
+ try {
+ OmKeyInfo omKeyInfo = new OmKeyInfo.Builder()
+ .setReplicationConfig(new ECReplicationConfig("rs-3-2-1024K"))
+ .setVolumeName("vol").setBucketName("bucket").setKeyName("mykey")
+ .setDataSize(10).setCreationTime(1).setModificationTime(1).build();
+ Mockito.when(clientProtocol.getOzoneManagerClient())
+ .thenReturn(ozoneManagerProtocol);
+ Mockito.when(ozoneManagerProtocol.lookupKey(any())).thenReturn(omKeyInfo);
+ FileChecksum checksum = OzoneClientUtils
+ .getFileChecksumWithCombineMode(volume, bucket, "test", 1,
+ OzoneClientConfig.ChecksumCombineMode.MD5MD5CRC, clientProtocol);
+ assertNull(checksum);
+ } finally {
+ Mockito.reset(volume);
+ Mockito.reset(bucket);
+ Mockito.reset(volume);
+ Mockito.reset(volume);
Review Comment:
Oops, this is my mistake. I meant to clean all other mocks :-)
--
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] umamaheswararao commented on a diff in pull request #3489: HDDS-6815: EC: getFileCheckSum should return null EC files until ECFileChecksum implemented.
Posted by GitBox <gi...@apache.org>.
umamaheswararao commented on code in PR #3489:
URL: https://github.com/apache/ozone/pull/3489#discussion_r891694500
##########
hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/OzoneClientUtils.java:
##########
@@ -205,8 +207,19 @@ public static FileChecksum getFileChecksumWithCombineMode(OzoneVolume volume,
if (keyName.length() == 0) {
return null;
}
- BaseFileChecksumHelper helper = new ReplicatedFileChecksumHelper(
- volume, bucket, keyName, length, combineMode, rpcClient);
+ OmKeyArgs keyArgs = new OmKeyArgs.Builder().setVolumeName(volume.getName())
+ .setBucketName(bucket.getName()).setKeyName(keyName)
+ .setRefreshPipeline(true).setSortDatanodesInPipeline(true)
+ .setLatestVersionLocation(true).build();
+ OmKeyInfo keyInfo = rpcClient.getOzoneManagerClient().lookupKey(keyArgs);
+
+ if (keyInfo.getReplicationConfig()
Review Comment:
Sure, I thought I added a comment somewhere. Let me do that right away.
--
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