Apache9 commented on a change in pull request #3359:
URL: https://github.com/apache/hbase/pull/3359#discussion_r677913764
##########
File path:
hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/BackupInfo.java
##########
@@ -351,19 +355,19 @@ public void setIncrBackupFileList(List<String>
incrBackupFileList) {
/**
* Set the new region server log timestamps after distributed log roll
- * @param newTableSetTimestampMap table timestamp map
+ * @param prevTableSetTimestampMap table timestamp map
*/
- public void setIncrTimestampMap(HashMap<TableName,
- HashMap<String, Long>> newTableSetTimestampMap) {
- this.tableSetTimestampMap = newTableSetTimestampMap;
+ public void setIncrTimestampMap(Map<TableName,
+ Map<String, Long>> prevTableSetTimestampMap) {
+ this.incrTimestampMap = prevTableSetTimestampMap;
Review comment:
Is this a bug in the old code? We do not change the name of the method
but change the implementation?
##########
File path:
hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/impl/IncrementalBackupManager.java
##########
@@ -99,68 +95,19 @@ public IncrementalBackupManager(Connection conn,
Configuration conf) throws IOEx
newTimestamps = readRegionServerLastLogRollResult();
logList = getLogFilesForNewBackup(previousTimestampMins, newTimestamps,
conf, savedStartCode);
- List<WALItem> logFromSystemTable =
- getLogFilesFromBackupSystem(previousTimestampMins, newTimestamps,
getBackupInfo()
- .getBackupRootDir());
- logList = excludeAlreadyBackedUpAndProcV2WALs(logList, logFromSystemTable);
+ logList = excludeProcV2WALs(logList);
backupInfo.setIncrBackupFileList(logList);
return newTimestamps;
}
- /**
- * Get list of WAL files eligible for incremental backup.
- *
- * @return list of WAL files
- * @throws IOException if getting the list of WAL files fails
- */
- public List<String> getIncrBackupLogFileList() throws IOException {
- List<String> logList;
- HashMap<String, Long> newTimestamps;
- HashMap<String, Long> previousTimestampMins;
-
- String savedStartCode = readBackupStartCode();
-
- // key: tableName
- // value: <RegionServer,PreviousTimeStamp>
- HashMap<TableName, HashMap<String, Long>> previousTimestampMap =
readLogTimestampMap();
-
- previousTimestampMins =
BackupUtils.getRSLogTimestampMins(previousTimestampMap);
-
- if (LOG.isDebugEnabled()) {
- LOG.debug("StartCode " + savedStartCode + "for backupID " +
backupInfo.getBackupId());
- }
- // get all new log files from .logs and .oldlogs after last TS and before
new timestamp
- if (savedStartCode == null || previousTimestampMins == null
- || previousTimestampMins.isEmpty()) {
- throw new IOException(
- "Cannot read any previous back up timestamps from backup system
table. "
- + "In order to create an incremental backup, at least one full
backup is needed.");
- }
-
- newTimestamps = readRegionServerLastLogRollResult();
-
- logList = getLogFilesForNewBackup(previousTimestampMins, newTimestamps,
conf, savedStartCode);
- List<WALItem> logFromSystemTable =
- getLogFilesFromBackupSystem(previousTimestampMins, newTimestamps,
getBackupInfo()
- .getBackupRootDir());
-
- logList = excludeAlreadyBackedUpAndProcV2WALs(logList, logFromSystemTable);
- backupInfo.setIncrBackupFileList(logList);
-
- return logList;
- }
-
- private List<String> excludeAlreadyBackedUpAndProcV2WALs(List<String>
logList,
- List<WALItem> logFromSystemTable) {
- Set<String> walFileNameSet = convertToSet(logFromSystemTable);
-
+ private List<String> excludeProcV2WALs(List<String> logList) {
List<String> list = new ArrayList<>();
for (int i=0; i < logList.size(); i++) {
Path p = new Path(logList.get(i));
String name = p.getName();
- if (walFileNameSet.contains(name) ||
name.startsWith(WALProcedureStore.LOG_PREFIX)) {
+ if (name.startsWith(WALProcedureStore.LOG_PREFIX)) {
Review comment:
On master branch we do not have WALProcedure any more. Now we use
RegionProcedureStore, it has its own WAL. can file another issue to review this.
##########
File path:
hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/impl/BackupSystemTable.java
##########
@@ -178,11 +174,10 @@ public String toString() {
final static byte[] BL_PREPARE = Bytes.toBytes("R");
Review comment:
nits: Open another PR to change the style, it should be 'static final'
instead of 'final static'. This could reduce the checkstyle warnings.
--
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]
