frostruan commented on code in PR #5408:
URL: https://github.com/apache/hbase/pull/5408#discussion_r2362235480
##########
hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/util/BackupUtils.java:
##########
@@ -770,4 +773,52 @@ public static String findMostRecentBackupId(String[]
backupIds) {
return BackupRestoreConstants.BACKUPID_PREFIX + recentTimestamp;
}
+ /**
+ * roll WAL writer for all region servers and record the newest log roll
result
+ */
+ public static void logRoll(Connection conn, String backupRootDir,
Configuration conf)
+ throws IOException {
+ boolean legacy = conf.getBoolean("hbase.backup.logroll.legacy.used",
false);
+ if (legacy) {
+ logRollV1(conn, backupRootDir);
+ } else {
+ logRollV2(conn, backupRootDir);
+ }
+ }
+
+ private static void logRollV1(Connection conn, String backupRootDir) throws
IOException {
+ try (Admin admin = conn.getAdmin()) {
+
admin.execProcedure(LogRollMasterProcedureManager.ROLLLOG_PROCEDURE_SIGNATURE,
+ LogRollMasterProcedureManager.ROLLLOG_PROCEDURE_NAME,
+ ImmutableMap.of("backupRoot", backupRootDir));
+ }
+ }
+
+ private static void logRollV2(Connection conn, String backupRootDir) throws
IOException {
Review Comment:
> I don't think that this method handles cleaning up any servers that no
longer exist on the cluster, which means we'll hang onto oldWALs from those
hosts indefinitely due to the BackupLogCleaner
>
> Please correct me if I'm misunderstanding, but I think we also want to
make sure that we're removing entries from the system tables for hosts that are
no longer part of the cluster
Thanks for reviewing.
I think the functionality of logRollV2 should be same as logRollV1, and I
don't think any additional functionality should be introduced or existing
functionality should be reduced.
As for the problem of cleaning up the dead server log roll result you
mentioned, I have a few questions, would you mind explaining more to help me
understand ?
1. Will keeping old WALs from dead servers indefinitely cause any problems?
2. If clean it up, is there any chance for potential data loss?
3. Does zk-based log roll (ie. the logRollV1) procedure need to clean it too
?
Thanks.
--
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]
