You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "swamirishi (via GitHub)" <gi...@apache.org> on 2023/09/07 22:28:20 UTC
[GitHub] [ozone] swamirishi opened a new pull request, #5258: HDDS-9236. Fix snapdiff output for key modification
swamirishi opened a new pull request, #5258:
URL: https://github.com/apache/ozone/pull/5258
## What changes were proposed in this pull request?
Currently snapdiff checks only key versions to check if a key has been modified. We have to check if other fields have also changed like (Acl changes, replication config changes etc.)
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-9236
Please replace this section with the link to the Apache JIRA)
Integration 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.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] hemantk-12 merged pull request #5258: HDDS-9236. Fix snapdiff output for key modification
Posted by "hemantk-12 (via GitHub)" <gi...@apache.org>.
hemantk-12 merged PR #5258:
URL: https://github.com/apache/ozone/pull/5258
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] hemantk-12 commented on a diff in pull request #5258: HDDS-9236. Fix snapdiff output for key modification
Posted by "hemantk-12 (via GitHub)" <gi...@apache.org>.
hemantk-12 commented on code in PR #5258:
URL: https://github.com/apache/ozone/pull/5258#discussion_r1321941976
##########
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmKeyInfo.java:
##########
@@ -760,6 +760,41 @@ public String getObjectInfo() {
'}';
}
+
+ public boolean isKeyInfoSame(OmKeyInfo omKeyInfo, boolean checkPath,
Review Comment:
I think `isKeyInfoSame` is very specific to snapshot diff and should be added to snapshot util.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] swamirishi commented on pull request #5258: HDDS-9236. Fix snapdiff output for key modification
Posted by "swamirishi (via GitHub)" <gi...@apache.org>.
swamirishi commented on PR #5258:
URL: https://github.com/apache/ozone/pull/5258#issuecomment-1711899483
@hemantk-12 @smengcl Please take a look at the 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.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] swamirishi commented on a diff in pull request #5258: HDDS-9236. Fix snapdiff output for key modification
Posted by "swamirishi (via GitHub)" <gi...@apache.org>.
swamirishi commented on code in PR #5258:
URL: https://github.com/apache/ozone/pull/5258#discussion_r1323356222
##########
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmKeyInfo.java:
##########
@@ -760,6 +760,41 @@ public String getObjectInfo() {
'}';
}
+
+ public boolean isKeyInfoSame(OmKeyInfo omKeyInfo, boolean checkPath,
Review Comment:
It is better this way to put in key info, if some field is added in omKeyInfo it should impact the equals method as well thus impacting snapshot diff.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] swamirishi commented on a diff in pull request #5258: HDDS-9236. Fix snapdiff output for key modification
Posted by "swamirishi (via GitHub)" <gi...@apache.org>.
swamirishi commented on code in PR #5258:
URL: https://github.com/apache/ozone/pull/5258#discussion_r1323356222
##########
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmKeyInfo.java:
##########
@@ -760,6 +760,41 @@ public String getObjectInfo() {
'}';
}
+
+ public boolean isKeyInfoSame(OmKeyInfo omKeyInfo, boolean checkPath,
Review Comment:
It is better this way to put in key info if some field is added in omKeyInfo it should impact the equals method as well thus impacting snapshot diff.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] smengcl commented on a diff in pull request #5258: HDDS-9236. Fix snapdiff output for key modification
Posted by "smengcl (via GitHub)" <gi...@apache.org>.
smengcl commented on code in PR #5258:
URL: https://github.com/apache/ozone/pull/5258#discussion_r1322008458
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/SnapshotDiffManager.java:
##########
@@ -1468,7 +1468,9 @@ private boolean isObjectModified(String fromObjectName, String toObjectName,
final WithObjectID fromObject = fromSnapshotTable.get(fromObjectName);
final WithObjectID toObject = toSnapshotTable.get(toObjectName);
if ((fromObject instanceof OmKeyInfo) && (toObject instanceof OmKeyInfo)) {
- return !SnapshotDeletingService.isBlockLocationInfoSame(
+ return !((OmKeyInfo) fromObject).isKeyInfoSame((OmKeyInfo) toObject,
+ false, false, false, false)
+ || !SnapshotDeletingService.isBlockLocationInfoSame(
(OmKeyInfo) fromObject, (OmKeyInfo) toObject);
Review Comment:
Ideally this should be one check for the key.
Would you wrap this in a reasonably-named helper method for readability?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] swamirishi commented on a diff in pull request #5258: HDDS-9236. Fix snapdiff output for key modification
Posted by "swamirishi (via GitHub)" <gi...@apache.org>.
swamirishi commented on code in PR #5258:
URL: https://github.com/apache/ozone/pull/5258#discussion_r1323355283
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/SnapshotDiffManager.java:
##########
@@ -1468,7 +1468,9 @@ private boolean isObjectModified(String fromObjectName, String toObjectName,
final WithObjectID fromObject = fromSnapshotTable.get(fromObjectName);
final WithObjectID toObject = toSnapshotTable.get(toObjectName);
if ((fromObject instanceof OmKeyInfo) && (toObject instanceof OmKeyInfo)) {
- return !SnapshotDeletingService.isBlockLocationInfoSame(
+ return !((OmKeyInfo) fromObject).isKeyInfoSame((OmKeyInfo) toObject,
+ false, false, false, false)
+ || !SnapshotDeletingService.isBlockLocationInfoSame(
(OmKeyInfo) fromObject, (OmKeyInfo) toObject);
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.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] hemantk-12 commented on pull request #5258: HDDS-9236. Fix snapdiff output for key modification
Posted by "hemantk-12 (via GitHub)" <gi...@apache.org>.
hemantk-12 commented on PR #5258:
URL: https://github.com/apache/ozone/pull/5258#issuecomment-1720055584
Thank @swamirishi for working on this and @smengcl for reviewing.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] hemantk-12 commented on a diff in pull request #5258: HDDS-9236. Fix snapdiff output for key modification
Posted by "hemantk-12 (via GitHub)" <gi...@apache.org>.
hemantk-12 commented on code in PR #5258:
URL: https://github.com/apache/ozone/pull/5258#discussion_r1324888504
##########
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmKeyInfo.java:
##########
@@ -760,6 +760,41 @@ public String getObjectInfo() {
'}';
}
+
+ public boolean isKeyInfoSame(OmKeyInfo omKeyInfo, boolean checkPath,
Review Comment:
Fair enough.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org