Skip to content

Commit 416ca9d

Browse files
committed
add new check for full backup command regards to continuous backup flag
1 parent 9b2319b commit 416ca9d

File tree

2 files changed

+87
-8
lines changed

2 files changed

+87
-8
lines changed

hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/impl/BackupCommands.java

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@
4747
import java.io.IOException;
4848
import java.net.URI;
4949
import java.util.List;
50+
import java.util.Set;
5051
import org.apache.commons.lang3.StringUtils;
5152
import org.apache.hadoop.conf.Configuration;
5253
import org.apache.hadoop.conf.Configured;
@@ -353,6 +354,46 @@ public void execute() throws IOException {
353354
throw new IOException(INCORRECT_USAGE);
354355
}
355356

357+
/*
358+
* The `continuousBackup` flag is specified only during the first full backup to initiate
359+
* continuous WAL replication. After that, it is redundant because the tables are already set
360+
* up for continuous backup. If the `continuousBackup` flag is not explicitly enabled, we need
361+
* to determine the backup mode based on the current state of the specified tables: - If all
362+
* the specified tables are already part of continuous backup, we treat the request as a
363+
* continuous backup request and proceed accordingly (since these tables are already
364+
* continuously backed up, no additional setup is needed). - If none of the specified tables
365+
* are part of continuous backup, we treat the request as a normal full backup without
366+
* continuous backup. - If the request includes a mix of tables—some with continuous backup
367+
* enabled and others without—we cannot determine a clear backup strategy. In this case, we
368+
* throw an error. If all tables are already in continuous backup mode, we explicitly set the
369+
* `continuousBackup` flag to `true` so that the request is processed using the continuous
370+
* backup approach rather than the normal full backup flow.
371+
*/
372+
if (!continuousBackup && tableNameList != null && !tableNameList.isEmpty()) {
373+
try (BackupSystemTable backupSystemTable = new BackupSystemTable(conn)) {
374+
Set<TableName> continuousBackupTableSet =
375+
backupSystemTable.getContinuousBackupTableSet().keySet();
376+
377+
boolean allTablesInContinuousBackup = continuousBackupTableSet.containsAll(tableNameList);
378+
boolean noTablesInContinuousBackup =
379+
tableNameList.stream().noneMatch(continuousBackupTableSet::contains);
380+
381+
// Ensure that all tables are either fully in continuous backup or not at all
382+
if (!allTablesInContinuousBackup && !noTablesInContinuousBackup) {
383+
System.err
384+
.println("ERROR: Some tables are already in continuous backup, while others are not. "
385+
+ "Cannot mix both in a single request.");
386+
printUsage();
387+
throw new IOException(INCORRECT_USAGE);
388+
}
389+
390+
// If all tables are already in continuous backup, enable the flag
391+
if (allTablesInContinuousBackup) {
392+
continuousBackup = true;
393+
}
394+
}
395+
}
396+
356397
try (BackupAdminImpl admin = new BackupAdminImpl(conn)) {
357398
BackupRequest.Builder builder = new BackupRequest.Builder();
358399
BackupRequest request = builder.withBackupType(backupType).withTableList(tableNameList)

hbase-backup/src/test/java/org/apache/hadoop/hbase/backup/TestContinuousBackup.java

Lines changed: 46 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ public void testContinuousBackupWithFullBackup() throws Exception {
9595
int before = table.getBackupHistory().size();
9696

9797
// Run backup
98-
String[] args = buildBackupArgs("full", new TableName[] { tableName });
98+
String[] args = buildBackupArgs("full", new TableName[] { tableName }, true);
9999
int ret = ToolRunner.run(conf1, new BackupDriver(), args);
100100
assertEquals("Backup should succeed", 0, ret);
101101

@@ -133,12 +133,12 @@ public void testContinuousBackupForMultipleTables() throws Exception {
133133
int before = table.getBackupHistory().size();
134134

135135
// Create full backup for table1
136-
String[] args = buildBackupArgs("full", new TableName[] { tableName1 });
136+
String[] args = buildBackupArgs("full", new TableName[] { tableName1 }, true);
137137
int ret = ToolRunner.run(conf1, new BackupDriver(), args);
138138
assertEquals("Backup should succeed", 0, ret);
139139

140140
// Create full backup for table2
141-
args = buildBackupArgs("full", new TableName[] { tableName2 });
141+
args = buildBackupArgs("full", new TableName[] { tableName2 }, true);
142142
ret = ToolRunner.run(conf1, new BackupDriver(), args);
143143
assertEquals("Backup should succeed", 0, ret);
144144

@@ -165,6 +165,39 @@ public void testContinuousBackupForMultipleTables() throws Exception {
165165
verifyTableInBackupSystemTable(tableName2);
166166
}
167167

168+
@Test
169+
public void testInvalidBackupScenarioWithContinuousEnabled() throws Exception {
170+
LOG.info("Testing invalid backup scenario with continuous backup enabled");
171+
String methodName = Thread.currentThread().getStackTrace()[1].getMethodName();
172+
TableName tableName1 = TableName.valueOf("table_" + methodName);
173+
TEST_UTIL.createTable(tableName1, "cf");
174+
TableName tableName2 = TableName.valueOf("table_" + methodName + "2");
175+
TEST_UTIL.createTable(tableName2, "cf");
176+
177+
try (BackupSystemTable table = new BackupSystemTable(TEST_UTIL.getConnection())) {
178+
int before = table.getBackupHistory().size();
179+
180+
// Create full backup for table1 with continuous backup enabled
181+
String[] args = buildBackupArgs("full", new TableName[] { tableName1 }, true);
182+
int ret = ToolRunner.run(conf1, new BackupDriver(), args);
183+
assertEquals("Backup should succeed", 0, ret);
184+
185+
// Create full backup for table2 without continuous backup enabled
186+
args = buildBackupArgs("full", new TableName[] { tableName2 }, false);
187+
ret = ToolRunner.run(conf1, new BackupDriver(), args);
188+
assertEquals("Backup should succeed", 0, ret);
189+
190+
// Attempt full backup for both tables without continuous backup enabled (should fail)
191+
args = buildBackupArgs("full", new TableName[] { tableName1, tableName2 }, false);
192+
ret = ToolRunner.run(conf1, new BackupDriver(), args);
193+
assertTrue("Backup should fail due to mismatch in continuous backup settings", ret != 0);
194+
195+
// Verify backup history size is unchanged after the failed backup
196+
int after = table.getBackupHistory().size();
197+
assertEquals("Backup history should remain unchanged on failure", before + 2, after);
198+
}
199+
}
200+
168201
@Test
169202
public void testContinuousBackupWithWALDirNotSpecified() throws Exception {
170203
LOG.info("Testing that continuous backup fails when WAL directory is not specified");
@@ -179,7 +212,7 @@ public void testContinuousBackupWithWALDirNotSpecified() throws Exception {
179212
int before = table.getBackupHistory().size();
180213

181214
// Run full backup without specifying WAL directory (invalid scenario)
182-
String[] args = buildBackupArgs("full", new TableName[] { tableName });
215+
String[] args = buildBackupArgs("full", new TableName[] { tableName }, true);
183216
int ret = ToolRunner.run(conf1, new BackupDriver(), args);
184217

185218
assertTrue("Backup should fail when WAL directory is not specified", ret != 0);
@@ -204,7 +237,7 @@ public void testContinuousBackupWithIncrementalBackup() throws Exception {
204237
int before = table.getBackupHistory().size();
205238

206239
// Run incremental backup with continuous backup flag (invalid scenario)
207-
String[] args = buildBackupArgs("incremental", new TableName[] { tableName });
240+
String[] args = buildBackupArgs("incremental", new TableName[] { tableName }, true);
208241
int ret = ToolRunner.run(conf1, new BackupDriver(), args);
209242

210243
assertTrue("Backup should fail when using continuous backup with incremental mode", ret != 0);
@@ -226,12 +259,17 @@ private void verifyReplicationPeerSubscription(TableName table) throws IOExcepti
226259
}
227260
}
228261

229-
private String[] buildBackupArgs(String backupType, TableName[] tables) {
262+
private String[] buildBackupArgs(String backupType, TableName[] tables,
263+
boolean continuousEnabled) {
230264
String tableNames =
231265
Arrays.stream(tables).map(TableName::getNameAsString).collect(Collectors.joining(","));
232266

233-
return new String[] { "create", backupType, BACKUP_ROOT_DIR, "-t", tableNames,
234-
"-" + OPTION_ENABLE_CONTINUOUS_BACKUP };
267+
if (continuousEnabled) {
268+
return new String[] { "create", backupType, BACKUP_ROOT_DIR, "-t", tableNames,
269+
"-" + OPTION_ENABLE_CONTINUOUS_BACKUP };
270+
} else {
271+
return new String[] { "create", backupType, BACKUP_ROOT_DIR, "-t", tableNames };
272+
}
235273
}
236274

237275
private BackupManifest getLatestBackupManifest(List<BackupInfo> backups) throws IOException {

0 commit comments

Comments
 (0)