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 2019/12/19 21:02:34 UTC
[GitHub] [hadoop-ozone] hanishakoneru opened a new pull request #381: Hdds
2781. Add ObjectID and updateID to BucketInfo to avoid replaying transactions
hanishakoneru opened a new pull request #381: Hdds 2781. Add ObjectID and updateID to BucketInfo to avoid replaying transactions
URL: https://github.com/apache/hadoop-ozone/pull/381
## What changes were proposed in this pull request?
This Jira has 2 objectives:
1. Add objectID and updateID to BucketInfo proto persisted to DB.
2. To ensure that bucket operations are idempotent, compare the transactionID with the objectID and updateID to make sure that the transaction is not a replay. If the transactionID <= updateID, then it implies that the transaction is a replay and hence it should be skipped.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-2781
## How was this patch tested?
Will add unit tests in the another commit.
----------------------------------------------------------------
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
#381: HDDS-2781. Add ObjectID and updateID to BucketInfo to avoid replaying
transactions
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on a change in pull request #381: HDDS-2781. Add ObjectID and updateID to BucketInfo to avoid replaying transactions
URL: https://github.com/apache/hadoop-ozone/pull/381#discussion_r361206564
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketDeleteRequest.java
##########
@@ -135,12 +151,16 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
DeleteBucketResponse.newBuilder().build());
// Add to double buffer.
- omClientResponse = new OMBucketDeleteResponse(volumeName, bucketName,
- omResponse.build());
+ omClientResponse = new OMBucketDeleteResponse(omResponse.build(),
+ volumeName, bucketName);
+ LOG.debug("Deleted bucket:{} in volume:{}", bucketName, volumeName);
} catch (IOException ex) {
+ omMetrics.incNumBucketDeleteFails();
+ LOG.error("Delete bucket failed for bucket:{} in volume:{}", bucketName,
Review comment:
Can we move back logging to before?
Why that is done like that way is, logging to be done outside the lock.
----------------------------------------------------------------
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
#381: HDDS-2781. Add ObjectID and updateID to BucketInfo to avoid replaying
transactions
Posted by GitBox <gi...@apache.org>.
hanishakoneru commented on a change in pull request #381: HDDS-2781. Add ObjectID and updateID to BucketInfo to avoid replaying transactions
URL: https://github.com/apache/hadoop-ozone/pull/381#discussion_r363965115
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketDeleteRequest.java
##########
@@ -135,12 +151,16 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
DeleteBucketResponse.newBuilder().build());
// Add to double buffer.
- omClientResponse = new OMBucketDeleteResponse(volumeName, bucketName,
- omResponse.build());
+ omClientResponse = new OMBucketDeleteResponse(omResponse.build(),
+ volumeName, bucketName);
+ LOG.debug("Deleted bucket:{} in volume:{}", bucketName, volumeName);
} catch (IOException ex) {
+ omMetrics.incNumBucketDeleteFails();
+ LOG.error("Delete bucket failed for bucket:{} in volume:{}", bucketName,
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
#381: HDDS-2781. Add ObjectID and updateID to BucketInfo to avoid replaying
transactions
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on a change in pull request #381: HDDS-2781. Add ObjectID and updateID to BucketInfo to avoid replaying transactions
URL: https://github.com/apache/hadoop-ozone/pull/381#discussion_r361206631
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketSetPropertyRequest.java
##########
@@ -168,12 +191,17 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
omResponse.setSetBucketPropertyResponse(
SetBucketPropertyResponse.newBuilder().build());
- omClientResponse =
- new OMBucketSetPropertyResponse(omBucketInfo, omResponse.build());
+ omClientResponse = new OMBucketSetPropertyResponse(
+ omResponse.build(), omBucketInfo);
+ LOG.debug("Setting bucket property for bucket:{} in volume:{}",
+ bucketName, volumeName);
} catch (IOException ex) {
+ LOG.error("Setting bucket property failed for bucket:{} in volume:{}",
Review comment:
Same here, can we move logging outside the lock.
----------------------------------------------------------------
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
#381: HDDS-2781. Add ObjectID and updateID to BucketInfo to avoid replaying
transactions
Posted by GitBox <gi...@apache.org>.
hanishakoneru commented on a change in pull request #381: HDDS-2781. Add ObjectID and updateID to BucketInfo to avoid replaying transactions
URL: https://github.com/apache/hadoop-ozone/pull/381#discussion_r364993872
##########
File path: hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmBucketInfo.java
##########
@@ -266,8 +266,6 @@ public static Builder newBuilder() {
auditMap.put(OzoneConsts.STORAGE_TYPE,
(this.storageType != null) ? this.storageType.name() : null);
auditMap.put(OzoneConsts.CREATION_TIME, String.valueOf(this.creationTime));
- auditMap.put(OzoneConsts.OBJECT_ID, String.valueOf(this.getObjectID()));
- auditMap.put(OzoneConsts.UPDATE_ID, String.valueOf(this.getUpdateID()));
Review comment:
@bharatviswa504 pointed out that audit logs are actually used for finding out what operations were performed on the cluster, as was discussed in some PR. And for debugging, we can use the debug logs if required.
I am ok to go either way.
----------------------------------------------------------------
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
#381: HDDS-2781. Add ObjectID and updateID to BucketInfo to avoid replaying
transactions
Posted by GitBox <gi...@apache.org>.
hanishakoneru commented on a change in pull request #381: HDDS-2781. Add ObjectID and updateID to BucketInfo to avoid replaying transactions
URL: https://github.com/apache/hadoop-ozone/pull/381#discussion_r363965094
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketSetPropertyRequest.java
##########
@@ -168,12 +191,17 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
omResponse.setSetBucketPropertyResponse(
SetBucketPropertyResponse.newBuilder().build());
- omClientResponse =
- new OMBucketSetPropertyResponse(omBucketInfo, omResponse.build());
+ omClientResponse = new OMBucketSetPropertyResponse(
+ omResponse.build(), omBucketInfo);
+ LOG.debug("Setting bucket property for bucket:{} in volume:{}",
+ bucketName, volumeName);
} catch (IOException ex) {
+ LOG.error("Setting bucket property failed for bucket:{} in volume:{}",
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] anuengineer commented on a change in pull request
#381: HDDS-2781. Add ObjectID and updateID to BucketInfo to avoid replaying
transactions
Posted by GitBox <gi...@apache.org>.
anuengineer commented on a change in pull request #381: HDDS-2781. Add ObjectID and updateID to BucketInfo to avoid replaying transactions
URL: https://github.com/apache/hadoop-ozone/pull/381#discussion_r364905128
##########
File path: hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmBucketInfo.java
##########
@@ -266,8 +266,6 @@ public static Builder newBuilder() {
auditMap.put(OzoneConsts.STORAGE_TYPE,
(this.storageType != null) ? this.storageType.name() : null);
auditMap.put(OzoneConsts.CREATION_TIME, String.valueOf(this.creationTime));
- auditMap.put(OzoneConsts.OBJECT_ID, String.valueOf(this.getObjectID()));
- auditMap.put(OzoneConsts.UPDATE_ID, String.valueOf(this.getUpdateID()));
Review comment:
I am in 2 minds about this change. This is useful if someone deletes and re-creates a bucket with same name it would be easy for us to identify that information. No need to fix in this, just curious why you thought this is a bad idea.
----------------------------------------------------------------
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 #381: HDDS-2781.
Add ObjectID and updateID to BucketInfo to avoid replaying transactions
Posted by GitBox <gi...@apache.org>.
hanishakoneru commented on issue #381: HDDS-2781. Add ObjectID and updateID to BucketInfo to avoid replaying transactions
URL: https://github.com/apache/hadoop-ozone/pull/381#issuecomment-571786694
Thanks for the review @bharatviswa504 . I have addressed your comments in the new commit.
----------------------------------------------------------------
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
#381: Hdds 2781. Add ObjectID and updateID to BucketInfo to avoid replaying
transactions
Posted by GitBox <gi...@apache.org>.
hanishakoneru commented on a change in pull request #381: Hdds 2781. Add ObjectID and updateID to BucketInfo to avoid replaying transactions
URL: https://github.com/apache/hadoop-ozone/pull/381#discussion_r360521506
##########
File path: hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmBucketInfo.java
##########
@@ -212,6 +266,8 @@ public static Builder newBuilder() {
auditMap.put(OzoneConsts.STORAGE_TYPE,
(this.storageType != null) ? this.storageType.name() : null);
auditMap.put(OzoneConsts.CREATION_TIME, String.valueOf(this.creationTime));
+ auditMap.put(OzoneConsts.OBJECT_ID, String.valueOf(this.getObjectID()));
Review comment:
Sure. I will remove this info from audit log.
----------------------------------------------------------------
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
#381: Hdds 2781. Add ObjectID and updateID to BucketInfo to avoid replaying
transactions
Posted by GitBox <gi...@apache.org>.
hanishakoneru commented on a change in pull request #381: Hdds 2781. Add ObjectID and updateID to BucketInfo to avoid replaying transactions
URL: https://github.com/apache/hadoop-ozone/pull/381#discussion_r360194597
##########
File path: hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmBucketInfo.java
##########
@@ -188,6 +218,30 @@ public BucketEncryptionKeyInfo getEncryptionKeyInfo() {
return bekInfo;
}
+ /**
+ * 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 OmVolumeArgs is
Review comment:
Yes. Thanks. Will fix it
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
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 #381: HDDS-2781.
Add ObjectID and updateID to BucketInfo to avoid replaying transactions
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on issue #381: HDDS-2781. Add ObjectID and updateID to BucketInfo to avoid replaying transactions
URL: https://github.com/apache/hadoop-ozone/pull/381#issuecomment-573836783
Thank You @hanishakoneru for the contribution and @anuengineer 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] anuengineer commented on a change in pull request
#381: HDDS-2781. Add ObjectID and updateID to BucketInfo to avoid replaying
transactions
Posted by GitBox <gi...@apache.org>.
anuengineer commented on a change in pull request #381: HDDS-2781. Add ObjectID and updateID to BucketInfo to avoid replaying transactions
URL: https://github.com/apache/hadoop-ozone/pull/381#discussion_r364905844
##########
File path: hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmBucketInfo.java
##########
@@ -266,8 +266,6 @@ public static Builder newBuilder() {
auditMap.put(OzoneConsts.STORAGE_TYPE,
(this.storageType != null) ? this.storageType.name() : null);
auditMap.put(OzoneConsts.CREATION_TIME, String.valueOf(this.creationTime));
- auditMap.put(OzoneConsts.OBJECT_ID, String.valueOf(this.getObjectID()));
- auditMap.put(OzoneConsts.UPDATE_ID, String.valueOf(this.getUpdateID()));
Review comment:
Does this break the Audit Parser code or Audit 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] hanishakoneru commented on issue #381: Hdds 2781.
Add ObjectID and updateID to BucketInfo to avoid replaying transactions
Posted by GitBox <gi...@apache.org>.
hanishakoneru commented on issue #381: Hdds 2781. Add ObjectID and updateID to BucketInfo to avoid replaying transactions
URL: https://github.com/apache/hadoop-ozone/pull/381#issuecomment-568042697
> I see for comments, it is marked as done/updated, but they are missing in the PR. Just letting you know about this.
Sorry I meant I fixed it and will post in the next commit. I am working on unit 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] bharatviswa504 commented on a change in pull request
#381: Hdds 2781. Add ObjectID and updateID to BucketInfo to avoid replaying
transactions
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on a change in pull request #381: Hdds 2781. Add ObjectID and updateID to BucketInfo to avoid replaying transactions
URL: https://github.com/apache/hadoop-ozone/pull/381#discussion_r360512195
##########
File path: hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmBucketInfo.java
##########
@@ -212,6 +266,8 @@ public static Builder newBuilder() {
auditMap.put(OzoneConsts.STORAGE_TYPE,
(this.storageType != null) ? this.storageType.name() : null);
auditMap.put(OzoneConsts.CREATION_TIME, String.valueOf(this.creationTime));
+ auditMap.put(OzoneConsts.OBJECT_ID, String.valueOf(this.getObjectID()));
Review comment:
Yaa agreed, but recently in one of PR a discussion came up that audit log is for to find information who did what operation, but not for debugging. So, if needed during debugging, we can log this information during performing the actual operation.
Having precise/required information in audit logs will help when flushing audit logs and also the size of the audit log. So, I feel having this information is not really required. Thoughts?
----------------------------------------------------------------
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 #381: HDDS-2781.
Add ObjectID and updateID to BucketInfo to avoid replaying transactions
Posted by GitBox <gi...@apache.org>.
bharatviswa504 merged pull request #381: HDDS-2781. Add ObjectID and updateID to BucketInfo to avoid replaying transactions
URL: https://github.com/apache/hadoop-ozone/pull/381
----------------------------------------------------------------
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