devmadhuu commented on PR #5490:
URL: https://github.com/apache/ozone/pull/5490#issuecomment-1825249553

   > // Handle the event based on its type:
   > // - PUT with a new key: Insert the new value.
   > // - PUT with an existing key: Update the existing value.
   > // - DELETE with an existing key: Remove the value.
   > // - DELETE with a non-existing key: No action, log a warning if necessary.
   
   Thanks @ArafatKhan2198 for review and noticing that a method was left empty 
for its javadoc comments. May be because we follow for not adding javadoc for 
private methods. Nonetheless , we should add comments wherever necessary for 
our own understanding. Thanks again for pointing this out. I have added now and 
rectified all comments for easy understanding per your suggestion.
   
   > If it's possible we could rephrase the paragraph at the starting of the 
process() method better clarity and understanding. Please let me know if I have 
gotten anything wrong in my understanding.
   > 
   > ```
   > // Note on DTOKEN_TABLE handling:
   > // - The DTOKEN_TABLE uniquely utilizes OzoneTokenIdentifier, not a 
String, as its key.
   > // - Directly casting to String during deserialization would result in 
errors.
   > // - The omdbLatestUpdateEvents currently maps keys as Strings. Changing 
this to Object would 
   > //   significantly affect all ReconOmTasks, hence it remains unchanged for 
now.
   > // - As DTOKEN_TABLE data is not currently required for synchronization in 
the Recon OM DB snapshot,
   > //   no immediate changes are needed.
   > // - Future integration of DTOKEN_TABLE data will involve storing and 
retrieving events using Object-type keys.
   > ```
   > 
   > 
https://github.com/apache/ozone/blob/5e80d8283bd13878d55a2f3a241586930c97f32e/hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/tasks/OMDBUpdatesHandler.java#L95
   
   I read the above existing comment and it looks ok to me with more 
information, so leaving it as it is as of now. I also observed that this 
comment was modified as part of #HDDS-7455.
   
   > Could you change this as well to :- `if (action.equals(PUT))`
   > 
   > 
https://github.com/apache/ozone/blob/5e80d8283bd13878d55a2f3a241586930c97f32e/hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/tasks/OMDBUpdatesHandler.java#L130
   
   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: [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]

Reply via email to