Copilot commented on code in PR #7150:
URL: https://github.com/apache/hbase/pull/7150#discussion_r2201467565


##########
hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/impl/BulkLoad.java:
##########
@@ -64,6 +66,10 @@ public byte[] getRowKey() {
     return rowKey;
   }
 
+  public long getTimestamp() {
+    return timestamp;
+  }
+
   @Override
   public boolean equals(Object o) {

Review Comment:
   The new `timestamp` field is not included in the equals (and likely 
hashCode) methods; this can cause two bulk-load entries with different 
timestamps to compare equal. Update equals and hashCode to include `timestamp`.



##########
hbase-server/src/main/java/org/apache/hadoop/hbase/tool/BulkLoadHFilesTool.java:
##########
@@ -1195,6 +1195,11 @@ public int run(String[] args) throws Exception {
   public static void main(String[] args) throws Exception {
     Configuration conf = HBaseConfiguration.create();
     int ret = ToolRunner.run(conf, new BulkLoadHFilesTool(conf), args);
+    if (ret == 0) {
+      System.out.println("Bulk load completed successfully.");
+      System.out.println("IMPORTANT: Please take a backup of the table 
immediately if this table "

Review Comment:
   Use the framework’s logger (e.g., LOG.info) instead of System.out.println 
for consistency with the rest of HBase’s logging.
   ```suggestion
         LOG.info("Bulk load completed successfully.");
         LOG.info("IMPORTANT: Please take a backup of the table immediately if 
this table "
   ```



##########
hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/PointInTimeRestoreDriver.java:
##########
@@ -69,6 +72,12 @@ protected int executeRestore(boolean check, TableName[] 
fromTables, TableName[]
       return -1;
     }
 
+    boolean force = cmd.hasOption(OPTION_FORCE_RESTORE);
+    if (force) {
+      LOG.debug("Found force option (-{}) in restore command, "

Review Comment:
   [nitpick] This debug-level message may not be visible in default logs; 
consider logging at INFO or printing a user-facing warning so users know 
they’ve forced a restore.
   ```suggestion
         LOG.info("Found force option (-{}) in restore command, "
   ```



-- 
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: issues-unsubscr...@hbase.apache.org

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

Reply via email to