hgromer commented on code in PR #7523:
URL: https://github.com/apache/hbase/pull/7523#discussion_r2593766937
##########
hbase-backup/src/test/java/org/apache/hadoop/hbase/backup/master/TestBackupLogCleaner.java:
##########
@@ -193,35 +200,131 @@ public void testBackupLogCleaner() throws Exception {
// Taking the minimum timestamp (= 2), this means all WALs preceding B3
can be deleted.
deletable = cleaner.getDeletableFiles(walFilesAfterB5);
assertEquals(toSet(walFilesAfterB2), toSet(deletable));
+ } finally {
+
TEST_UTIL.truncateTable(BackupSystemTable.getTableName(TEST_UTIL.getConfiguration())).close();
}
}
- private Set<FileStatus> mergeAsSet(Collection<FileStatus> toCopy,
Collection<FileStatus> toAdd) {
- Set<FileStatus> result = new LinkedHashSet<>(toCopy);
- result.addAll(toAdd);
- return result;
+ @Test
+ public void testDoesNotDeleteWALsFromNewServers() throws Exception {
+ Path backupRoot1 = new Path(BACKUP_ROOT_DIR, "backup1");
+ List<TableName> tableSetFull = List.of(table1, table2, table3, table4);
+
+ try (BackupSystemTable systemTable = new
BackupSystemTable(TEST_UTIL.getConnection())) {
+ LOG.info("Creating initial backup B1");
+ String backupIdB1 = backupTables(BackupType.FULL, tableSetFull,
backupRoot1.toString());
+ assertTrue(checkSucceeded(backupIdB1));
+
+ List<FileStatus> walsAfterB1 =
getListOfWALFiles(TEST_UTIL.getConfiguration());
+ LOG.info("WALs after B1: {}", walsAfterB1.size());
+
+ String startCodeStr =
systemTable.readBackupStartCode(backupRoot1.toString());
+ long b1StartCode = Long.parseLong(startCodeStr);
+ LOG.info("B1 startCode: {}", b1StartCode);
+
+ // Add a new RegionServer to the cluster
+ LOG.info("Adding new RegionServer to cluster");
+ HRegionServer newRS =
TEST_UTIL.getMiniHBaseCluster().startRegionServer().getRegionServer();
+ ServerName newServerName = newRS.getServerName();
+ LOG.info("New RegionServer started: {}", newServerName);
+
+ // Move a region to the new server to ensure it creates a WAL
+ List<RegionInfo> regions = TEST_UTIL.getAdmin().getRegions(table1);
+ RegionInfo regionToMove = regions.get(0);
+
+ LOG.info("Moving region {} to new server {}",
regionToMove.getEncodedName(), newServerName);
+ TEST_UTIL.getAdmin().move(regionToMove.getEncodedNameAsBytes(),
newServerName);
+
+ TEST_UTIL.waitFor(30000, () -> {
+ try {
+ HRegionLocation location =
TEST_UTIL.getConnection().getRegionLocator(table1)
+ .getRegionLocation(regionToMove.getStartKey());
+ return location.getServerName().equals(newServerName);
+ } catch (IOException e) {
+ return false;
+ }
+ });
+
+ // Write some data to trigger WAL creation on the new server
+ try (Table t1 = TEST_UTIL.getConnection().getTable(table1)) {
+ for (int i = 0; i < 100; i++) {
+ Put p = new Put(Bytes.toBytes("newserver-row-" + i));
+ p.addColumn(famName, qualName, Bytes.toBytes("val" + i));
+ t1.put(p);
+ }
+ }
+ TEST_UTIL.getAdmin().flushRegion(regionToMove.getEncodedNameAsBytes());
+
+ List<FileStatus> walsAfterNewServer =
getListOfWALFiles(TEST_UTIL.getConfiguration());
+ LOG.info("WALs after adding new server: {}", walsAfterNewServer.size());
+ assertTrue("Should have more WALs after new server",
+ walsAfterNewServer.size() > walsAfterB1.size());
+
+ List<FileStatus> newServerWALs = new ArrayList<>(walsAfterNewServer);
+ newServerWALs.removeAll(walsAfterB1);
+ assertFalse("Should have WALs from new server", newServerWALs.isEmpty());
+
+ BackupLogCleaner cleaner = new BackupLogCleaner();
+ cleaner.setConf(TEST_UTIL.getConfiguration());
+ cleaner.init(Map.of(HMaster.MASTER,
TEST_UTIL.getHBaseCluster().getMaster()));
+
+ Set<FileStatus> deletable =
+ ImmutableSet.copyOf(cleaner.getDeletableFiles(walsAfterNewServer));
+ for (FileStatus newWAL : newServerWALs) {
+ assertFalse("WAL from new server should NOT be deletable: " +
newWAL.getPath(),
Review Comment:
This assertion fails without the patch, which proves the data loss scenario
--
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]
