ndimiduk commented on code in PR #6195:
URL: https://github.com/apache/hbase/pull/6195#discussion_r1749932567


##########
hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/master/BackupLogCleaner.java:
##########
@@ -176,12 +187,29 @@ protected static boolean canDeleteFile(Map<Address, Long> 
addressToLastBackupMap
       Address walServerAddress = Address.fromString(hostname);
       long walTimestamp = AbstractFSWALProvider.getTimestamp(path.getName());
 
-      if (
-        !addressToLastBackupMap.containsKey(walServerAddress)
-          || addressToLastBackupMap.get(walServerAddress) >= walTimestamp
-      ) {
+      if (!addressToLastBackupMap.containsKey(walServerAddress)) {
+        if (LOG.isDebugEnabled()) {
+          LOG.debug("No backup found for server: {}. Deleting file: {}",
+            walServerAddress.getHostName(), path);
+        }
         return true;
       }
+
+      Long lastBackupTs = addressToLastBackupMap.get(walServerAddress);
+      if (lastBackupTs > walTimestamp) {
+        if (LOG.isDebugEnabled()) {
+          LOG.debug(
+            "Backup found for server: {}. Backup from {} is newer than file, 
so deleting: {}",

Review Comment:
   In my experience, operators are very accustomed to thinking in UTC, no 
matter where they live. What I'm saying is that I expect an operator/SRE to be 
able to read UTC timestamps as tablestakes. A human being reading milliseconds 
since unix epoc, less so.
   
   But anyway, just an observation, not a blocking suggestion; this code (as 
in, all of hbase) is littered with logging epoc millis.



-- 
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]

Reply via email to