Re: [PR] chore(utilities): add SQS backlog / in-flight visibility to S3 events source [hudi]

["voonhous (via GitHub)" <gi...@apache.org>]

voonhous commented on code in PR #19333:
URL: https://github.com/apache/hudi/pull/19333#discussion_r3628623845


##########
hudi-utilities/src/main/java/org/apache/hudi/utilities/sources/S3EventsSource.java:
##########
@@ -109,6 +113,8 @@ public void close() throws IOException {
 
   @Override
   public void onCommit(String lastCkptStr) {
+    LOG.info("S3EventsSource.onCommit: deleting {} processed messages from queue, checkpoint={}.",
+        processedMessages.size(), lastCkptStr);

Review Comment:
   Applied. Checked the claim first -- every log4j2 pattern in the repo carries `%c` (`%-4r [%t] %-5p %c %x - %m%n`, `%d{yy/MM/dd HH:mm:ss} %p %c{1}: %m%n`), so the class name was indeed duplicated.
   
   Worth noting for future instances of this nit: `%c` covers the *class*, not the *method* -- none of the patterns use `%M`. Dropping `onCommit` is fine here only because it is the sole place in this class that deletes processed messages. It would not be safe to strip a method-name prefix by reflex.
   
   Now reads:
   
   ```java
   LOG.info("Deleting {} processed messages from SQS queue, checkpoint={}.",
       processedMessages.size(), lastCkptStr);
   ```



-- 
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: commits-unsubscribe@hudi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org