devmadhuu commented on code in PR #5043:
URL: https://github.com/apache/ozone/pull/5043#discussion_r1260552251
##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/tasks/FileSizeCountTask.java:
##########
@@ -158,50 +158,64 @@ public Pair<String, Boolean> process(OMUpdateEventBatch
events) {
String updatedKey = omdbUpdateEvent.getKey();
Object value = omdbUpdateEvent.getValue();
Object oldValue = omdbUpdateEvent.getOldValue();
+ // Check if values are instance of OmKeyInfo
+ if (!isOmKeyInfo(value)) {
+ continue;
+ }
+ OmKeyInfo omKeyInfo = (OmKeyInfo) value;
- if (value instanceof OmKeyInfo) {
- OmKeyInfo omKeyInfo = (OmKeyInfo) value;
- OmKeyInfo omKeyInfoOld = (OmKeyInfo) oldValue;
-
- try {
- switch (omdbUpdateEvent.getAction()) {
- case PUT:
- handlePutKeyEvent(omKeyInfo, fileSizeCountMap);
- break;
-
- case DELETE:
- handleDeleteKeyEvent(updatedKey, omKeyInfo, fileSizeCountMap);
- break;
+ try {
+ switch (omdbUpdateEvent.getAction()) {
+ case PUT:
+ handlePutKeyEvent(omKeyInfo, fileSizeCountMap);
+ break;
- case UPDATE:
- if (omKeyInfoOld != null) {
- handleDeleteKeyEvent(updatedKey, omKeyInfoOld, fileSizeCountMap);
- handlePutKeyEvent(omKeyInfo, fileSizeCountMap);
- } else {
- LOG.warn("Update event does not have the old keyInfo for {}.",
- updatedKey);
- }
- break;
+ case DELETE:
+ handleDeleteKeyEvent(updatedKey, omKeyInfo, fileSizeCountMap);
+ break;
- default:
- LOG.trace("Skipping DB update event : {}",
- omdbUpdateEvent.getAction());
+ case UPDATE:
+ if (oldValue != null && isOmKeyInfo(oldValue)) {
+ OmKeyInfo omKeyInfoOld = (OmKeyInfo) oldValue;
+ handleDeleteKeyEvent(updatedKey, omKeyInfoOld, fileSizeCountMap);
+ handlePutKeyEvent(omKeyInfo, fileSizeCountMap);
+ } else {
+ LOG.warn("Update event does not have the old keyInfo for {}.",
Review Comment:
Message may not be reflecting both conditions here which one didn't satisfy.
##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/tasks/ContainerKeyMapperTask.java:
##########
@@ -216,6 +216,10 @@ public Pair<String, Boolean> process(OMUpdateEventBatch
events) {
continue;
}
String updatedKey = omdbUpdateEvent.getKey();
+ if (!isOmKeyInfo(omdbUpdateEvent.getValue())) {
Review Comment:
you can combine this if condition with if condition at line 215 to avoid
line 218 execution.
##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/tasks/NSSummaryTaskWithLegacy.java:
##########
@@ -206,6 +206,22 @@ public boolean processWithLegacy(OMUpdateEventBatch
events) {
return true;
}
+ /**
+ * Checks if the object passed is an instance of `OmKeyInfo`.
+ *
+ * @param obj The object to check.
+ * @return True if the object is an instance of `OmKeyInfo`, false otherwise.
+ */
+ private boolean isOmKeyInfo(Object obj) {
Review Comment:
Can we move this method at common place ? We may not need this here or in
any class if we add a logic in
org.apache.hadoop.ozone.recon.tasks.OMDBUpdatesHandler#processEvent
##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/tasks/ContainerKeyMapperTask.java:
##########
@@ -263,6 +268,22 @@ public Pair<String, Boolean> process(OMUpdateEventBatch
events) {
return new ImmutablePair<>(getTaskName(), true);
}
+ /**
+ * Checks if the object passed is an instance of `OmKeyInfo`.
+ *
+ * @param obj The object to check.
+ * @return True if the object is an instance of `OmKeyInfo`, false otherwise.
+ */
+ private boolean isOmKeyInfo(Object obj) {
+ if (obj instanceof OmKeyInfo) {
+ return true;
+ } else {
+ LOG.warn("Unexpected value type {} for key. Skipping processing.",
+ obj.getClass().getName());
+ return false;
Review Comment:
@ArafatKhan2198 thanks for providing workaround, however as we know ideally
keyTable and fileTable should not have RepeatedKeyInfo objects and events for
these key types should not raised for these 2 tables. So as @smengcl suggested,
may be you can add some validation logic in
"`org.apache.hadoop.ozone.recon.tasks.OMDBUpdatesHandler#processEvent`" where
you can maintain an enum or map for keyType class name with its corresponding
tablename as reference data to validate against and while if validation fails,
you can log as ERROR with details like keyType, tablename, event action (PUT,
DELETE, UPDATE etc), and not allow such events to process as well to avoid
flowing downstream. This way we can stop such events there itself and change
will be at one place instead of all classes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
