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 2020/01/10 01:47:53 UTC
[GitHub] [hadoop-ozone] hanishakoneru opened a new pull request #428:
HDDS-2868. Add ObjectID and UpdateID to OMKeyInfo.
hanishakoneru opened a new pull request #428: HDDS-2868. Add ObjectID and UpdateID to OMKeyInfo.
URL: https://github.com/apache/hadoop-ozone/pull/428
## What changes were proposed in this pull request?
Similar to OMVolumeInfo and OMBucketInfo, we need objectID and updateID in OMKeyInfo as well to ensure that transactions are idempotent.
This Jira adds objectID and updateID to OMKeyInfo proto and sets these values when creating/ updating keys.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-2868
## How was this patch tested?
Integration and Acceptance tests. Follow-up Jiras for key operation idempotence will add more tests.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] arp7 commented on a change in pull request #428:
HDDS-2868. Add ObjectID and UpdateID to OMKeyInfo.
Posted by GitBox <gi...@apache.org>.
arp7 commented on a change in pull request #428: HDDS-2868. Add ObjectID and UpdateID to OMKeyInfo.
URL: https://github.com/apache/hadoop-ozone/pull/428#discussion_r365630790
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRenameRequest.java
##########
@@ -112,6 +112,7 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
OMMetadataManager omMetadataManager = ozoneManager.getMetadataManager();
boolean acquiredLock = false;
OMClientResponse omClientResponse = null;
+ boolean success = true;
Review comment:
It may be safer to initialize success to false, and set it to true at the end of the try block when all operations are complete.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] arp7 commented on a change in pull request #428:
HDDS-2868. Add ObjectID and UpdateID to OMKeyInfo.
Posted by GitBox <gi...@apache.org>.
arp7 commented on a change in pull request #428: HDDS-2868. Add ObjectID and UpdateID to OMKeyInfo.
URL: https://github.com/apache/hadoop-ozone/pull/428#discussion_r365630675
##########
File path: hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmKeyInfo.java
##########
@@ -52,6 +52,9 @@
private HddsProtos.ReplicationType type;
private HddsProtos.ReplicationFactor factor;
private FileEncryptionInfo encInfo;
+ private long objectID;
+ private long updateID;
+
Review comment:
Also what happens for deployed clusters where the persisted keys have no object Id or update ID? What do we initialize it to?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] hanishakoneru commented on a change in pull request
#428: HDDS-2868. Add ObjectID and UpdateID to OMKeyInfo.
Posted by GitBox <gi...@apache.org>.
hanishakoneru commented on a change in pull request #428: HDDS-2868. Add ObjectID and UpdateID to OMKeyInfo.
URL: https://github.com/apache/hadoop-ozone/pull/428#discussion_r366546223
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/s3/multipart/S3MultipartUploadCompleteRequest.java
##########
@@ -229,17 +229,33 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
// This is a newly added key, it does not have any versions.
OmKeyLocationInfoGroup keyLocationInfoGroup = new
OmKeyLocationInfoGroup(0, partLocationInfos);
+
+ // Get the objectID of the key from OpenKeyTable
+ OmKeyInfo dbOpenKeyInfo = omMetadataManager.getOpenKeyTable()
+ .get(multipartKey);
+
// A newly created key, this is the first version.
- omKeyInfo = new OmKeyInfo.Builder().setVolumeName(volumeName)
+ OmKeyInfo.Builder builder =
+ new OmKeyInfo.Builder().setVolumeName(volumeName)
.setBucketName(bucketName).setKeyName(keyName)
.setReplicationFactor(factor).setReplicationType(type)
.setCreationTime(keyArgs.getModificationTime())
.setModificationTime(keyArgs.getModificationTime())
.setDataSize(dataSize)
.setOmKeyLocationInfos(
Collections.singletonList(keyLocationInfoGroup))
- .setAcls(OzoneAclUtil.fromProtobuf(keyArgs.getAclsList()))
- .build();
+ .setAcls(OzoneAclUtil.fromProtobuf(keyArgs.getAclsList()));
+ // Check if db entry has ObjectID. This check is required because
+ // it is possible that between multipart key uploads and complete,
+ // we had an upgrade.
+ // TODO: If on upgrades we do not consider the non-completed multipart
+ // uploads, then this check can be removed.
+ if (dbOpenKeyInfo.getObjectID() != 0) {
Review comment:
Done.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] bharatviswa504 commented on a change in pull request
#428: HDDS-2868. Add ObjectID and UpdateID to OMKeyInfo.
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on a change in pull request #428: HDDS-2868. Add ObjectID and UpdateID to OMKeyInfo.
URL: https://github.com/apache/hadoop-ozone/pull/428#discussion_r365092199
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRequest.java
##########
@@ -416,12 +419,14 @@ protected OmKeyInfo prepareKeyInfo(
@Nonnull KeyArgs keyArgs, @Nonnull String dbKeyName, long size,
@Nonnull List<OmKeyLocationInfo> locations,
@Nullable FileEncryptionInfo encInfo,
- @Nonnull PrefixManager prefixManager, @Nullable OmBucketInfo omBucketInfo)
+ @Nonnull PrefixManager prefixManager,
+ @Nullable OmBucketInfo omBucketInfo,
+ @Nonnull long transactionLogIndex)
Review comment:
NIT: primitive type, we dont need @nonNull
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] hanishakoneru commented on issue #428: HDDS-2868.
Add ObjectID and UpdateID to OMKeyInfo.
Posted by GitBox <gi...@apache.org>.
hanishakoneru commented on issue #428: HDDS-2868. Add ObjectID and UpdateID to OMKeyInfo.
URL: https://github.com/apache/hadoop-ozone/pull/428#issuecomment-574894026
Thank you @bharatviswa504 and @arp7 for the reviews and merging the PR.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] bharatviswa504 commented on a change in pull request
#428: HDDS-2868. Add ObjectID and UpdateID to OMKeyInfo.
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on a change in pull request #428: HDDS-2868. Add ObjectID and UpdateID to OMKeyInfo.
URL: https://github.com/apache/hadoop-ozone/pull/428#discussion_r366011437
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java
##########
@@ -943,9 +943,13 @@ private OmMultipartInfo createMultipartInfo(OmKeyArgs keyArgs,
long currentTime = Time.now();
Map<Integer, PartKeyInfo> partKeyInfoMap = new HashMap<>();
- OmMultipartKeyInfo multipartKeyInfo = new OmMultipartKeyInfo(
- multipartUploadID, currentTime, keyArgs.getType(),
- keyArgs.getFactor(), partKeyInfoMap);
+ OmMultipartKeyInfo multipartKeyInfo = new OmMultipartKeyInfo.Builder()
Review comment:
No @arp7. For write requests new OM HA code is used.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] bharatviswa504 commented on a change in pull request
#428: HDDS-2868. Add ObjectID and UpdateID to OMKeyInfo.
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on a change in pull request #428: HDDS-2868. Add ObjectID and UpdateID to OMKeyInfo.
URL: https://github.com/apache/hadoop-ozone/pull/428#discussion_r366092550
##########
File path: hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmMultipartKeyInfo.java
##########
@@ -35,22 +35,67 @@
private final ReplicationType replicationType;
private final ReplicationFactor replicationFactor;
private TreeMap<Integer, PartKeyInfo> partKeyInfoList;
+ private long objectID;
+ private long updateID;
/**
* Construct OmMultipartKeyInfo object which holds multipart upload
* information for a key.
*/
public OmMultipartKeyInfo(String id, long creationTime,
- ReplicationType replicationType,
- ReplicationFactor replicationFactor,
- Map<Integer, PartKeyInfo> list) {
+ ReplicationType replicationType, ReplicationFactor replicationFactor,
+ Map<Integer, PartKeyInfo> list, long objectID, long updateID) {
this.uploadID = id;
this.creationTime = creationTime;
this.replicationType = replicationType;
this.replicationFactor = replicationFactor;
this.partKeyInfoList = new TreeMap<>(list);
+ this.objectID = objectID;
+ this.updateID = updateID;
}
+ /**
+ * Set the Object ID. If this value is already set then this function throws.
+ * There is a reason why we cannot use the final here. The
+ * OMMultipartKeyInfo is deserialized from the protobuf in many places in
+ * code. We need to set this object ID, after it is deserialized.
+ *
+ * @param obId - long
+ */
+ public void setObjectID(long obId) {
Review comment:
This method is not called any where, as we use builder methods.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] hanishakoneru commented on a change in pull request
#428: HDDS-2868. Add ObjectID and UpdateID to OMKeyInfo.
Posted by GitBox <gi...@apache.org>.
hanishakoneru commented on a change in pull request #428: HDDS-2868. Add ObjectID and UpdateID to OMKeyInfo.
URL: https://github.com/apache/hadoop-ozone/pull/428#discussion_r366533057
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMDirectoryCreateRequest.java
##########
@@ -210,7 +210,7 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
private OmKeyInfo createDirectoryKeyInfo(OzoneManager ozoneManager,
OmBucketInfo omBucketInfo, String volumeName, String bucketName,
- String keyName, KeyArgs keyArgs)
+ String keyName, KeyArgs keyArgs, long transactionLogIndex)
Review comment:
No Arpit. Transaction log index is the ratis log index. It is monotonically increasing always.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] hanishakoneru commented on a change in pull request
#428: HDDS-2868. Add ObjectID and UpdateID to OMKeyInfo.
Posted by GitBox <gi...@apache.org>.
hanishakoneru commented on a change in pull request #428: HDDS-2868. Add ObjectID and UpdateID to OMKeyInfo.
URL: https://github.com/apache/hadoop-ozone/pull/428#discussion_r366546340
##########
File path: hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmMultipartKeyInfo.java
##########
@@ -83,6 +128,72 @@ public ReplicationFactor getReplicationFactor() {
return replicationFactor;
}
+ /**
+ * Builder of OmKeyInfo.
Review comment:
Done.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] hanishakoneru commented on a change in pull request
#428: HDDS-2868. Add ObjectID and UpdateID to OMKeyInfo.
Posted by GitBox <gi...@apache.org>.
hanishakoneru commented on a change in pull request #428: HDDS-2868. Add ObjectID and UpdateID to OMKeyInfo.
URL: https://github.com/apache/hadoop-ozone/pull/428#discussion_r366532636
##########
File path: hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmKeyInfo.java
##########
@@ -52,6 +52,9 @@
private HddsProtos.ReplicationType type;
private HddsProtos.ReplicationFactor factor;
private FileEncryptionInfo encInfo;
+ private long objectID;
Review comment:
If there are any old objects in the DB, then those will have objectId and updateId as 0. It is not invalid but if it is 0, then we cannot determine if a corresponding transaction is a replay.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] hanishakoneru commented on a change in pull request
#428: HDDS-2868. Add ObjectID and UpdateID to OMKeyInfo.
Posted by GitBox <gi...@apache.org>.
hanishakoneru commented on a change in pull request #428: HDDS-2868. Add ObjectID and UpdateID to OMKeyInfo.
URL: https://github.com/apache/hadoop-ozone/pull/428#discussion_r365400428
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/s3/multipart/S3MultipartUploadAbortRequest.java
##########
@@ -120,6 +124,21 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
multipartKeyInfo = omMetadataManager
.getMultipartInfoTable().get(multipartKey);
+ // Set updateID to current transactionLogIndex for all parts
+ TreeMap<Integer, PartKeyInfo> partKeyInfoMap = multipartKeyInfo
+ .getPartKeyInfoMap();
+ for (Map.Entry<Integer, PartKeyInfo> partKeyInfoEntry :
+ partKeyInfoMap.entrySet()) {
+ PartKeyInfo partKeyInfo = partKeyInfoEntry.getValue();
+ OmKeyInfo currentKeyPartInfo = OmKeyInfo.getFromProtobuf(
Review comment:
Yes, but this entry is added to DeletedTable. So I thought we should keep a record of the transaction which deleted the Key.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] bharatviswa504 commented on a change in pull request
#428: HDDS-2868. Add ObjectID and UpdateID to OMKeyInfo.
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on a change in pull request #428: HDDS-2868. Add ObjectID and UpdateID to OMKeyInfo.
URL: https://github.com/apache/hadoop-ozone/pull/428#discussion_r366079870
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRenameRequest.java
##########
@@ -155,6 +156,9 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
//Set modification time
fromKeyValue.setModificationTime(renameKeyArgs.getModificationTime());
+ // Set the UpdateID to current transactionLogIndex
+ fromKeyValue.setUpdateID(transactionLogIndex);
Review comment:
As this is like new key is getting created, so do we need to set objectID also here?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] bharatviswa504 commented on a change in pull request
#428: HDDS-2868. Add ObjectID and UpdateID to OMKeyInfo.
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on a change in pull request #428: HDDS-2868. Add ObjectID and UpdateID to OMKeyInfo.
URL: https://github.com/apache/hadoop-ozone/pull/428#discussion_r366009210
##########
File path: hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmMultipartKeyInfo.java
##########
@@ -83,6 +128,72 @@ public ReplicationFactor getReplicationFactor() {
return replicationFactor;
}
+ /**
+ * Builder of OmKeyInfo.
Review comment:
Minor NIT: OmKeyInfo -> OmMultipartKeyInfo
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] hanishakoneru commented on a change in pull request
#428: HDDS-2868. Add ObjectID and UpdateID to OMKeyInfo.
Posted by GitBox <gi...@apache.org>.
hanishakoneru commented on a change in pull request #428: HDDS-2868. Add ObjectID and UpdateID to OMKeyInfo.
URL: https://github.com/apache/hadoop-ozone/pull/428#discussion_r366546111
##########
File path: hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmKeyInfo.java
##########
@@ -135,6 +141,47 @@ public void updateModifcationTime() {
this.modificationTime = Time.monotonicNow();
}
+ /**
+ * Set the Object ID. If this value is already set then this function throws.
+ * There is a reason why we cannot use the final here. The OMKeyInfo is
+ * deserialized from the protobuf in many places in code. We need to set
+ * this object ID, after it is deserialized.
+ *
+ * @param obId - long
+ */
+ public void setObjectID(long obId) {
+ if(this.objectID != 0) {
Review comment:
Removed
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] hanishakoneru commented on a change in pull request
#428: HDDS-2868. Add ObjectID and UpdateID to OMKeyInfo.
Posted by GitBox <gi...@apache.org>.
hanishakoneru commented on a change in pull request #428: HDDS-2868. Add ObjectID and UpdateID to OMKeyInfo.
URL: https://github.com/apache/hadoop-ozone/pull/428#discussion_r365400759
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRequest.java
##########
@@ -416,12 +419,14 @@ protected OmKeyInfo prepareKeyInfo(
@Nonnull KeyArgs keyArgs, @Nonnull String dbKeyName, long size,
@Nonnull List<OmKeyLocationInfo> locations,
@Nullable FileEncryptionInfo encInfo,
- @Nonnull PrefixManager prefixManager, @Nullable OmBucketInfo omBucketInfo)
+ @Nonnull PrefixManager prefixManager,
+ @Nullable OmBucketInfo omBucketInfo,
+ @Nonnull long transactionLogIndex)
Review comment:
done
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] hanishakoneru commented on a change in pull request
#428: HDDS-2868. Add ObjectID and UpdateID to OMKeyInfo.
Posted by GitBox <gi...@apache.org>.
hanishakoneru commented on a change in pull request #428: HDDS-2868. Add ObjectID and UpdateID to OMKeyInfo.
URL: https://github.com/apache/hadoop-ozone/pull/428#discussion_r366546500
##########
File path: hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmKeyInfo.java
##########
@@ -135,6 +141,47 @@ public void updateModifcationTime() {
this.modificationTime = Time.monotonicNow();
}
+ /**
+ * Set the Object ID. If this value is already set then this function throws.
+ * There is a reason why we cannot use the final here. The OMKeyInfo is
+ * deserialized from the protobuf in many places in code. We need to set
+ * this object ID, after it is deserialized.
+ *
+ * @param obId - long
+ */
+ public void setObjectID(long obId) {
+ if(this.objectID != 0) {
+ throw new UnsupportedOperationException("Attempt to modify object ID " +
+ "which is not zero. Current Object ID is " + this.objectID);
+ }
+ this.objectID = obId;
+ }
+
+ /**
+ * Sets the update ID. For each modification of this object, we will set
+ * this to a value greater than the current value.
+ * @param updateID long
+ */
+ public void setUpdateID(long updateID) {
+ this.updateID = updateID;
Review comment:
Added the check.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] bharatviswa504 commented on a change in pull request
#428: HDDS-2868. Add ObjectID and UpdateID to OMKeyInfo.
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on a change in pull request #428: HDDS-2868. Add ObjectID and UpdateID to OMKeyInfo.
URL: https://github.com/apache/hadoop-ozone/pull/428#discussion_r365092128
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRequest.java
##########
@@ -349,7 +349,8 @@ protected OmKeyInfo createKeyInfo(@Nonnull KeyArgs keyArgs,
@Nonnull HddsProtos.ReplicationType type, long size,
@Nullable FileEncryptionInfo encInfo,
@Nonnull PrefixManager prefixManager,
- @Nullable OmBucketInfo omBucketInfo) {
+ @Nullable OmBucketInfo omBucketInfo,
+ @Nonnull long transactionLogIndex) {
Review comment:
NIT: primitive type, we dont need @NonNull
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] prashantpogde commented on a change in pull request
#428: HDDS-2868. Add ObjectID and UpdateID to OMKeyInfo.
Posted by GitBox <gi...@apache.org>.
prashantpogde commented on a change in pull request #428: HDDS-2868. Add ObjectID and UpdateID to OMKeyInfo.
URL: https://github.com/apache/hadoop-ozone/pull/428#discussion_r372191805
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMDirectoryCreateRequest.java
##########
@@ -210,7 +210,7 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
private OmKeyInfo createDirectoryKeyInfo(OzoneManager ozoneManager,
OmBucketInfo omBucketInfo, String volumeName, String bucketName,
- String keyName, KeyArgs keyArgs)
+ String keyName, KeyArgs keyArgs, long transactionLogIndex)
Review comment:
Can transactionLogIndex be used as a unique objectId over time ?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] hanishakoneru commented on a change in pull request
#428: HDDS-2868. Add ObjectID and UpdateID to OMKeyInfo.
Posted by GitBox <gi...@apache.org>.
hanishakoneru commented on a change in pull request #428: HDDS-2868. Add ObjectID and UpdateID to OMKeyInfo.
URL: https://github.com/apache/hadoop-ozone/pull/428#discussion_r366546400
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRenameRequest.java
##########
@@ -112,6 +112,7 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
OMMetadataManager omMetadataManager = ozoneManager.getMetadataManager();
boolean acquiredLock = false;
OMClientResponse omClientResponse = null;
+ boolean success = true;
Review comment:
Done.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] hanishakoneru commented on a change in pull request
#428: HDDS-2868. Add ObjectID and UpdateID to OMKeyInfo.
Posted by GitBox <gi...@apache.org>.
hanishakoneru commented on a change in pull request #428: HDDS-2868. Add ObjectID and UpdateID to OMKeyInfo.
URL: https://github.com/apache/hadoop-ozone/pull/428#discussion_r365400868
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/s3/multipart/S3InitiateMultipartUploadRequest.java
##########
@@ -158,9 +158,10 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
.setOmKeyLocationInfos(Collections.singletonList(
new OmKeyLocationInfoGroup(0, new ArrayList<>())))
.setAcls(OzoneAclUtil.fromProtobuf(keyArgs.getAclsList()))
+ .setObjectID(transactionLogIndex)
Review comment:
Thanks for catching this. Updated the patch.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] bharatviswa504 commented on issue #428: HDDS-2868.
Add ObjectID and UpdateID to OMKeyInfo.
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on issue #428: HDDS-2868. Add ObjectID and UpdateID to OMKeyInfo.
URL: https://github.com/apache/hadoop-ozone/pull/428#issuecomment-574869324
Thank You @hanishakoneru for the contribution and @arp7 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] bharatviswa504 commented on a change in pull request
#428: HDDS-2868. Add ObjectID and UpdateID to OMKeyInfo.
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on a change in pull request #428: HDDS-2868. Add ObjectID and UpdateID to OMKeyInfo.
URL: https://github.com/apache/hadoop-ozone/pull/428#discussion_r366011437
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java
##########
@@ -943,9 +943,13 @@ private OmMultipartInfo createMultipartInfo(OmKeyArgs keyArgs,
long currentTime = Time.now();
Map<Integer, PartKeyInfo> partKeyInfoMap = new HashMap<>();
- OmMultipartKeyInfo multipartKeyInfo = new OmMultipartKeyInfo(
- multipartUploadID, currentTime, keyArgs.getType(),
- keyArgs.getFactor(), partKeyInfoMap);
+ OmMultipartKeyInfo multipartKeyInfo = new OmMultipartKeyInfo.Builder()
Review comment:
No @arp7. For write requests new OM HA code is used. (The new code is used for both OM HA and non-HA)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] hanishakoneru commented on issue #428: HDDS-2868.
Add ObjectID and UpdateID to OMKeyInfo.
Posted by GitBox <gi...@apache.org>.
hanishakoneru commented on issue #428: HDDS-2868. Add ObjectID and UpdateID to OMKeyInfo.
URL: https://github.com/apache/hadoop-ozone/pull/428#issuecomment-574349224
Thank you @bharatviswa504 and @arp7 for the reviews. I have addressed the comments.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] hanishakoneru commented on a change in pull request
#428: HDDS-2868. Add ObjectID and UpdateID to OMKeyInfo.
Posted by GitBox <gi...@apache.org>.
hanishakoneru commented on a change in pull request #428: HDDS-2868. Add ObjectID and UpdateID to OMKeyInfo.
URL: https://github.com/apache/hadoop-ozone/pull/428#discussion_r366542253
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyDeleteRequest.java
##########
@@ -126,6 +126,8 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
if (omKeyInfo == null) {
throw new OMException("Key not found", KEY_NOT_FOUND);
}
+ // Set the UpdateID to current transactionLogIndex
+ omKeyInfo.setUpdateID(transactionLogIndex);
Review comment:
Deleted Keys are put in DeletedKey Table and then purged when we get KeyPurgeRequest. As the purge request could also be a replay, we should only delete the keys which were deleted before the purge request. For this check, we need to set the updateID here.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] bharatviswa504 commented on a change in pull request
#428: HDDS-2868. Add ObjectID and UpdateID to OMKeyInfo.
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on a change in pull request #428: HDDS-2868. Add ObjectID and UpdateID to OMKeyInfo.
URL: https://github.com/apache/hadoop-ozone/pull/428#discussion_r366091719
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/s3/multipart/S3MultipartUploadCompleteRequest.java
##########
@@ -229,17 +229,33 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
// This is a newly added key, it does not have any versions.
OmKeyLocationInfoGroup keyLocationInfoGroup = new
OmKeyLocationInfoGroup(0, partLocationInfos);
+
+ // Get the objectID of the key from OpenKeyTable
+ OmKeyInfo dbOpenKeyInfo = omMetadataManager.getOpenKeyTable()
+ .get(multipartKey);
+
// A newly created key, this is the first version.
- omKeyInfo = new OmKeyInfo.Builder().setVolumeName(volumeName)
+ OmKeyInfo.Builder builder =
+ new OmKeyInfo.Builder().setVolumeName(volumeName)
.setBucketName(bucketName).setKeyName(keyName)
.setReplicationFactor(factor).setReplicationType(type)
.setCreationTime(keyArgs.getModificationTime())
.setModificationTime(keyArgs.getModificationTime())
.setDataSize(dataSize)
.setOmKeyLocationInfos(
Collections.singletonList(keyLocationInfoGroup))
- .setAcls(OzoneAclUtil.fromProtobuf(keyArgs.getAclsList()))
- .build();
+ .setAcls(OzoneAclUtil.fromProtobuf(keyArgs.getAclsList()));
+ // Check if db entry has ObjectID. This check is required because
+ // it is possible that between multipart key uploads and complete,
+ // we had an upgrade.
+ // TODO: If on upgrades we do not consider the non-completed multipart
+ // uploads, then this check can be removed.
+ if (dbOpenKeyInfo.getObjectID() != 0) {
Review comment:
As discussed offline, for this we can set ObjectID, which we have in the dbOpenKeyInfo.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] arp7 commented on a change in pull request #428:
HDDS-2868. Add ObjectID and UpdateID to OMKeyInfo.
Posted by GitBox <gi...@apache.org>.
arp7 commented on a change in pull request #428: HDDS-2868. Add ObjectID and UpdateID to OMKeyInfo.
URL: https://github.com/apache/hadoop-ozone/pull/428#discussion_r365629921
##########
File path: hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmKeyInfo.java
##########
@@ -135,6 +141,47 @@ public void updateModifcationTime() {
this.modificationTime = Time.monotonicNow();
}
+ /**
+ * Set the Object ID. If this value is already set then this function throws.
+ * There is a reason why we cannot use the final here. The OMKeyInfo is
+ * deserialized from the protobuf in many places in code. We need to set
+ * this object ID, after it is deserialized.
+ *
+ * @param obId - long
+ */
+ public void setObjectID(long obId) {
+ if(this.objectID != 0) {
+ throw new UnsupportedOperationException("Attempt to modify object ID " +
+ "which is not zero. Current Object ID is " + this.objectID);
+ }
+ this.objectID = obId;
+ }
+
+ /**
+ * Sets the update ID. For each modification of this object, we will set
+ * this to a value greater than the current value.
+ * @param updateID long
+ */
+ public void setUpdateID(long updateID) {
+ this.updateID = updateID;
Review comment:
assert that it is greater than current updateID?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] bharatviswa504 commented on a change in pull request
#428: HDDS-2868. Add ObjectID and UpdateID to OMKeyInfo.
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on a change in pull request #428: HDDS-2868. Add ObjectID and UpdateID to OMKeyInfo.
URL: https://github.com/apache/hadoop-ozone/pull/428#discussion_r366028051
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyDeleteRequest.java
##########
@@ -126,6 +126,8 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
if (omKeyInfo == null) {
throw new OMException("Key not found", KEY_NOT_FOUND);
}
+ // Set the UpdateID to current transactionLogIndex
+ omKeyInfo.setUpdateID(transactionLogIndex);
Review comment:
I think, we don't need to set updateID, as we delete this entry from KeyTable anyway. (As we set Optional.absent() below, which will be deleted from OM DB eventually when double buffer picks). Or is there any scenario by setting this helps.
Example: Create key Key1 - 1, Delete Key Key1 - 2 Create Key Key1 (again) - 3 Now replay happening from 1, as now in KeyTable it will have updateID 3, we think it as replay just by checking OmKeyInfo.getUpdateID, similar to Bucket/Volume delete.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] bharatviswa504 commented on a change in pull request
#428: HDDS-2868. Add ObjectID and UpdateID to OMKeyInfo.
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on a change in pull request #428: HDDS-2868. Add ObjectID and UpdateID to OMKeyInfo.
URL: https://github.com/apache/hadoop-ozone/pull/428#discussion_r365093345
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/s3/multipart/S3InitiateMultipartUploadRequest.java
##########
@@ -158,9 +158,10 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
.setOmKeyLocationInfos(Collections.singletonList(
new OmKeyLocationInfoGroup(0, new ArrayList<>())))
.setAcls(OzoneAclUtil.fromProtobuf(keyArgs.getAclsList()))
+ .setObjectID(transactionLogIndex)
Review comment:
Missing to add objectID and updateID in multipartKeyInfo
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] hanishakoneru commented on a change in pull request
#428: HDDS-2868. Add ObjectID and UpdateID to OMKeyInfo.
Posted by GitBox <gi...@apache.org>.
hanishakoneru commented on a change in pull request #428: HDDS-2868. Add ObjectID and UpdateID to OMKeyInfo.
URL: https://github.com/apache/hadoop-ozone/pull/428#discussion_r366533447
##########
File path: hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmKeyInfo.java
##########
@@ -52,6 +52,9 @@
private HddsProtos.ReplicationType type;
private HddsProtos.ReplicationFactor factor;
private FileEncryptionInfo encInfo;
+ private long objectID;
+ private long updateID;
+
Review comment:
It will default to 0 as we use long primitive type here.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] bharatviswa504 commented on a change in pull request
#428: HDDS-2868. Add ObjectID and UpdateID to OMKeyInfo.
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on a change in pull request #428: HDDS-2868. Add ObjectID and UpdateID to OMKeyInfo.
URL: https://github.com/apache/hadoop-ozone/pull/428#discussion_r366092784
##########
File path: hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmKeyInfo.java
##########
@@ -135,6 +141,47 @@ public void updateModifcationTime() {
this.modificationTime = Time.monotonicNow();
}
+ /**
+ * Set the Object ID. If this value is already set then this function throws.
+ * There is a reason why we cannot use the final here. The OMKeyInfo is
+ * deserialized from the protobuf in many places in code. We need to set
+ * this object ID, after it is deserialized.
+ *
+ * @param obId - long
+ */
+ public void setObjectID(long obId) {
+ if(this.objectID != 0) {
Review comment:
This method is not called anywhere, as we use builder methods.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] bharatviswa504 commented on a change in pull request
#428: HDDS-2868. Add ObjectID and UpdateID to OMKeyInfo.
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on a change in pull request #428: HDDS-2868. Add ObjectID and UpdateID to OMKeyInfo.
URL: https://github.com/apache/hadoop-ozone/pull/428#discussion_r367121158
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRenameRequest.java
##########
@@ -155,6 +156,9 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
//Set modification time
fromKeyValue.setModificationTime(renameKeyArgs.getModificationTime());
+ // Set the UpdateID to current transactionLogIndex
+ fromKeyValue.setUpdateID(transactionLogIndex);
Review comment:
As discussed offline, we can change this when we use them in finding a transaction is replay or not.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] arp7 commented on a change in pull request #428:
HDDS-2868. Add ObjectID and UpdateID to OMKeyInfo.
Posted by GitBox <gi...@apache.org>.
arp7 commented on a change in pull request #428: HDDS-2868. Add ObjectID and UpdateID to OMKeyInfo.
URL: https://github.com/apache/hadoop-ozone/pull/428#discussion_r365630503
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java
##########
@@ -943,9 +943,13 @@ private OmMultipartInfo createMultipartInfo(OmKeyArgs keyArgs,
long currentTime = Time.now();
Map<Integer, PartKeyInfo> partKeyInfoMap = new HashMap<>();
- OmMultipartKeyInfo multipartKeyInfo = new OmMultipartKeyInfo(
- multipartUploadID, currentTime, keyArgs.getType(),
- keyArgs.getFactor(), partKeyInfoMap);
+ OmMultipartKeyInfo multipartKeyInfo = new OmMultipartKeyInfo.Builder()
Review comment:
Is this code still used? @bharatviswa504
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] arp7 commented on a change in pull request #428:
HDDS-2868. Add ObjectID and UpdateID to OMKeyInfo.
Posted by GitBox <gi...@apache.org>.
arp7 commented on a change in pull request #428: HDDS-2868. Add ObjectID and UpdateID to OMKeyInfo.
URL: https://github.com/apache/hadoop-ozone/pull/428#discussion_r367117592
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java
##########
@@ -943,9 +943,13 @@ private OmMultipartInfo createMultipartInfo(OmKeyArgs keyArgs,
long currentTime = Time.now();
Map<Integer, PartKeyInfo> partKeyInfoMap = new HashMap<>();
- OmMultipartKeyInfo multipartKeyInfo = new OmMultipartKeyInfo(
- multipartUploadID, currentTime, keyArgs.getType(),
- keyArgs.getFactor(), partKeyInfoMap);
+ OmMultipartKeyInfo multipartKeyInfo = new OmMultipartKeyInfo.Builder()
Review comment:
Should we file a separate jira to delete this code 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] arp7 commented on a change in pull request #428:
HDDS-2868. Add ObjectID and UpdateID to OMKeyInfo.
Posted by GitBox <gi...@apache.org>.
arp7 commented on a change in pull request #428: HDDS-2868. Add ObjectID and UpdateID to OMKeyInfo.
URL: https://github.com/apache/hadoop-ozone/pull/428#discussion_r365630562
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMDirectoryCreateRequest.java
##########
@@ -210,7 +210,7 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
private OmKeyInfo createDirectoryKeyInfo(OzoneManager ozoneManager,
OmBucketInfo omBucketInfo, String volumeName, String bucketName,
- String keyName, KeyArgs keyArgs)
+ String keyName, KeyArgs keyArgs, long transactionLogIndex)
Review comment:
Is the transaction log index reset when the Ratis term changes?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] bharatviswa504 commented on a change in pull request
#428: HDDS-2868. Add ObjectID and UpdateID to OMKeyInfo.
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on a change in pull request #428: HDDS-2868. Add ObjectID and UpdateID to OMKeyInfo.
URL: https://github.com/apache/hadoop-ozone/pull/428#discussion_r365093741
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/s3/multipart/S3MultipartUploadAbortRequest.java
##########
@@ -120,6 +124,21 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
multipartKeyInfo = omMetadataManager
.getMultipartInfoTable().get(multipartKey);
+ // Set updateID to current transactionLogIndex for all parts
+ TreeMap<Integer, PartKeyInfo> partKeyInfoMap = multipartKeyInfo
+ .getPartKeyInfoMap();
+ for (Map.Entry<Integer, PartKeyInfo> partKeyInfoEntry :
+ partKeyInfoMap.entrySet()) {
+ PartKeyInfo partKeyInfo = partKeyInfoEntry.getValue();
+ OmKeyInfo currentKeyPartInfo = OmKeyInfo.getFromProtobuf(
Review comment:
Not understood the reason of setting updateID here, as below we are setting Optional.absent(), so it means entry is deleted from DB.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] hanishakoneru commented on a change in pull request
#428: HDDS-2868. Add ObjectID and UpdateID to OMKeyInfo.
Posted by GitBox <gi...@apache.org>.
hanishakoneru commented on a change in pull request #428: HDDS-2868. Add ObjectID and UpdateID to OMKeyInfo.
URL: https://github.com/apache/hadoop-ozone/pull/428#discussion_r366544023
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRenameRequest.java
##########
@@ -155,6 +156,9 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
//Set modification time
fromKeyValue.setModificationTime(renameKeyArgs.getModificationTime());
+ // Set the UpdateID to current transactionLogIndex
+ fromKeyValue.setUpdateID(transactionLogIndex);
Review comment:
I think either way should be ok. I thought since its the same key being renamed, we could have the same objectID. But I am ok changing it to current transactionLogIndex. Let me know what you think.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] bharatviswa504 merged pull request #428: HDDS-2868.
Add ObjectID and UpdateID to OMKeyInfo.
Posted by GitBox <gi...@apache.org>.
bharatviswa504 merged pull request #428: HDDS-2868. Add ObjectID and UpdateID to OMKeyInfo.
URL: https://github.com/apache/hadoop-ozone/pull/428
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] hanishakoneru commented on a change in pull request
#428: HDDS-2868. Add ObjectID and UpdateID to OMKeyInfo.
Posted by GitBox <gi...@apache.org>.
hanishakoneru commented on a change in pull request #428: HDDS-2868. Add ObjectID and UpdateID to OMKeyInfo.
URL: https://github.com/apache/hadoop-ozone/pull/428#discussion_r366546149
##########
File path: hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmMultipartKeyInfo.java
##########
@@ -35,22 +35,67 @@
private final ReplicationType replicationType;
private final ReplicationFactor replicationFactor;
private TreeMap<Integer, PartKeyInfo> partKeyInfoList;
+ private long objectID;
+ private long updateID;
/**
* Construct OmMultipartKeyInfo object which holds multipart upload
* information for a key.
*/
public OmMultipartKeyInfo(String id, long creationTime,
- ReplicationType replicationType,
- ReplicationFactor replicationFactor,
- Map<Integer, PartKeyInfo> list) {
+ ReplicationType replicationType, ReplicationFactor replicationFactor,
+ Map<Integer, PartKeyInfo> list, long objectID, long updateID) {
this.uploadID = id;
this.creationTime = creationTime;
this.replicationType = replicationType;
this.replicationFactor = replicationFactor;
this.partKeyInfoList = new TreeMap<>(list);
+ this.objectID = objectID;
+ this.updateID = updateID;
}
+ /**
+ * Set the Object ID. If this value is already set then this function throws.
+ * There is a reason why we cannot use the final here. The
+ * OMMultipartKeyInfo is deserialized from the protobuf in many places in
+ * code. We need to set this object ID, after it is deserialized.
+ *
+ * @param obId - long
+ */
+ public void setObjectID(long obId) {
Review comment:
Removed.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] hanishakoneru commented on a change in pull request
#428: HDDS-2868. Add ObjectID and UpdateID to OMKeyInfo.
Posted by GitBox <gi...@apache.org>.
hanishakoneru commented on a change in pull request #428: HDDS-2868. Add ObjectID and UpdateID to OMKeyInfo.
URL: https://github.com/apache/hadoop-ozone/pull/428#discussion_r365400719
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRequest.java
##########
@@ -349,7 +349,8 @@ protected OmKeyInfo createKeyInfo(@Nonnull KeyArgs keyArgs,
@Nonnull HddsProtos.ReplicationType type, long size,
@Nullable FileEncryptionInfo encInfo,
@Nonnull PrefixManager prefixManager,
- @Nullable OmBucketInfo omBucketInfo) {
+ @Nullable OmBucketInfo omBucketInfo,
+ @Nonnull long transactionLogIndex) {
Review comment:
done
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] arp7 commented on a change in pull request #428:
HDDS-2868. Add ObjectID and UpdateID to OMKeyInfo.
Posted by GitBox <gi...@apache.org>.
arp7 commented on a change in pull request #428: HDDS-2868. Add ObjectID and UpdateID to OMKeyInfo.
URL: https://github.com/apache/hadoop-ozone/pull/428#discussion_r365629947
##########
File path: hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmKeyInfo.java
##########
@@ -52,6 +52,9 @@
private HddsProtos.ReplicationType type;
private HddsProtos.ReplicationFactor factor;
private FileEncryptionInfo encInfo;
+ private long objectID;
Review comment:
Is 0 an invalid object ID and update id?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] bharatviswa504 commented on a change in pull request
#428: HDDS-2868. Add ObjectID and UpdateID to OMKeyInfo.
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on a change in pull request #428: HDDS-2868. Add ObjectID and UpdateID to OMKeyInfo.
URL: https://github.com/apache/hadoop-ozone/pull/428#discussion_r366030312
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/s3/multipart/S3MultipartUploadAbortRequest.java
##########
@@ -120,6 +124,7 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
multipartKeyInfo = omMetadataManager
.getMultipartInfoTable().get(multipartKey);
+ multipartKeyInfo.setUpdateID(transactionLogIndex);
Review comment:
Same as above, I think we don't need to set updateID here, as anyway we are removing entry from multipartInfo table. Is there is any use case that will help in the detection of replay by setting here?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org