From bbeb82336515d7e9cf1b0889ec6d68b65885fdf2 Mon Sep 17 00:00:00 2001 From: Ankit Solomon Date: Wed, 28 May 2025 10:47:31 +0530 Subject: [PATCH 1/5] HBASE-28994 Enhance backup describe command --- .../hadoop/hbase/backup/BackupInfo.java | 3 ++ .../hbase/backup/TestBackupDescribe.java | 51 +++++++++++++++++++ .../src/main/protobuf/Backup.proto | 1 + 3 files changed, 55 insertions(+) diff --git a/hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/BackupInfo.java b/hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/BackupInfo.java index 862a9cbad107..8b16b2c6e5a7 100644 --- a/hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/BackupInfo.java +++ b/hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/BackupInfo.java @@ -427,6 +427,7 @@ public BackupProtos.BackupInfo toProtosBackupInfo() { builder.setBackupType(BackupProtos.BackupType.valueOf(getType().name())); builder.setWorkersNumber(workers); builder.setBandwidth(bandwidth); + builder.setContinuousBackupEnabled(isContinuousBackupEnabled()); return builder.build(); } @@ -522,6 +523,7 @@ public static BackupInfo fromProto(BackupProtos.BackupInfo proto) { context.setType(BackupType.valueOf(proto.getBackupType().name())); context.setWorkers(proto.getWorkersNumber()); context.setBandwidth(proto.getBandwidth()); + context.setContinuousBackupEnabled(proto.getContinuousBackupEnabled()); return context; } @@ -549,6 +551,7 @@ public String getShortDescription() { sb.append("{"); sb.append("ID=" + backupId).append(","); sb.append("Type=" + getType()).append(","); + sb.append("IsContinuous=" + isContinuousBackupEnabled()).append(","); sb.append("Tables=" + getTableListAsString()).append(","); sb.append("State=" + getState()).append(","); Calendar cal = Calendar.getInstance(); diff --git a/hbase-backup/src/test/java/org/apache/hadoop/hbase/backup/TestBackupDescribe.java b/hbase-backup/src/test/java/org/apache/hadoop/hbase/backup/TestBackupDescribe.java index 7ce039fd6668..c6f61d50e719 100644 --- a/hbase-backup/src/test/java/org/apache/hadoop/hbase/backup/TestBackupDescribe.java +++ b/hbase-backup/src/test/java/org/apache/hadoop/hbase/backup/TestBackupDescribe.java @@ -17,12 +17,17 @@ */ package org.apache.hadoop.hbase.backup; +import static org.apache.hadoop.hbase.backup.BackupRestoreConstants.CONF_CONTINUOUS_BACKUP_WAL_DIR; +import static org.apache.hadoop.hbase.backup.BackupRestoreConstants.OPTION_ENABLE_CONTINUOUS_BACKUP; +import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotEquals; import static org.junit.Assert.assertTrue; import java.io.ByteArrayOutputStream; import java.io.PrintStream; import java.util.List; +import org.apache.hadoop.fs.FileSystem; +import org.apache.hadoop.fs.Path; import org.apache.hadoop.hbase.HBaseClassTestRule; import org.apache.hadoop.hbase.TableName; import org.apache.hadoop.hbase.backup.BackupInfo.BackupState; @@ -107,4 +112,50 @@ public void testBackupDescribeCommand() throws Exception { table.close(); assertTrue(response.indexOf(desc) >= 0); } + + @Test + public void testBackupDescribeCommandForContinuousBackup() throws Exception { + LOG.info("test backup describe on a single table with data: command-line"); + BackupSystemTable table = new BackupSystemTable(TEST_UTIL.getConnection()); + + Path root = TEST_UTIL.getDataTestDirOnTestFS(); + Path backupWalDir = new Path(root, "testBackupDescribeCommand"); + FileSystem fs = FileSystem.get(conf1); + fs.mkdirs(backupWalDir); + conf1.set(CONF_CONTINUOUS_BACKUP_WAL_DIR, backupWalDir.toString()); + + String[] backupArgs = new String[] { "create", BackupType.FULL.name(), BACKUP_ROOT_DIR, "-t", + table1.getNameAsString(), "-" + OPTION_ENABLE_CONTINUOUS_BACKUP }; + int ret = ToolRunner.run(conf1, new BackupDriver(), backupArgs); + assertEquals("Backup should succeed", 0, ret); + String backupId = table.getBackupHistory().get(0).getBackupId(); + assertTrue(checkSucceeded(backupId)); + LOG.info("backup complete"); + + BackupInfo info = getBackupAdmin().getBackupInfo(backupId); + assertTrue(info.getState() == BackupState.COMPLETE); + + ByteArrayOutputStream baos = new ByteArrayOutputStream(); + System.setOut(new PrintStream(baos)); + + String[] args = new String[] { "describe", backupId }; + // Run backup + ret = ToolRunner.run(conf1, new BackupDriver(), args); + assertTrue(ret == 0); + String response = baos.toString(); + assertTrue(response.indexOf(backupId) > 0); + assertTrue(response.indexOf("COMPLETE") > 0); + assertTrue(response.indexOf("IsContinuous=true") > 0); + + BackupInfo status = table.readBackupInfo(backupId); + String desc = status.getShortDescription(); + table.close(); + assertTrue(response.indexOf(desc) >= 0); + + // cleanup + if (fs.exists(backupWalDir)) { + fs.delete(backupWalDir, true); + } + conf1.unset(CONF_CONTINUOUS_BACKUP_WAL_DIR); + } } diff --git a/hbase-protocol-shaded/src/main/protobuf/Backup.proto b/hbase-protocol-shaded/src/main/protobuf/Backup.proto index 95a298673251..bf6ec7d43fcc 100644 --- a/hbase-protocol-shaded/src/main/protobuf/Backup.proto +++ b/hbase-protocol-shaded/src/main/protobuf/Backup.proto @@ -93,6 +93,7 @@ message BackupInfo { optional uint32 workers_number = 11; optional uint64 bandwidth = 12; map table_set_timestamp = 13; + optional bool continuousBackupEnabled = 14; message RSTimestampMap { map rs_timestamp = 1; From 7222ba069a58bca655aee822f1a9a2b1a20ad76d Mon Sep 17 00:00:00 2001 From: Ankit Solomon Date: Wed, 25 Jun 2025 08:51:14 +0530 Subject: [PATCH 2/5] Added incrCommittedWalTs --- .../hadoop/hbase/backup/BackupInfo.java | 21 ++++++ .../hbase/backup/TestBackupDescribe.java | 69 ++++++++++--------- .../src/main/protobuf/Backup.proto | 3 +- 3 files changed, 58 insertions(+), 35 deletions(-) diff --git a/hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/BackupInfo.java b/hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/BackupInfo.java index 8b16b2c6e5a7..fdc7deb6484d 100644 --- a/hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/BackupInfo.java +++ b/hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/BackupInfo.java @@ -173,6 +173,11 @@ public enum BackupPhase { private boolean continuousBackupEnabled; + /** + * Committed WAL timestamp for incremental backup in case of continuous backup + */ + private long incrCommittedWalTs; + public BackupInfo() { backupTableInfoMap = new HashMap<>(); } @@ -428,6 +433,7 @@ public BackupProtos.BackupInfo toProtosBackupInfo() { builder.setWorkersNumber(workers); builder.setBandwidth(bandwidth); builder.setContinuousBackupEnabled(isContinuousBackupEnabled()); + builder.setIncrCommittedWalTs(getIncrCommittedWalTs()); return builder.build(); } @@ -524,6 +530,7 @@ public static BackupInfo fromProto(BackupProtos.BackupInfo proto) { context.setWorkers(proto.getWorkersNumber()); context.setBandwidth(proto.getBandwidth()); context.setContinuousBackupEnabled(proto.getContinuousBackupEnabled()); + context.setIncrCommittedWalTs(proto.getIncrCommittedWalTs()); return context; } @@ -567,6 +574,12 @@ public String getShortDescription() { cal.setTimeInMillis(getCompleteTs()); date = cal.getTime(); sb.append("End time=" + date).append(","); + if (isContinuousBackupEnabled() && getType() == BackupType.INCREMENTAL) { + cal = Calendar.getInstance(); + cal.setTimeInMillis(getIncrCommittedWalTs()); + date = cal.getTime(); + sb.append("Committed WAL time for incremental backup=" + date).append(","); + } } sb.append("Progress=" + getProgress() + "%"); sb.append("}"); @@ -607,4 +620,12 @@ public void setContinuousBackupEnabled(boolean continuousBackupEnabled) { public boolean isContinuousBackupEnabled() { return this.continuousBackupEnabled; } + + public void setIncrCommittedWalTs(long timestamp) { + this.incrCommittedWalTs = timestamp; + } + + public long getIncrCommittedWalTs() { + return this.incrCommittedWalTs; + } } diff --git a/hbase-backup/src/test/java/org/apache/hadoop/hbase/backup/TestBackupDescribe.java b/hbase-backup/src/test/java/org/apache/hadoop/hbase/backup/TestBackupDescribe.java index c6f61d50e719..6d5a15caade6 100644 --- a/hbase-backup/src/test/java/org/apache/hadoop/hbase/backup/TestBackupDescribe.java +++ b/hbase-backup/src/test/java/org/apache/hadoop/hbase/backup/TestBackupDescribe.java @@ -105,6 +105,7 @@ public void testBackupDescribeCommand() throws Exception { String response = baos.toString(); assertTrue(response.indexOf(backupId) > 0); assertTrue(response.indexOf("COMPLETE") > 0); + assertTrue(response.contains("IsContinuous=false")); BackupSystemTable table = new BackupSystemTable(TEST_UTIL.getConnection()); BackupInfo status = table.readBackupInfo(backupId); @@ -116,46 +117,46 @@ public void testBackupDescribeCommand() throws Exception { @Test public void testBackupDescribeCommandForContinuousBackup() throws Exception { LOG.info("test backup describe on a single table with data: command-line"); - BackupSystemTable table = new BackupSystemTable(TEST_UTIL.getConnection()); - Path root = TEST_UTIL.getDataTestDirOnTestFS(); Path backupWalDir = new Path(root, "testBackupDescribeCommand"); FileSystem fs = FileSystem.get(conf1); fs.mkdirs(backupWalDir); conf1.set(CONF_CONTINUOUS_BACKUP_WAL_DIR, backupWalDir.toString()); - String[] backupArgs = new String[] { "create", BackupType.FULL.name(), BACKUP_ROOT_DIR, "-t", - table1.getNameAsString(), "-" + OPTION_ENABLE_CONTINUOUS_BACKUP }; - int ret = ToolRunner.run(conf1, new BackupDriver(), backupArgs); - assertEquals("Backup should succeed", 0, ret); - String backupId = table.getBackupHistory().get(0).getBackupId(); - assertTrue(checkSucceeded(backupId)); - LOG.info("backup complete"); - - BackupInfo info = getBackupAdmin().getBackupInfo(backupId); - assertTrue(info.getState() == BackupState.COMPLETE); - - ByteArrayOutputStream baos = new ByteArrayOutputStream(); - System.setOut(new PrintStream(baos)); - - String[] args = new String[] { "describe", backupId }; - // Run backup - ret = ToolRunner.run(conf1, new BackupDriver(), args); - assertTrue(ret == 0); - String response = baos.toString(); - assertTrue(response.indexOf(backupId) > 0); - assertTrue(response.indexOf("COMPLETE") > 0); - assertTrue(response.indexOf("IsContinuous=true") > 0); - - BackupInfo status = table.readBackupInfo(backupId); - String desc = status.getShortDescription(); - table.close(); - assertTrue(response.indexOf(desc) >= 0); - - // cleanup - if (fs.exists(backupWalDir)) { - fs.delete(backupWalDir, true); + try (BackupSystemTable table = new BackupSystemTable(TEST_UTIL.getConnection())) { + String[] backupArgs = new String[] { "create", BackupType.FULL.name(), BACKUP_ROOT_DIR, "-t", + table2.getNameAsString(), "-" + OPTION_ENABLE_CONTINUOUS_BACKUP }; + int ret = ToolRunner.run(conf1, new BackupDriver(), backupArgs); + assertEquals("Backup should succeed", 0, ret); + List backups = table.getBackupHistory(); + String backupId = backups.get(0).getBackupId(); + assertTrue(checkSucceeded(backupId)); + LOG.info("backup complete"); + + BackupInfo info = getBackupAdmin().getBackupInfo(backupId); + assertTrue(info.getState() == BackupState.COMPLETE); + + ByteArrayOutputStream baos = new ByteArrayOutputStream(); + System.setOut(new PrintStream(baos)); + + String[] args = new String[] { "describe", backupId }; + // Run backup + ret = ToolRunner.run(conf1, new BackupDriver(), args); + assertTrue(ret == 0); + String response = baos.toString(); + assertTrue(response.indexOf(backupId) > 0); + assertTrue(response.indexOf("COMPLETE") > 0); + assertTrue(response.contains("IsContinuous=true")); + + BackupInfo status = table.readBackupInfo(backupId); + String desc = status.getShortDescription(); + assertTrue(response.indexOf(desc) >= 0); + + } finally { + if (fs.exists(backupWalDir)) { + fs.delete(backupWalDir, true); + } + conf1.unset(CONF_CONTINUOUS_BACKUP_WAL_DIR); } - conf1.unset(CONF_CONTINUOUS_BACKUP_WAL_DIR); } } diff --git a/hbase-protocol-shaded/src/main/protobuf/Backup.proto b/hbase-protocol-shaded/src/main/protobuf/Backup.proto index bf6ec7d43fcc..0ad1f5ba6191 100644 --- a/hbase-protocol-shaded/src/main/protobuf/Backup.proto +++ b/hbase-protocol-shaded/src/main/protobuf/Backup.proto @@ -93,7 +93,8 @@ message BackupInfo { optional uint32 workers_number = 11; optional uint64 bandwidth = 12; map table_set_timestamp = 13; - optional bool continuousBackupEnabled = 14; + optional bool continuous_backup_enabled = 14; + optional uint64 incr_committed_wal_ts = 15; message RSTimestampMap { map rs_timestamp = 1; From 58a57dce09473ad25d8140683f157daa2496a50d Mon Sep 17 00:00:00 2001 From: Ankit Solomon Date: Wed, 25 Jun 2025 11:14:23 +0530 Subject: [PATCH 3/5] Minor fix --- .../org/apache/hadoop/hbase/backup/BackupInfo.java | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/BackupInfo.java b/hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/BackupInfo.java index 20525afb270d..887181476c23 100644 --- a/hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/BackupInfo.java +++ b/hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/BackupInfo.java @@ -178,11 +178,6 @@ public enum BackupPhase { private boolean continuousBackupEnabled; - /** - * Committed WAL timestamp for incremental backup in case of continuous backup - */ - private long incrCommittedWalTs; - public BackupInfo() { backupTableInfoMap = new HashMap<>(); } @@ -633,12 +628,4 @@ public void setContinuousBackupEnabled(boolean continuousBackupEnabled) { public boolean isContinuousBackupEnabled() { return this.continuousBackupEnabled; } - - public void setIncrCommittedWalTs(long timestamp) { - this.incrCommittedWalTs = timestamp; - } - - public long getIncrCommittedWalTs() { - return this.incrCommittedWalTs; - } } From e6414821aac8cf3fdbbb0c5bc5ebdd838194f06f Mon Sep 17 00:00:00 2001 From: Ankit Solomon Date: Wed, 25 Jun 2025 16:05:51 +0530 Subject: [PATCH 4/5] Minor fix --- .../main/java/org/apache/hadoop/hbase/backup/BackupInfo.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/BackupInfo.java b/hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/BackupInfo.java index 887181476c23..47731b935efe 100644 --- a/hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/BackupInfo.java +++ b/hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/BackupInfo.java @@ -582,7 +582,7 @@ public String getShortDescription() { cal.setTimeInMillis(getCompleteTs()); date = cal.getTime(); sb.append("End time=" + date).append(","); - if (isContinuousBackupEnabled() && getType() == BackupType.INCREMENTAL) { + if (getType() == BackupType.INCREMENTAL) { cal = Calendar.getInstance(); cal.setTimeInMillis(getIncrCommittedWalTs()); date = cal.getTime(); From b16ebee2e9d531b75afccbde62ce6771e4f2077c Mon Sep 17 00:00:00 2001 From: Ankit Solomon Date: Thu, 10 Jul 2025 20:49:44 +0530 Subject: [PATCH 5/5] Address review comments --- .../hbase/backup/TestBackupDescribe.java | 51 ++++++++++++++++--- 1 file changed, 43 insertions(+), 8 deletions(-) diff --git a/hbase-backup/src/test/java/org/apache/hadoop/hbase/backup/TestBackupDescribe.java b/hbase-backup/src/test/java/org/apache/hadoop/hbase/backup/TestBackupDescribe.java index eee87134148b..54be17f94dae 100644 --- a/hbase-backup/src/test/java/org/apache/hadoop/hbase/backup/TestBackupDescribe.java +++ b/hbase-backup/src/test/java/org/apache/hadoop/hbase/backup/TestBackupDescribe.java @@ -19,6 +19,8 @@ import static org.apache.hadoop.hbase.backup.BackupRestoreConstants.CONF_CONTINUOUS_BACKUP_WAL_DIR; import static org.apache.hadoop.hbase.backup.BackupRestoreConstants.OPTION_ENABLE_CONTINUOUS_BACKUP; +import static org.apache.hadoop.hbase.replication.regionserver.ReplicationMarkerChore.REPLICATION_MARKER_ENABLED_DEFAULT; +import static org.apache.hadoop.hbase.replication.regionserver.ReplicationMarkerChore.REPLICATION_MARKER_ENABLED_KEY; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotEquals; import static org.junit.Assert.assertTrue; @@ -33,7 +35,9 @@ import org.apache.hadoop.hbase.backup.BackupInfo.BackupState; import org.apache.hadoop.hbase.backup.impl.BackupCommands; import org.apache.hadoop.hbase.backup.impl.BackupSystemTable; +import org.apache.hadoop.hbase.client.Put; import org.apache.hadoop.hbase.testclassification.LargeTests; +import org.apache.hadoop.hbase.util.Bytes; import org.apache.hadoop.util.ToolRunner; import org.junit.ClassRule; import org.junit.Test; @@ -123,10 +127,12 @@ public void testBackupDescribeCommandForContinuousBackup() throws Exception { FileSystem fs = FileSystem.get(conf1); fs.mkdirs(backupWalDir); conf1.set(CONF_CONTINUOUS_BACKUP_WAL_DIR, backupWalDir.toString()); + conf1.setBoolean(REPLICATION_MARKER_ENABLED_KEY, true); try (BackupSystemTable table = new BackupSystemTable(TEST_UTIL.getConnection())) { + // Continuous backup String[] backupArgs = new String[] { "create", BackupType.FULL.name(), BACKUP_ROOT_DIR, "-t", - table2.getNameAsString(), "-" + OPTION_ENABLE_CONTINUOUS_BACKUP }; + table1.getNameAsString(), "-" + OPTION_ENABLE_CONTINUOUS_BACKUP }; int ret = ToolRunner.run(conf1, new BackupDriver(), backupArgs); assertEquals("Backup should succeed", 0, ret); List backups = table.getBackupHistory(); @@ -136,28 +142,57 @@ public void testBackupDescribeCommandForContinuousBackup() throws Exception { BackupInfo info = getBackupAdmin().getBackupInfo(backupId); assertTrue(info.getState() == BackupState.COMPLETE); - ByteArrayOutputStream baos = new ByteArrayOutputStream(); System.setOut(new PrintStream(baos)); + // Run backup describe String[] args = new String[] { "describe", backupId }; - // Run backup ret = ToolRunner.run(conf1, new BackupDriver(), args); assertTrue(ret == 0); String response = baos.toString(); - assertTrue(response.indexOf(backupId) > 0); - assertTrue(response.indexOf("COMPLETE") > 0); + assertTrue(response.contains(backupId)); + assertTrue(response.contains("COMPLETE")); assertTrue(response.contains("IsContinuous=true")); - BackupInfo status = table.readBackupInfo(backupId); String desc = status.getShortDescription(); - assertTrue(response.indexOf(desc) >= 0); - + assertTrue(response.contains(desc)); + + // load table + Put p; + for (int i = 0; i < NB_ROWS_IN_BATCH; i++) { + p = new Put(Bytes.toBytes("row" + i)); + p.addColumn(famName, qualName, Bytes.toBytes("val" + i)); + TEST_UTIL.getConnection().getTable(table1).put(p); + } + Thread.sleep(5000); + + // Incremental backup + backupArgs = new String[] { "create", BackupType.INCREMENTAL.name(), BACKUP_ROOT_DIR, "-t", + table1.getNameAsString() }; + ret = ToolRunner.run(conf1, new BackupDriver(), backupArgs); + assertEquals("Incremental Backup should succeed", 0, ret); + backups = table.getBackupHistory(); + String incrBackupId = backups.get(0).getBackupId(); + assertTrue(checkSucceeded(incrBackupId)); + LOG.info("Incremental backup complete"); + + // Run backup describe + args = new String[] { "describe", incrBackupId }; + ret = ToolRunner.run(conf1, new BackupDriver(), args); + assertTrue(ret == 0); + response = baos.toString(); + assertTrue(response.contains(incrBackupId)); + assertTrue(response.contains("COMPLETE")); + assertTrue(response.contains("Committed WAL time for incremental backup=")); + status = table.readBackupInfo(incrBackupId); + desc = status.getShortDescription(); + assertTrue(response.contains(desc)); } finally { if (fs.exists(backupWalDir)) { fs.delete(backupWalDir, true); } conf1.unset(CONF_CONTINUOUS_BACKUP_WAL_DIR); + conf1.setBoolean(REPLICATION_MARKER_ENABLED_KEY, REPLICATION_MARKER_ENABLED_DEFAULT); } } }