Sigma-Ma commented on code in PR #8512:
URL: https://github.com/apache/hbase/pull/8512#discussion_r3687899835
##########
hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/impl/IncrementalBackupManager.java:
##########
@@ -184,10 +184,10 @@ private List<String> getLogFilesForNewBackup(Map<String,
Long> olderTimestamps,
}
// Include the .oldlogs files too.
- FileStatus[] oldlogs = fs.listStatus(oldLogDir);
- for (FileStatus oldlog : oldlogs) {
- p = oldlog.getPath();
- currentLogFile = p.toString();
+ List<String> oldlogs = BackupUtils.getFiles(fs, oldLogDir, new
ArrayList<>(), path -> true);
Review Comment:
Good catch. I double-checked the built-in WAL creation and archive paths.
For the standard WAL providers, enabling
hbase.separate.oldlogdir.by.regionserver only changes the archive directory.
The WAL filename still normally contains the ServerName because WALFactory uses
the ServerName as the WAL prefix and archiving preserves the original filename.
However, parseHostFromOldLog currently ignores the parent directory, so a
path such as oldWALs/<serverName>/wal.<timestamp> would return null and the
archived WAL would be skipped by incremental backup collection.
I agree that we should handle this problem. I will update
parseHostFromOldLog to use a valid full ServerName from the parent directory,
falling back to the existing filename parsing for the flat oldWAL layout, and
adjust the regression test to cover a hostless WAL filename.
--
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]
