From 09115ec0da534eed9868ee523bab4a69cd7f5a93 Mon Sep 17 00:00:00 2001 From: Wei-Chiu Chuang Date: Tue, 21 Mar 2023 16:04:12 -0700 Subject: [PATCH 1/4] WIP Change-Id: Ib1c8ef8c517a77bea76407a6a356e2320e36fccb --- .../hadoop/ozone/om/helpers/OmKeyInfo.java | 38 ++++++++++++++++--- .../ozone/om/helpers/TestOmKeyInfo.java | 2 + .../src/main/proto/OmClientProtocol.proto | 1 + 3 files changed, 36 insertions(+), 5 deletions(-) diff --git a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmKeyInfo.java b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmKeyInfo.java index 4bb60506975e..a6cf543485ec 100644 --- a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmKeyInfo.java +++ b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmKeyInfo.java @@ -78,6 +78,9 @@ public final class OmKeyInfo extends WithParentObjectId { */ private List acls; + + private boolean isHsync; + @SuppressWarnings("parameternumber") OmKeyInfo(String volumeName, String bucketName, String keyName, List versions, long dataSize, @@ -110,13 +113,14 @@ public final class OmKeyInfo extends WithParentObjectId { Map metadata, FileEncryptionInfo encInfo, List acls, long parentObjectID, long objectID, long updateID, - FileChecksum fileChecksum, boolean isFile) { + FileChecksum fileChecksum, boolean isFile, boolean isHsync) { this(volumeName, bucketName, keyName, versions, dataSize, creationTime, modificationTime, replicationConfig, metadata, encInfo, acls, objectID, updateID, fileChecksum); this.fileName = fileName; this.parentObjectID = parentObjectID; this.isFile = isFile; + this.isHsync = isHsync; } public String getVolumeName() { @@ -190,6 +194,14 @@ public boolean isFile() { return isFile; } + public void setHsync(boolean hsync) { + isHsync = hsync; + } + + public boolean isHsync() { + return isHsync; + } + /** * updates the length of the each block in the list given. * This will be called when the key is being committed to OzoneManager. @@ -424,6 +436,8 @@ public static class Builder { private boolean isFile; + private boolean isHsync; + public Builder() { this.metadata = new HashMap<>(); omKeyLocationInfoGroups = new ArrayList<>(); @@ -540,12 +554,18 @@ public Builder setFile(boolean isAFile) { return this; } + public Builder setHSync(boolean isHSync) { + this.isHsync = isHSync; + return this; + } + public OmKeyInfo build() { return new OmKeyInfo( volumeName, bucketName, keyName, fileName, omKeyLocationInfoGroups, dataSize, creationTime, modificationTime, replicationConfig, metadata, encInfo, acls, - parentObjectID, objectID, updateID, fileChecksum, isFile); + parentObjectID, objectID, updateID, fileChecksum, isFile, + isHsync); } } @@ -648,6 +668,7 @@ private KeyInfo getProtobuf(boolean ignorePipeline, String fullKeyName, kb.setFileEncryptionInfo(OMPBHelper.convert(encInfo)); } kb.setIsFile(isFile); + kb.setIsHsync(isHsync); return kb.build(); } @@ -695,6 +716,10 @@ public static OmKeyInfo getFromProtobuf(KeyInfo keyInfo) throws IOException { builder.setFile(keyInfo.getIsFile()); } + if (keyInfo.hasIsHsync()) { + builder.setHSync(keyInfo.getIsHsync()); + } + // not persisted to DB. FileName will be filtered out from keyName builder.setFileName(OzoneFSUtils.getFileName(keyInfo.getKeyName())); return builder.build(); @@ -711,7 +736,8 @@ public String getObjectInfo() { ", objectID='" + objectID + '\'' + ", parentID='" + parentObjectID + '\'' + ", replication='" + replicationConfig + '\'' + - ", fileChecksum='" + fileChecksum + + ", fileChecksum='" + fileChecksum + '\'' + + ", isHsync='" + isHsync + '}'; } @@ -737,7 +763,8 @@ public boolean equals(Object o) { Objects.equals(acls, omKeyInfo.acls) && objectID == omKeyInfo.objectID && updateID == omKeyInfo.updateID && - parentObjectID == omKeyInfo.parentObjectID; + parentObjectID == omKeyInfo.parentObjectID && + isHsync == omKeyInfo.isHsync; } @Override @@ -762,7 +789,8 @@ public OmKeyInfo copyObject() { .setUpdateID(updateID) .setParentObjectID(parentObjectID) .setFileName(fileName) - .setFile(isFile); + .setFile(isFile) + .setHSync(isHsync); keyLocationVersions.forEach(keyLocationVersion -> builder.addOmKeyLocationInfoGroup( diff --git a/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/helpers/TestOmKeyInfo.java b/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/helpers/TestOmKeyInfo.java index f4e5fc9932f0..6647ba71076b 100644 --- a/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/helpers/TestOmKeyInfo.java +++ b/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/helpers/TestOmKeyInfo.java @@ -82,6 +82,7 @@ public void getProtobufMessageEC() throws IOException { // EC Config key = createOmKeyInfo(new ECReplicationConfig(3, 2)); + Assert.assertFalse(key.isHsync()); omKeyProto = key.getProtobuf(ClientVersion.CURRENT_VERSION); Assert.assertEquals(3, omKeyProto.getEcReplicationConfig().getData()); @@ -112,6 +113,7 @@ private OmKeyInfo createOmKeyInfo(ReplicationConfig replicationConfig) { .setReplicationConfig(replicationConfig) .addMetadata("key1", "value1") .addMetadata("key2", "value2") + .setHSync(false) .build(); } diff --git a/hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto b/hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto index 295065abfdd7..b1005dc61360 100644 --- a/hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto +++ b/hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto @@ -1017,6 +1017,7 @@ message KeyInfo { optional hadoop.hdds.ECReplicationConfig ecReplicationConfig = 17; optional FileChecksumProto fileChecksum = 18; optional bool isFile = 19; + optional bool isHsync = 20; } message DirectoryInfo { From a4632abf6a540a2379389602b09231bd69e1f563 Mon Sep 17 00:00:00 2001 From: Wei-Chiu Chuang Date: Tue, 21 Mar 2023 16:27:18 -0700 Subject: [PATCH 2/4] Update Change-Id: I94d4a9ee17d4ccebdb231a38373d075a3e9532f5 --- .../hadoop/ozone/om/helpers/OmKeyInfo.java | 16 +++++++++------- .../src/main/proto/OmClientProtocol.proto | 1 - 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmKeyInfo.java b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmKeyInfo.java index a6cf543485ec..589a5dd6d000 100644 --- a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmKeyInfo.java +++ b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmKeyInfo.java @@ -78,7 +78,6 @@ public final class OmKeyInfo extends WithParentObjectId { */ private List acls; - private boolean isHsync; @SuppressWarnings("parameternumber") @@ -502,6 +501,8 @@ public Builder addMetadata(String key, String value) { public Builder addAllMetadata(Map newMetadata) { metadata.putAll(newMetadata); + + setHSync(metadata.containsKey(OzoneConsts.HSYNC_CLIENT_ID)); return this; } @@ -667,8 +668,6 @@ private KeyInfo getProtobuf(boolean ignorePipeline, String fullKeyName, if (encInfo != null) { kb.setFileEncryptionInfo(OMPBHelper.convert(encInfo)); } - kb.setIsFile(isFile); - kb.setIsHsync(isHsync); return kb.build(); } @@ -683,6 +682,9 @@ public static OmKeyInfo getFromProtobuf(KeyInfo keyInfo) throws IOException { OmKeyLocationInfoGroup.getFromProtobuf(keyLocationList)); } + Map newMetadata = + KeyValueUtil.getFromProtobuf(keyInfo.getMetadataList()); + Builder builder = new Builder() .setVolumeName(keyInfo.getVolumeName()) .setBucketName(keyInfo.getBucketName()) @@ -694,7 +696,7 @@ public static OmKeyInfo getFromProtobuf(KeyInfo keyInfo) throws IOException { .setReplicationConfig(ReplicationConfig .fromProto(keyInfo.getType(), keyInfo.getFactor(), keyInfo.getEcReplicationConfig())) - .addAllMetadata(KeyValueUtil.getFromProtobuf(keyInfo.getMetadataList())) + .addAllMetadata(newMetadata) .setFileEncryptionInfo(keyInfo.hasFileEncryptionInfo() ? OMPBHelper.convert(keyInfo.getFileEncryptionInfo()) : null) .setAcls(OzoneAclUtil.fromProtobuf(keyInfo.getAclsList())); @@ -716,9 +718,7 @@ public static OmKeyInfo getFromProtobuf(KeyInfo keyInfo) throws IOException { builder.setFile(keyInfo.getIsFile()); } - if (keyInfo.hasIsHsync()) { - builder.setHSync(keyInfo.getIsHsync()); - } + builder.setHSync(newMetadata.containsKey(OzoneConsts.HSYNC_CLIENT_ID)); // not persisted to DB. FileName will be filtered out from keyName builder.setFileName(OzoneFSUtils.getFileName(keyInfo.getKeyName())); @@ -804,6 +804,8 @@ public OmKeyInfo copyObject() { if (metadata != null) { metadata.forEach((k, v) -> builder.addMetadata(k, v)); + + builder.setHSync(metadata.containsKey(OzoneConsts.HSYNC_CLIENT_ID)); } if (fileChecksum != null) { diff --git a/hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto b/hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto index b1005dc61360..295065abfdd7 100644 --- a/hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto +++ b/hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto @@ -1017,7 +1017,6 @@ message KeyInfo { optional hadoop.hdds.ECReplicationConfig ecReplicationConfig = 17; optional FileChecksumProto fileChecksum = 18; optional bool isFile = 19; - optional bool isHsync = 20; } message DirectoryInfo { From 4eb8a987d253043cb9fe6fd084b9d6666e70c8ec Mon Sep 17 00:00:00 2001 From: Wei-Chiu Chuang Date: Wed, 22 Mar 2023 15:58:06 -0700 Subject: [PATCH 3/4] Fix test bugs Change-Id: I0dce6b498db984fd0b6c5ceda1e2203ccb0d482d --- .../main/java/org/apache/hadoop/ozone/om/helpers/OmKeyInfo.java | 1 + 1 file changed, 1 insertion(+) diff --git a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmKeyInfo.java b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmKeyInfo.java index 589a5dd6d000..b725e24d6ca2 100644 --- a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmKeyInfo.java +++ b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmKeyInfo.java @@ -668,6 +668,7 @@ private KeyInfo getProtobuf(boolean ignorePipeline, String fullKeyName, if (encInfo != null) { kb.setFileEncryptionInfo(OMPBHelper.convert(encInfo)); } + kb.setIsFile(isFile); return kb.build(); } From 7afdec9f5fe16f42151d0662e94be2a85ff444f1 Mon Sep 17 00:00:00 2001 From: Wei-Chiu Chuang Date: Sun, 26 Mar 2023 11:02:47 -0700 Subject: [PATCH 4/4] Update: no more extra field for isHsync. Change-Id: Ic92b5a2430222f368307b837f3bfe22bef24fb5e --- .../hadoop/ozone/om/helpers/OmKeyInfo.java | 41 ++++--------------- .../ozone/om/helpers/TestOmKeyInfo.java | 6 ++- 2 files changed, 12 insertions(+), 35 deletions(-) diff --git a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmKeyInfo.java b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmKeyInfo.java index b725e24d6ca2..752e5b04d366 100644 --- a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmKeyInfo.java +++ b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmKeyInfo.java @@ -78,8 +78,6 @@ public final class OmKeyInfo extends WithParentObjectId { */ private List acls; - private boolean isHsync; - @SuppressWarnings("parameternumber") OmKeyInfo(String volumeName, String bucketName, String keyName, List versions, long dataSize, @@ -112,14 +110,13 @@ public final class OmKeyInfo extends WithParentObjectId { Map metadata, FileEncryptionInfo encInfo, List acls, long parentObjectID, long objectID, long updateID, - FileChecksum fileChecksum, boolean isFile, boolean isHsync) { + FileChecksum fileChecksum, boolean isFile) { this(volumeName, bucketName, keyName, versions, dataSize, creationTime, modificationTime, replicationConfig, metadata, encInfo, acls, objectID, updateID, fileChecksum); this.fileName = fileName; this.parentObjectID = parentObjectID; this.isFile = isFile; - this.isHsync = isHsync; } public String getVolumeName() { @@ -193,12 +190,8 @@ public boolean isFile() { return isFile; } - public void setHsync(boolean hsync) { - isHsync = hsync; - } - public boolean isHsync() { - return isHsync; + return metadata.containsKey(OzoneConsts.HSYNC_CLIENT_ID); } /** @@ -435,8 +428,6 @@ public static class Builder { private boolean isFile; - private boolean isHsync; - public Builder() { this.metadata = new HashMap<>(); omKeyLocationInfoGroups = new ArrayList<>(); @@ -501,8 +492,6 @@ public Builder addMetadata(String key, String value) { public Builder addAllMetadata(Map newMetadata) { metadata.putAll(newMetadata); - - setHSync(metadata.containsKey(OzoneConsts.HSYNC_CLIENT_ID)); return this; } @@ -555,18 +544,12 @@ public Builder setFile(boolean isAFile) { return this; } - public Builder setHSync(boolean isHSync) { - this.isHsync = isHSync; - return this; - } - public OmKeyInfo build() { return new OmKeyInfo( volumeName, bucketName, keyName, fileName, omKeyLocationInfoGroups, dataSize, creationTime, modificationTime, replicationConfig, metadata, encInfo, acls, - parentObjectID, objectID, updateID, fileChecksum, isFile, - isHsync); + parentObjectID, objectID, updateID, fileChecksum, isFile); } } @@ -683,9 +666,6 @@ public static OmKeyInfo getFromProtobuf(KeyInfo keyInfo) throws IOException { OmKeyLocationInfoGroup.getFromProtobuf(keyLocationList)); } - Map newMetadata = - KeyValueUtil.getFromProtobuf(keyInfo.getMetadataList()); - Builder builder = new Builder() .setVolumeName(keyInfo.getVolumeName()) .setBucketName(keyInfo.getBucketName()) @@ -697,7 +677,7 @@ public static OmKeyInfo getFromProtobuf(KeyInfo keyInfo) throws IOException { .setReplicationConfig(ReplicationConfig .fromProto(keyInfo.getType(), keyInfo.getFactor(), keyInfo.getEcReplicationConfig())) - .addAllMetadata(newMetadata) + .addAllMetadata(KeyValueUtil.getFromProtobuf(keyInfo.getMetadataList())) .setFileEncryptionInfo(keyInfo.hasFileEncryptionInfo() ? OMPBHelper.convert(keyInfo.getFileEncryptionInfo()) : null) .setAcls(OzoneAclUtil.fromProtobuf(keyInfo.getAclsList())); @@ -719,8 +699,6 @@ public static OmKeyInfo getFromProtobuf(KeyInfo keyInfo) throws IOException { builder.setFile(keyInfo.getIsFile()); } - builder.setHSync(newMetadata.containsKey(OzoneConsts.HSYNC_CLIENT_ID)); - // not persisted to DB. FileName will be filtered out from keyName builder.setFileName(OzoneFSUtils.getFileName(keyInfo.getKeyName())); return builder.build(); @@ -737,8 +715,7 @@ public String getObjectInfo() { ", objectID='" + objectID + '\'' + ", parentID='" + parentObjectID + '\'' + ", replication='" + replicationConfig + '\'' + - ", fileChecksum='" + fileChecksum + '\'' + - ", isHsync='" + isHsync + + ", fileChecksum='" + fileChecksum + '}'; } @@ -764,8 +741,7 @@ public boolean equals(Object o) { Objects.equals(acls, omKeyInfo.acls) && objectID == omKeyInfo.objectID && updateID == omKeyInfo.updateID && - parentObjectID == omKeyInfo.parentObjectID && - isHsync == omKeyInfo.isHsync; + parentObjectID == omKeyInfo.parentObjectID; } @Override @@ -790,8 +766,7 @@ public OmKeyInfo copyObject() { .setUpdateID(updateID) .setParentObjectID(parentObjectID) .setFileName(fileName) - .setFile(isFile) - .setHSync(isHsync); + .setFile(isFile); keyLocationVersions.forEach(keyLocationVersion -> builder.addOmKeyLocationInfoGroup( @@ -805,8 +780,6 @@ public OmKeyInfo copyObject() { if (metadata != null) { metadata.forEach((k, v) -> builder.addMetadata(k, v)); - - builder.setHSync(metadata.containsKey(OzoneConsts.HSYNC_CLIENT_ID)); } if (fileChecksum != null) { diff --git a/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/helpers/TestOmKeyInfo.java b/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/helpers/TestOmKeyInfo.java index 6647ba71076b..0d421dc15273 100644 --- a/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/helpers/TestOmKeyInfo.java +++ b/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/helpers/TestOmKeyInfo.java @@ -27,6 +27,7 @@ import org.apache.hadoop.hdds.scm.pipeline.PipelineID; import org.apache.hadoop.ozone.ClientVersion; import org.apache.hadoop.ozone.OzoneAcl; +import org.apache.hadoop.ozone.OzoneConsts; import org.apache.hadoop.ozone.om.helpers.OmKeyInfo.Builder; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos; import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer; @@ -59,6 +60,10 @@ public void protobufConversion() throws IOException { key.getProtobuf(ClientVersion.CURRENT_VERSION)); Assert.assertEquals(key, keyAfterSerialization); + + Assert.assertFalse(key.isHsync()); + key.getMetadata().put(OzoneConsts.HSYNC_CLIENT_ID, "clientid"); + Assert.assertTrue(key.isHsync()); } @Test @@ -113,7 +118,6 @@ private OmKeyInfo createOmKeyInfo(ReplicationConfig replicationConfig) { .setReplicationConfig(replicationConfig) .addMetadata("key1", "value1") .addMetadata("key2", "value2") - .setHSync(false) .build(); }