From b840c998243bdf53de010e42289803966cd59806 Mon Sep 17 00:00:00 2001 From: Charles Connell Date: Fri, 7 Aug 2026 15:36:51 -0400 Subject: [PATCH 1/2] Persist all fields of BackupInfo --- .../hadoop/hbase/backup/BackupInfo.java | 40 +++- .../backup/TestBackupInfoSerialization.java | 180 ++++++++++++++++++ .../src/main/protobuf/Backup.proto | 4 + 3 files changed, 216 insertions(+), 8 deletions(-) create mode 100644 hbase-backup/src/test/java/org/apache/hadoop/hbase/backup/TestBackupInfoSerialization.java 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 edf5a2b517ff..28b65dca4941 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 @@ -435,6 +435,18 @@ public BackupProtos.BackupInfo toProtosBackupInfo() { builder.setBackupType(BackupProtos.BackupType.valueOf(getType().name())); builder.setWorkersNumber(workers); builder.setBandwidth(bandwidth); + builder.setTotalBytesCopied(totalBytesCopied); + builder.setNoChecksumVerify(noChecksumVerify); + if (incrBackupFileList != null) { + builder.addAllIncrBackupFileList(incrBackupFileList); + } + if (incrTimestampMap != null) { + for (Entry> entry : incrTimestampMap.entrySet()) { + builder.putIncrTimestampMap(entry.getKey().getNameAsString(), + BackupProtos.BackupInfo.RSTimestampMap.newBuilder().putAllRsTimestamp(entry.getValue()) + .build()); + } + } return builder.build(); } @@ -507,7 +519,7 @@ public static BackupInfo fromProto(BackupProtos.BackupInfo proto) { BackupInfo context = new BackupInfo(); context.setBackupId(proto.getBackupId()); context.setBackupTableInfoMap(toMap(proto.getBackupTableInfoList())); - context.setTableSetTimestampMap(getTableSetTimestampMap(proto.getTableSetTimestampMap())); + context.setTableSetTimestampMap(toTableTimestampMap(proto.getTableSetTimestampMap())); context.setCompleteTs(proto.getCompleteTs()); if (proto.hasFailedMessage()) { context.setFailedMsg(proto.getFailedMessage()); @@ -516,8 +528,13 @@ public static BackupInfo fromProto(BackupProtos.BackupInfo proto) { context.setState(BackupInfo.BackupState.valueOf(proto.getBackupState().name())); } - context - .setHLogTargetDir(BackupUtils.getLogBackupDir(proto.getBackupRootDir(), proto.getBackupId())); + // Only incremental backups have a WAL target directory. Setting this unconditionally would + // hand FULL backups a non-null path that never existed, which cleanupHLogDir() would then + // try to delete. + if (BackupType.valueOf(proto.getBackupType().name()) == BackupType.INCREMENTAL) { + context.setHLogTargetDir( + BackupUtils.getLogBackupDir(proto.getBackupRootDir(), proto.getBackupId())); + } if (proto.hasBackupPhase()) { context.setPhase(BackupPhase.valueOf(proto.getBackupPhase().name())); @@ -530,6 +547,14 @@ public static BackupInfo fromProto(BackupProtos.BackupInfo proto) { context.setType(BackupType.valueOf(proto.getBackupType().name())); context.setWorkers(proto.getWorkersNumber()); context.setBandwidth(proto.getBandwidth()); + context.setTotalBytesCopied(proto.getTotalBytesCopied()); + context.setNoChecksumVerify(proto.getNoChecksumVerify()); + if (proto.getIncrBackupFileListCount() > 0) { + context.setIncrBackupFileList(new ArrayList<>(proto.getIncrBackupFileListList())); + } + if (proto.getIncrTimestampMapCount() > 0) { + context.setIncrTimestampMap(toTableTimestampMap(proto.getIncrTimestampMapMap())); + } return context; } @@ -542,14 +567,13 @@ private static Map toMap(List> - getTableSetTimestampMap(Map map) { - Map> tableSetTimestampMap = new HashMap<>(); + toTableTimestampMap(Map map) { + Map> result = new HashMap<>(); for (Entry entry : map.entrySet()) { - tableSetTimestampMap.put(TableName.valueOf(entry.getKey()), - entry.getValue().getRsTimestampMap()); + result.put(TableName.valueOf(entry.getKey()), entry.getValue().getRsTimestampMap()); } - return tableSetTimestampMap; + return result; } public String getShortDescription() { diff --git a/hbase-backup/src/test/java/org/apache/hadoop/hbase/backup/TestBackupInfoSerialization.java b/hbase-backup/src/test/java/org/apache/hadoop/hbase/backup/TestBackupInfoSerialization.java new file mode 100644 index 000000000000..cd4d50880fba --- /dev/null +++ b/hbase-backup/src/test/java/org/apache/hadoop/hbase/backup/TestBackupInfoSerialization.java @@ -0,0 +1,180 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.hadoop.hbase.backup; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.io.IOException; +import java.util.Arrays; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import org.apache.hadoop.hbase.TableName; +import org.apache.hadoop.hbase.testclassification.SmallTests; +import org.junit.jupiter.api.Tag; +import org.junit.jupiter.api.Test; + +@Tag(SmallTests.TAG) +public class TestBackupInfoSerialization { + + private static BackupInfo newIncrementalBackupInfo() { + return new BackupInfo("backup_1234567890", BackupType.INCREMENTAL, + new TableName[] { TableName.valueOf("t1") }, "/hbase/backup"); + } + + private static BackupInfo newFullBackupInfo() { + return new BackupInfo("backup_1234567890", BackupType.FULL, + new TableName[] { TableName.valueOf("t1") }, "/hbase/backup"); + } + + @Test + public void testIncrBackupFileListSurvivesRoundTrip() throws IOException { + List walFiles = Arrays.asList("/hbase/oldWALs/host1.example.com,16020,1234567890.100", + "/hbase/oldWALs/host2.example.com,16020,1234567890.200"); + + BackupInfo original = newIncrementalBackupInfo(); + original.setIncrBackupFileList(walFiles); + + BackupInfo roundTripped = BackupInfo.fromByteArray(original.toByteArray()); + + assertEquals(walFiles, roundTripped.getIncrBackupFileList()); + } + + @Test + public void testIncrBackupFileListOrderIsPreserved() throws IOException { + List walFiles = Arrays.asList("/hbase/oldWALs/c.example.com,16020,1.300", + "/hbase/oldWALs/a.example.com,16020,1.100", "/hbase/oldWALs/b.example.com,16020,1.200"); + + BackupInfo original = newIncrementalBackupInfo(); + original.setIncrBackupFileList(walFiles); + + BackupInfo roundTripped = BackupInfo.fromByteArray(original.toByteArray()); + + assertEquals(walFiles, roundTripped.getIncrBackupFileList()); + } + + @Test + public void testUnsetIncrBackupFileListRemainsNull() throws IOException { + BackupInfo original = newIncrementalBackupInfo(); + + BackupInfo roundTripped = BackupInfo.fromByteArray(original.toByteArray()); + + assertNull(roundTripped.getIncrBackupFileList()); + } + + @Test + public void testEmptyIncrBackupFileListRemainsNull() throws IOException { + BackupInfo original = newIncrementalBackupInfo(); + original.setIncrBackupFileList(Arrays.asList()); + + BackupInfo roundTripped = BackupInfo.fromByteArray(original.toByteArray()); + + assertNull(roundTripped.getIncrBackupFileList()); + } + + @Test + public void testEqualsDistinguishesIncrBackupFileList() throws IOException { + BackupInfo withFiles = newIncrementalBackupInfo(); + withFiles.setIncrBackupFileList(Arrays.asList("/hbase/oldWALs/host1,16020,1.100")); + + BackupInfo withoutFiles = newIncrementalBackupInfo(); + + assertTrue(!withFiles.equals(withoutFiles)); + } + + @Test + public void testTotalBytesCopiedSurvivesRoundTrip() throws IOException { + BackupInfo original = newIncrementalBackupInfo(); + original.setTotalBytesCopied(9876543210L); + + BackupInfo roundTripped = BackupInfo.fromByteArray(original.toByteArray()); + + assertEquals(9876543210L, roundTripped.getTotalBytesCopied()); + } + + @Test + public void testNoChecksumVerifySurvivesRoundTripWhenTrue() throws IOException { + BackupInfo original = newIncrementalBackupInfo(); + original.setNoChecksumVerify(true); + + BackupInfo roundTripped = BackupInfo.fromByteArray(original.toByteArray()); + + assertTrue(roundTripped.getNoChecksumVerify()); + } + + @Test + public void testNoChecksumVerifySurvivesRoundTripWhenFalse() throws IOException { + BackupInfo original = newIncrementalBackupInfo(); + original.setNoChecksumVerify(false); + + BackupInfo roundTripped = BackupInfo.fromByteArray(original.toByteArray()); + + assertFalse(roundTripped.getNoChecksumVerify()); + } + + @Test + public void testIncrTimestampMapSurvivesRoundTrip() throws IOException { + Map t1Timestamps = new HashMap<>(); + t1Timestamps.put("host1.example.com:16020", 100L); + t1Timestamps.put("host2.example.com:16020", 200L); + Map t2Timestamps = new HashMap<>(); + t2Timestamps.put("host1.example.com:16020", 300L); + + Map> incrTimestampMap = new HashMap<>(); + incrTimestampMap.put(TableName.valueOf("t1"), t1Timestamps); + incrTimestampMap.put(TableName.valueOf("t2"), t2Timestamps); + + BackupInfo original = newIncrementalBackupInfo(); + original.setIncrTimestampMap(incrTimestampMap); + + BackupInfo roundTripped = BackupInfo.fromByteArray(original.toByteArray()); + + assertEquals(incrTimestampMap, roundTripped.getIncrTimestampMap()); + } + + @Test + public void testUnsetIncrTimestampMapRemainsNull() throws IOException { + BackupInfo original = newIncrementalBackupInfo(); + + BackupInfo roundTripped = BackupInfo.fromByteArray(original.toByteArray()); + + assertNull(roundTripped.getIncrTimestampMap()); + } + + @Test + public void testIncrementalBackupRetainsHLogTargetDir() throws IOException { + BackupInfo original = newIncrementalBackupInfo(); + + BackupInfo roundTripped = BackupInfo.fromByteArray(original.toByteArray()); + + assertEquals(original.getHLogTargetDir(), roundTripped.getHLogTargetDir()); + } + + @Test + public void testFullBackupHasNoHLogTargetDir() throws IOException { + BackupInfo original = newFullBackupInfo(); + assertNull(original.getHLogTargetDir()); + + BackupInfo roundTripped = BackupInfo.fromByteArray(original.toByteArray()); + + assertNull(roundTripped.getHLogTargetDir()); + } +} diff --git a/hbase-protocol-shaded/src/main/protobuf/Backup.proto b/hbase-protocol-shaded/src/main/protobuf/Backup.proto index a114001ba504..4a830f5d8cbf 100644 --- a/hbase-protocol-shaded/src/main/protobuf/Backup.proto +++ b/hbase-protocol-shaded/src/main/protobuf/Backup.proto @@ -93,6 +93,10 @@ message BackupInfo { optional uint32 workers_number = 11; optional uint64 bandwidth = 12; map table_set_timestamp = 13; + repeated string incr_backup_file_list = 14; + optional uint64 total_bytes_copied = 15; + optional bool no_checksum_verify = 16; + map incr_timestamp_map = 17; message RSTimestampMap { map rs_timestamp = 1; From 4cada866811c477737590f430e0f519d9c4ef7eb Mon Sep 17 00:00:00 2001 From: Charles Connell Date: Sat, 8 Aug 2026 11:57:29 -0400 Subject: [PATCH 2/2] simplify changes --- .../hadoop/hbase/backup/BackupInfo.java | 22 ++++++++----------- .../backup/TestBackupInfoSerialization.java | 14 ------------ 2 files changed, 9 insertions(+), 27 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 28b65dca4941..97ca0323fc90 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 @@ -519,7 +519,7 @@ public static BackupInfo fromProto(BackupProtos.BackupInfo proto) { BackupInfo context = new BackupInfo(); context.setBackupId(proto.getBackupId()); context.setBackupTableInfoMap(toMap(proto.getBackupTableInfoList())); - context.setTableSetTimestampMap(toTableTimestampMap(proto.getTableSetTimestampMap())); + context.setTableSetTimestampMap(getTableSetTimestampMap(proto.getTableSetTimestampMap())); context.setCompleteTs(proto.getCompleteTs()); if (proto.hasFailedMessage()) { context.setFailedMsg(proto.getFailedMessage()); @@ -528,13 +528,8 @@ public static BackupInfo fromProto(BackupProtos.BackupInfo proto) { context.setState(BackupInfo.BackupState.valueOf(proto.getBackupState().name())); } - // Only incremental backups have a WAL target directory. Setting this unconditionally would - // hand FULL backups a non-null path that never existed, which cleanupHLogDir() would then - // try to delete. - if (BackupType.valueOf(proto.getBackupType().name()) == BackupType.INCREMENTAL) { - context.setHLogTargetDir( - BackupUtils.getLogBackupDir(proto.getBackupRootDir(), proto.getBackupId())); - } + context + .setHLogTargetDir(BackupUtils.getLogBackupDir(proto.getBackupRootDir(), proto.getBackupId())); if (proto.hasBackupPhase()) { context.setPhase(BackupPhase.valueOf(proto.getBackupPhase().name())); @@ -553,7 +548,7 @@ public static BackupInfo fromProto(BackupProtos.BackupInfo proto) { context.setIncrBackupFileList(new ArrayList<>(proto.getIncrBackupFileListList())); } if (proto.getIncrTimestampMapCount() > 0) { - context.setIncrTimestampMap(toTableTimestampMap(proto.getIncrTimestampMapMap())); + context.setIncrTimestampMap(getTableSetTimestampMap(proto.getIncrTimestampMapMap())); } return context; } @@ -567,13 +562,14 @@ private static Map toMap(List> - toTableTimestampMap(Map map) { - Map> result = new HashMap<>(); + getTableSetTimestampMap(Map map) { + Map> tableSetTimestampMap = new HashMap<>(); for (Entry entry : map.entrySet()) { - result.put(TableName.valueOf(entry.getKey()), entry.getValue().getRsTimestampMap()); + tableSetTimestampMap.put(TableName.valueOf(entry.getKey()), + entry.getValue().getRsTimestampMap()); } - return result; + return tableSetTimestampMap; } public String getShortDescription() { diff --git a/hbase-backup/src/test/java/org/apache/hadoop/hbase/backup/TestBackupInfoSerialization.java b/hbase-backup/src/test/java/org/apache/hadoop/hbase/backup/TestBackupInfoSerialization.java index cd4d50880fba..7701dfafe317 100644 --- a/hbase-backup/src/test/java/org/apache/hadoop/hbase/backup/TestBackupInfoSerialization.java +++ b/hbase-backup/src/test/java/org/apache/hadoop/hbase/backup/TestBackupInfoSerialization.java @@ -40,11 +40,6 @@ private static BackupInfo newIncrementalBackupInfo() { new TableName[] { TableName.valueOf("t1") }, "/hbase/backup"); } - private static BackupInfo newFullBackupInfo() { - return new BackupInfo("backup_1234567890", BackupType.FULL, - new TableName[] { TableName.valueOf("t1") }, "/hbase/backup"); - } - @Test public void testIncrBackupFileListSurvivesRoundTrip() throws IOException { List walFiles = Arrays.asList("/hbase/oldWALs/host1.example.com,16020,1234567890.100", @@ -168,13 +163,4 @@ public void testIncrementalBackupRetainsHLogTargetDir() throws IOException { assertEquals(original.getHLogTargetDir(), roundTripped.getHLogTargetDir()); } - @Test - public void testFullBackupHasNoHLogTargetDir() throws IOException { - BackupInfo original = newFullBackupInfo(); - assertNull(original.getHLogTargetDir()); - - BackupInfo roundTripped = BackupInfo.fromByteArray(original.toByteArray()); - - assertNull(roundTripped.getHLogTargetDir()); - } }