devmadhuu commented on code in PR #5490:
URL: https://github.com/apache/ozone/pull/5490#discussion_r1407951364
##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/tasks/OMDBUpdatesHandler.java:
##########
@@ -153,8 +155,22 @@ private void processEvent(int cfIndex, byte[] keyBytes,
byte[]
}
}
} else if (action.equals(DELETE)) {
- if (oldValue != null && !omUpdateEventValidator.isValidEvent(tableName,
- oldValue, key, action)) {
+ if (null == oldValue) {
+ String keyStr = (key instanceof String) ? key.toString() : "";
+ if (keyStr.isEmpty()) {
+ LOG.warn(
+ "Only DTOKEN_TABLE table uses OzoneTokenIdentifier as key " +
+ "instead of String. Event on any other table in this " +
+ "condition may need to be investigated. This DELETE " +
+ "event is on {} table which is not useful for Recon to " +
+ "capture.", tableName);
+ }
+ LOG.warn("Old Value of Key: {} in table: {} should not be null " +
+ "for DELETE event ", keyStr, tableName);
+ return;
Review Comment:
@dombizita thanks for your review, Not sure If I understood correctly, but
as I can see that if key is not empty, then also we are printing that "old
value is null" information using message -> "Old Value of Key: {} in table: {}
should not be null " , so I believe the purpose is still getting solved. Pls
let me know if I understood something not correct.
##########
hadoop-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/tasks/TestOMDBUpdatesHandler.java:
##########
@@ -197,18 +195,8 @@ public void testDelete() throws Exception {
OmVolumeArgs volumeInfo = (OmVolumeArgs) volEvent.getValue();
assertEquals("sampleVol", volumeInfo.getVolume());
- // Assert the values of non existent keys are set to null.
- OMDBUpdateEvent nonExistKey = events.get(2);
- assertEquals(OMDBUpdateEvent.OMDBUpdateAction.DELETE,
- nonExistKey.getAction());
- assertEquals("/sampleVol/bucketOne/key_two", nonExistKey.getKey());
- assertNull(nonExistKey.getValue());
-
- OMDBUpdateEvent nonExistVolume = events.get(3);
- assertEquals(OMDBUpdateEvent.OMDBUpdateAction.DELETE,
- nonExistVolume.getAction());
- assertEquals(nonExistVolumeKey, nonExistVolume.getKey());
- assertNull(nonExistVolume.getValue());
+ // Assert for non existent keys, no events will be captured and handled.
+ assertEquals(2, events.size());
Review Comment:
Handled.
--
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]
