DieterDP-ng commented on code in PR #7523:
URL: https://github.com/apache/hbase/pull/7523#discussion_r2597856808
##########
hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/master/BackupLogCleaner.java:
##########
Review Comment:
My suggestion would be to change the following in the original code:
```
protected static boolean canDeleteFile(Map<Address, Long>
addressToBoundaryTs, Path path) {
...
if (!addressToBoundaryTs.containsKey(walServerAddress)) {
if (LOG.isDebugEnabled()) {
LOG.debug("No cleanup WAL time-boundary found for server: {}. Ok
to delete file: {}",
walServerAddress.getHostName(), path);
}
return true;
}
...
```
to:
```
// oldestStartCode calculated as suggested in this PR
protected static boolean canDeleteFile(Map<Address, Long>
addressToBoundaryTs, long oldestStartCode, Path path) {
...
if (!addressToBoundaryTs.containsKey(walServerAddress)) {
long walTimestamp =
AbstractFSWALProvider.getTimestamp(path.getName());
if (walTimestamp <= oldestStartCode) {
if (LOG.isDebugEnabled()) {
LOG.debug(...);
}
return true;
}
}
...
```
So this uses the specialized logic from the original solution for RS still
in use, and your fix for cases where RS have since appeared/disappeared.
I agree complexity is a bad thing, but I feel it is manageable in the log
cleaner, and brings relevant advantages. I won't make a hard stance about this.
It's just that I would expect that at some point someone in the future would
end up trying to re-introduce more specialized cleaning logic because they're
seeing log files sticking around longer than required.
--
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]
