rmdmattingly commented on code in PR #6294: URL: https://github.com/apache/hbase/pull/6294#discussion_r1793817516
########## hbase-protocol-shaded/src/main/protobuf/Backup.proto: ########## @@ -54,11 +54,12 @@ message TableServerTimestamp { /** * Structure keeps relevant info for backup restore session + * backup_root_dir is deprecated because we no longer store root dirs on disk HBASE-28882 */ message BackupImage { optional string backup_id = 1; optional BackupType backup_type = 2; - optional string backup_root_dir = 3; + optional string backup_root_dir = 3 [deprecated = true]; Review Comment: I'm not sure how cautiously we should treat the removal of this field. Deprecation and documentation seems like the safe approach, but this feature is in beta so maybe we can just remove it. ########## hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/BackupInfo.java: ########## @@ -267,7 +267,7 @@ public String getFailedMsg() { } public void setFailedMsg(String failedMsg) { - if (failedMsg.length() > MAX_FAILED_MESSAGE_LENGTH) { + if (failedMsg != null && failedMsg.length() > MAX_FAILED_MESSAGE_LENGTH) { Review Comment: This is an unrelated, but minor, change. I realized that there was a NPE opportunity here. -- 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