You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hbase.apache.org by nd...@apache.org on 2024/05/18 17:30:14 UTC
(hbase) branch branch-2 updated (1ef956a5c5f -> 794f6b477bb)
This is an automated email from the ASF dual-hosted git repository.
ndimiduk pushed a change to branch branch-2
in repository https://gitbox.apache.org/repos/asf/hbase.git
from 1ef956a5c5f HBASE-28595: check seq id of scan RPCs for closed scanners (#5910) (#5922)
new 472f6981dd4 HBASE-28568 Incremental backup set does not correctly shrink (#5876)
new 794f6b477bb HBASE-28568 Incremental backup set does not correctly shrink (addendum) (#5917)
The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
.../hadoop/hbase/backup/impl/BackupAdminImpl.java | 41 +++++++++-------------
.../hbase/backup/impl/BackupSystemTable.java | 31 ++++++++--------
.../hadoop/hbase/backup/TestBackupDelete.java | 25 +++++++++++++
3 files changed, 58 insertions(+), 39 deletions(-)
(hbase) 01/02: HBASE-28568 Incremental backup set does not correctly shrink (#5876)
Posted by nd...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
ndimiduk pushed a commit to branch branch-2
in repository https://gitbox.apache.org/repos/asf/hbase.git
commit 472f6981dd47fee72d3da74cb5f6c2112feb75d8
Author: DieterDP <90...@users.noreply.github.com>
AuthorDate: Fri May 17 12:48:10 2024 +0200
HBASE-28568 Incremental backup set does not correctly shrink (#5876)
The incremental backup set is the set of tables included when
an incremental backup is created, it is managed per backup
root dir and contains all tables that are present in at least
one backup (in that root dir).
The incremental backup set can only shrink when backups are
deleted. However, the implementation was incorrect, causing this
set to never be able to shrink.
Reviewed-by: Ray Mattingly <rm...@gmail.com>
Signed-off-by: Nick Dimiduk <nd...@apache.org>
---
.../hadoop/hbase/backup/impl/BackupAdminImpl.java | 40 +++++++++-------------
.../hbase/backup/impl/BackupSystemTable.java | 31 +++++++++--------
.../hadoop/hbase/backup/TestBackupDelete.java | 25 ++++++++++++++
3 files changed, 58 insertions(+), 38 deletions(-)
diff --git a/hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/impl/BackupAdminImpl.java b/hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/impl/BackupAdminImpl.java
index 69aef51a4ed..de8ca6b7497 100644
--- a/hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/impl/BackupAdminImpl.java
+++ b/hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/impl/BackupAdminImpl.java
@@ -94,7 +94,6 @@ public class BackupAdminImpl implements BackupAdmin {
public int deleteBackups(String[] backupIds) throws IOException {
int totalDeleted = 0;
- Map<String, HashSet<TableName>> allTablesMap = new HashMap<>();
boolean deleteSessionStarted;
boolean snapshotDone;
@@ -130,20 +129,16 @@ public class BackupAdminImpl implements BackupAdmin {
}
snapshotDone = true;
try {
+ List<String> affectedBackupRootDirs = new ArrayList<>();
for (int i = 0; i < backupIds.length; i++) {
BackupInfo info = sysTable.readBackupInfo(backupIds[i]);
- if (info != null) {
- String rootDir = info.getBackupRootDir();
- HashSet<TableName> allTables = allTablesMap.get(rootDir);
- if (allTables == null) {
- allTables = new HashSet<>();
- allTablesMap.put(rootDir, allTables);
- }
- allTables.addAll(info.getTableNames());
- totalDeleted += deleteBackup(backupIds[i], sysTable);
+ if (info == null) {
+ continue;
}
+ affectedBackupRootDirs.add(info.getBackupRootDir());
+ totalDeleted += deleteBackup(backupIds[i], sysTable);
}
- finalizeDelete(allTablesMap, sysTable);
+ finalizeDelete(affectedBackupRootDirs, sysTable);
// Finish
sysTable.finishDeleteOperation();
// delete snapshot
@@ -176,26 +171,23 @@ public class BackupAdminImpl implements BackupAdmin {
/**
* Updates incremental backup set for every backupRoot
- * @param tablesMap map [backupRoot: {@code Set<TableName>}]
- * @param table backup system table
+ * @param backupRoots backupRoots for which to revise the incremental backup set
+ * @param table backup system table
* @throws IOException if a table operation fails
*/
- private void finalizeDelete(Map<String, HashSet<TableName>> tablesMap, BackupSystemTable table)
+ private void finalizeDelete(List<String> backupRoots, BackupSystemTable table)
throws IOException {
- for (String backupRoot : tablesMap.keySet()) {
+ for (String backupRoot : backupRoots) {
Set<TableName> incrTableSet = table.getIncrementalBackupTableSet(backupRoot);
- Map<TableName, ArrayList<BackupInfo>> tableMap =
+ Map<TableName, List<BackupInfo>> tableMap =
table.getBackupHistoryForTableSet(incrTableSet, backupRoot);
- for (Map.Entry<TableName, ArrayList<BackupInfo>> entry : tableMap.entrySet()) {
- if (entry.getValue() == null) {
- // No more backups for a table
- incrTableSet.remove(entry.getKey());
- }
- }
+
+ // Keep only the tables that are present in other backups
+ incrTableSet.retainAll(tableMap.keySet());
+
+ table.deleteIncrementalBackupTableSet(backupRoot);
if (!incrTableSet.isEmpty()) {
table.addIncrementalBackupTableSet(incrTableSet, backupRoot);
- } else { // empty
- table.deleteIncrementalBackupTableSet(backupRoot);
}
}
}
diff --git a/hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/impl/BackupSystemTable.java b/hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/impl/BackupSystemTable.java
index b64587b6fca..bbcc3892ae7 100644
--- a/hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/impl/BackupSystemTable.java
+++ b/hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/impl/BackupSystemTable.java
@@ -86,8 +86,9 @@ import org.apache.hadoop.hbase.shaded.protobuf.generated.HBaseProtos;
* <ul>
* <li>1. Backup sessions rowkey= "session:"+backupId; value =serialized BackupInfo</li>
* <li>2. Backup start code rowkey = "startcode:"+backupRoot; value = startcode</li>
- * <li>3. Incremental backup set rowkey="incrbackupset:"+backupRoot; value=[list of tables]</li>
- * <li>4. Table-RS-timestamp map rowkey="trslm:"+backupRoot+table_name; value = map[RS-%3E last WAL
+ * <li>3. Incremental backup set rowkey="incrbackupset:"+backupRoot; table="meta:"+tablename of
+ * include table; value=empty</li>
+ * <li>4. Table-RS-timestamp map rowkey="trslm:"+backupRoot+table_name; value = map[RS-> last WAL
* timestamp]</li>
* <li>5. RS - WAL ts map rowkey="rslogts:"+backupRoot +server; value = last WAL timestamp</li>
* <li>6. WALs recorded rowkey="wals:"+WAL unique file name; value = backupId and full WAL file
@@ -839,23 +840,25 @@ public final class BackupSystemTable implements Closeable {
return tableHistory;
}
- public Map<TableName, ArrayList<BackupInfo>> getBackupHistoryForTableSet(Set<TableName> set,
+ /**
+ * Goes through all backup history corresponding to the provided root folder, and collects all
+ * backup info mentioning each of the provided tables.
+ * @param set the tables for which to collect the {@code BackupInfo}
+ * @param backupRoot backup destination path to retrieve backup history for
+ * @return a map containing (a subset of) the provided {@code TableName}s, mapped to a list of at
+ * least one {@code BackupInfo}
+ * @throws IOException if getting the backup history fails
+ */
+ public Map<TableName, List<BackupInfo>> getBackupHistoryForTableSet(Set<TableName> set,
String backupRoot) throws IOException {
List<BackupInfo> history = getBackupHistory(backupRoot);
- Map<TableName, ArrayList<BackupInfo>> tableHistoryMap = new HashMap<>();
- for (Iterator<BackupInfo> iterator = history.iterator(); iterator.hasNext();) {
- BackupInfo info = iterator.next();
- if (!backupRoot.equals(info.getBackupRootDir())) {
- continue;
- }
+ Map<TableName, List<BackupInfo>> tableHistoryMap = new HashMap<>();
+ for (BackupInfo info : history) {
List<TableName> tables = info.getTableNames();
for (TableName tableName : tables) {
if (set.contains(tableName)) {
- ArrayList<BackupInfo> list = tableHistoryMap.get(tableName);
- if (list == null) {
- list = new ArrayList<>();
- tableHistoryMap.put(tableName, list);
- }
+ List<BackupInfo> list =
+ tableHistoryMap.computeIfAbsent(tableName, k -> new ArrayList<>());
list.add(info);
}
}
diff --git a/hbase-backup/src/test/java/org/apache/hadoop/hbase/backup/TestBackupDelete.java b/hbase-backup/src/test/java/org/apache/hadoop/hbase/backup/TestBackupDelete.java
index 0c4d44d489d..ef40bc63d08 100644
--- a/hbase-backup/src/test/java/org/apache/hadoop/hbase/backup/TestBackupDelete.java
+++ b/hbase-backup/src/test/java/org/apache/hadoop/hbase/backup/TestBackupDelete.java
@@ -17,6 +17,7 @@
*/
package org.apache.hadoop.hbase.backup;
+import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;
import java.io.ByteArrayOutputStream;
@@ -30,6 +31,7 @@ import org.apache.hadoop.hbase.backup.impl.BackupSystemTable;
import org.apache.hadoop.hbase.testclassification.LargeTests;
import org.apache.hadoop.hbase.util.EnvironmentEdge;
import org.apache.hadoop.hbase.util.EnvironmentEdgeManager;
+import org.apache.hadoop.thirdparty.com.google.common.collect.Sets;
import org.apache.hadoop.util.ToolRunner;
import org.junit.Assert;
import org.junit.ClassRule;
@@ -158,4 +160,27 @@ public class TestBackupDelete extends TestBackupBase {
LOG.info(baos.toString());
assertTrue(output.indexOf("Deleted 1 backups") >= 0);
}
+
+ /**
+ * Verify that backup deletion updates the incremental-backup-set.
+ */
+ @Test
+ public void testBackupDeleteUpdatesIncrementalBackupSet() throws Exception {
+ LOG.info("Test backup delete updates the incremental backup set");
+ BackupSystemTable backupSystemTable = new BackupSystemTable(TEST_UTIL.getConnection());
+
+ String backupId1 = fullTableBackup(Lists.newArrayList(table1, table2));
+ assertTrue(checkSucceeded(backupId1));
+ assertEquals(Sets.newHashSet(table1, table2),
+ backupSystemTable.getIncrementalBackupTableSet(BACKUP_ROOT_DIR));
+
+ String backupId2 = fullTableBackup(Lists.newArrayList(table3));
+ assertTrue(checkSucceeded(backupId2));
+ assertEquals(Sets.newHashSet(table1, table2, table3),
+ backupSystemTable.getIncrementalBackupTableSet(BACKUP_ROOT_DIR));
+
+ getBackupAdmin().deleteBackups(new String[] { backupId1 });
+ assertEquals(Sets.newHashSet(table3),
+ backupSystemTable.getIncrementalBackupTableSet(BACKUP_ROOT_DIR));
+ }
}
(hbase) 02/02: HBASE-28568 Incremental backup set does not correctly shrink (addendum) (#5917)
Posted by nd...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
ndimiduk pushed a commit to branch branch-2
in repository https://gitbox.apache.org/repos/asf/hbase.git
commit 794f6b477bb7e68e37105f5f57177f4992aee1bd
Author: Nick Dimiduk <nd...@apache.org>
AuthorDate: Fri May 17 18:31:19 2024 +0200
HBASE-28568 Incremental backup set does not correctly shrink (addendum) (#5917)
Import the correct shaded Guava and run spotless:apply.
Signed-off-by: Duo Zhang <zh...@apache.org>
---
.../main/java/org/apache/hadoop/hbase/backup/impl/BackupAdminImpl.java | 1 -
.../src/test/java/org/apache/hadoop/hbase/backup/TestBackupDelete.java | 2 +-
2 files changed, 1 insertion(+), 2 deletions(-)
diff --git a/hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/impl/BackupAdminImpl.java b/hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/impl/BackupAdminImpl.java
index de8ca6b7497..f500581e9d8 100644
--- a/hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/impl/BackupAdminImpl.java
+++ b/hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/impl/BackupAdminImpl.java
@@ -21,7 +21,6 @@ import java.io.IOException;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
-import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
diff --git a/hbase-backup/src/test/java/org/apache/hadoop/hbase/backup/TestBackupDelete.java b/hbase-backup/src/test/java/org/apache/hadoop/hbase/backup/TestBackupDelete.java
index ef40bc63d08..785859c5280 100644
--- a/hbase-backup/src/test/java/org/apache/hadoop/hbase/backup/TestBackupDelete.java
+++ b/hbase-backup/src/test/java/org/apache/hadoop/hbase/backup/TestBackupDelete.java
@@ -31,7 +31,6 @@ import org.apache.hadoop.hbase.backup.impl.BackupSystemTable;
import org.apache.hadoop.hbase.testclassification.LargeTests;
import org.apache.hadoop.hbase.util.EnvironmentEdge;
import org.apache.hadoop.hbase.util.EnvironmentEdgeManager;
-import org.apache.hadoop.thirdparty.com.google.common.collect.Sets;
import org.apache.hadoop.util.ToolRunner;
import org.junit.Assert;
import org.junit.ClassRule;
@@ -41,6 +40,7 @@ import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.apache.hbase.thirdparty.com.google.common.collect.Lists;
+import org.apache.hbase.thirdparty.com.google.common.collect.Sets;
@Category(LargeTests.class)
public class TestBackupDelete extends TestBackupBase {