rakeshadr commented on code in PR #3201:
URL: https://github.com/apache/ozone/pull/3201#discussion_r849203542
##########
hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto:
##########
@@ -810,6 +810,41 @@ message KeyLocationList {
optional bool isMultipartKey = 4 [default = false];
}
+/**
+ * Checksum algorithms/types used in Ozone
+ * Make sure this enum's integer values match enum values' id properties
defined
+ * in org.apache.hadoop.util.DataChecksum.Type
+ */
+enum ChecksumTypeProto {
+ CHECKSUM_NULL = 0;
+ CHECKSUM_CRC32 = 1;
+ CHECKSUM_CRC32C = 2;
+}
+
+enum FileChecksumTypeProto {
+ MD5CRC = 1; // BlockChecksum obtained by taking the MD5 digest of chunk CRCs
+ COMPOSITE_CRC = 2; // Chunk-independent CRC, optionally striped
Review Comment:
Can you reverse this so that default will be COMPOSITE_CRC.
```
COMPOSITE_CRC = 1
MD5CRC = 2
```
##########
hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/checksum/BaseFileChecksumHelper.java:
##########
@@ -147,16 +147,40 @@ private void fetchBlocks() throws IOException {
.build();
OmKeyInfo keyInfo = ozoneManagerClient.lookupKey(keyArgs);
+ if (keyInfo.getFileChecksum() != null &&
+ isFullLength(keyInfo.getDataSize())) {
+ // if the checksum is cached in OM, and we request the checksum of
+ // the full length.
+ fileChecksum = keyInfo.getFileChecksum();
+ }
+
// use OmKeyArgs to call Om.lookup() and get OmKeyInfo
keyLocationInfos = keyInfo
.getLatestVersionLocations().getBlocksLatestVersionOnly();
}
+ /**
+ * Return true if the requested length is longer than the file length
+ * (dataSize).
+ *
+ * @param dataSize file length
+ * @return
+ */
+ private boolean isFullLength(long dataSize) {
+ return this.length >= dataSize;
+ }
+
/**
* Compute file checksum given the list of chunk checksums requested earlier.
+ *
+ * Skip computation if the already computed, or if the OmKeyInfo of the key
+ * in OM has pre-computed checksum.
* @throws IOException
*/
public void compute() throws IOException {
+ if (fileChecksum != null) {
+ return;
Review Comment:
Please add a debug log message to capture this condition.
##########
hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto:
##########
@@ -810,6 +810,41 @@ message KeyLocationList {
optional bool isMultipartKey = 4 [default = false];
}
+/**
+ * Checksum algorithms/types used in Ozone
+ * Make sure this enum's integer values match enum values' id properties
defined
+ * in org.apache.hadoop.util.DataChecksum.Type
+ */
+enum ChecksumTypeProto {
+ CHECKSUM_NULL = 0;
+ CHECKSUM_CRC32 = 1;
+ CHECKSUM_CRC32C = 2;
+}
+
+enum FileChecksumTypeProto {
+ MD5CRC = 1; // BlockChecksum obtained by taking the MD5 digest of chunk CRCs
+ COMPOSITE_CRC = 2; // Chunk-independent CRC, optionally striped
+}
+
+message CompositeCrcFileChecksumProto {
+ required ChecksumTypeProto checksumType = 1;
+ required uint32 bytesPerCrc = 2;
+ required uint32 crc = 3;
+}
+
+message MD5MD5Crc32FileChecksumProto {
+ required ChecksumTypeProto checksumType = 1;
+ required uint32 bytesPerCRC = 2;
+ required uint64 crcPerBlock = 3;
+ required bytes md5 = 4;
+}
+
+message FileChecksumProto {
+ required FileChecksumTypeProto checksumType = 1;
Review Comment:
Can you add test cases to cover `FileChecksumTypeProto + ChecksumTypeProto`
combinations.
##########
hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/checksum/BaseFileChecksumHelper.java:
##########
@@ -147,16 +147,40 @@ private void fetchBlocks() throws IOException {
.build();
OmKeyInfo keyInfo = ozoneManagerClient.lookupKey(keyArgs);
+ if (keyInfo.getFileChecksum() != null &&
+ isFullLength(keyInfo.getDataSize())) {
+ // if the checksum is cached in OM, and we request the checksum of
+ // the full length.
+ fileChecksum = keyInfo.getFileChecksum();
+ }
+
// use OmKeyArgs to call Om.lookup() and get OmKeyInfo
keyLocationInfos = keyInfo
.getLatestVersionLocations().getBlocksLatestVersionOnly();
}
+ /**
+ * Return true if the requested length is longer than the file length
+ * (dataSize).
+ *
+ * @param dataSize file length
+ * @return
+ */
+ private boolean isFullLength(long dataSize) {
+ return this.length >= dataSize;
Review Comment:
@jojochuang Is it `this.length > dataSize`, to satisfy longer than the file
length ?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
