Skip to content

Commit 65a6d8a

Browse files
authored
HBASE-29003 Proper bulk load tracking (#6506)
Signed-off-by: Ray Mattingly <[email protected]> The HBase backup mechanism keeps track of which HFiles were bulk loaded, so they can be included in incremental backups. Before this ticket, these bulk load records were only deleted when an incremental backup is created. This commit adds 2 more locations: 1) after a full backup. Since a full backup already captures all data, this meant that unnecessary HFiles were being included in the next incremental backup. 2) after a table delete/truncate/CF-deletion. Previously, if an HFile was loaded before a table was cleared, the next incremental backup would effectively still include the HFile. This lead to incorrect data being restored. This commit also completely refactors & simplifies the test for this functionality.
1 parent abc8b43 commit 65a6d8a

File tree

8 files changed

+272
-126
lines changed

8 files changed

+272
-126
lines changed
Lines changed: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,116 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing, software
13+
* distributed under the License is distributed on an "AS IS" BASIS,
14+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
15+
* See the License for the specific language governing permissions and
16+
* limitations under the License.
17+
*/
18+
package org.apache.hadoop.hbase.backup;
19+
20+
import java.io.IOException;
21+
import java.util.Arrays;
22+
import java.util.List;
23+
import java.util.Optional;
24+
import java.util.Set;
25+
import java.util.function.Predicate;
26+
import java.util.stream.Collectors;
27+
import org.apache.hadoop.conf.Configuration;
28+
import org.apache.hadoop.hbase.HBaseInterfaceAudience;
29+
import org.apache.hadoop.hbase.TableName;
30+
import org.apache.hadoop.hbase.backup.impl.BackupManager;
31+
import org.apache.hadoop.hbase.backup.impl.BackupSystemTable;
32+
import org.apache.hadoop.hbase.backup.impl.BulkLoad;
33+
import org.apache.hadoop.hbase.client.ColumnFamilyDescriptor;
34+
import org.apache.hadoop.hbase.client.Connection;
35+
import org.apache.hadoop.hbase.client.ConnectionFactory;
36+
import org.apache.hadoop.hbase.client.TableDescriptor;
37+
import org.apache.hadoop.hbase.coprocessor.MasterCoprocessor;
38+
import org.apache.hadoop.hbase.coprocessor.MasterCoprocessorEnvironment;
39+
import org.apache.hadoop.hbase.coprocessor.MasterObserver;
40+
import org.apache.hadoop.hbase.coprocessor.ObserverContext;
41+
import org.apache.yetus.audience.InterfaceAudience;
42+
import org.slf4j.Logger;
43+
import org.slf4j.LoggerFactory;
44+
45+
import org.apache.hbase.thirdparty.com.google.common.collect.Sets;
46+
47+
/**
48+
* An Observer to facilitate backup operations
49+
*/
50+
@InterfaceAudience.LimitedPrivate(HBaseInterfaceAudience.CONFIG)
51+
public class BackupMasterObserver implements MasterCoprocessor, MasterObserver {
52+
private static final Logger LOG = LoggerFactory.getLogger(BackupMasterObserver.class);
53+
54+
@Override
55+
public Optional<MasterObserver> getMasterObserver() {
56+
return Optional.of(this);
57+
}
58+
59+
@Override
60+
public void postDeleteTable(ObserverContext<MasterCoprocessorEnvironment> ctx,
61+
TableName tableName) throws IOException {
62+
Configuration cfg = ctx.getEnvironment().getConfiguration();
63+
if (!BackupManager.isBackupEnabled(cfg)) {
64+
LOG.debug("Skipping postDeleteTable hook since backup is disabled");
65+
return;
66+
}
67+
deleteBulkLoads(cfg, tableName, (ignored) -> true);
68+
}
69+
70+
@Override
71+
public void postTruncateTable(ObserverContext<MasterCoprocessorEnvironment> ctx,
72+
TableName tableName) throws IOException {
73+
Configuration cfg = ctx.getEnvironment().getConfiguration();
74+
if (!BackupManager.isBackupEnabled(cfg)) {
75+
LOG.debug("Skipping postTruncateTable hook since backup is disabled");
76+
return;
77+
}
78+
deleteBulkLoads(cfg, tableName, (ignored) -> true);
79+
}
80+
81+
@Override
82+
public void postModifyTable(final ObserverContext<MasterCoprocessorEnvironment> ctx,
83+
final TableName tableName, TableDescriptor oldDescriptor, TableDescriptor currentDescriptor)
84+
throws IOException {
85+
Configuration cfg = ctx.getEnvironment().getConfiguration();
86+
if (!BackupManager.isBackupEnabled(cfg)) {
87+
LOG.debug("Skipping postModifyTable hook since backup is disabled");
88+
return;
89+
}
90+
91+
Set<String> oldFamilies = Arrays.stream(oldDescriptor.getColumnFamilies())
92+
.map(ColumnFamilyDescriptor::getNameAsString).collect(Collectors.toSet());
93+
Set<String> newFamilies = Arrays.stream(currentDescriptor.getColumnFamilies())
94+
.map(ColumnFamilyDescriptor::getNameAsString).collect(Collectors.toSet());
95+
96+
Set<String> removedFamilies = Sets.difference(oldFamilies, newFamilies);
97+
if (!removedFamilies.isEmpty()) {
98+
Predicate<BulkLoad> filter = bulkload -> removedFamilies.contains(bulkload.getColumnFamily());
99+
deleteBulkLoads(cfg, tableName, filter);
100+
}
101+
}
102+
103+
/**
104+
* Deletes all bulk load entries for the given table, matching the provided predicate.
105+
*/
106+
private void deleteBulkLoads(Configuration config, TableName tableName,
107+
Predicate<BulkLoad> filter) throws IOException {
108+
try (Connection connection = ConnectionFactory.createConnection(config);
109+
BackupSystemTable tbl = new BackupSystemTable(connection)) {
110+
List<BulkLoad> bulkLoads = tbl.readBulkloadRows(List.of(tableName));
111+
List<byte[]> rowsToDelete =
112+
bulkLoads.stream().filter(filter).map(BulkLoad::getRowKey).toList();
113+
tbl.deleteBulkLoadedRows(rowsToDelete);
114+
}
115+
}
116+
}

hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/BackupRestoreConstants.java

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
package org.apache.hadoop.hbase.backup;
1919

2020
import org.apache.hadoop.hbase.HConstants;
21+
import org.apache.hadoop.hbase.coprocessor.CoprocessorHost;
2122
import org.apache.yetus.audience.InterfaceAudience;
2223

2324
/**
@@ -98,16 +99,17 @@ public interface BackupRestoreConstants {
9899

99100
String JOB_NAME_CONF_KEY = "mapreduce.job.name";
100101

101-
String BACKUP_CONFIG_STRING =
102-
BackupRestoreConstants.BACKUP_ENABLE_KEY + "=true\n" + "hbase.master.logcleaner.plugins="
103-
+ "YOUR_PLUGINS,org.apache.hadoop.hbase.backup.master.BackupLogCleaner\n"
104-
+ "hbase.procedure.master.classes=YOUR_CLASSES,"
105-
+ "org.apache.hadoop.hbase.backup.master.LogRollMasterProcedureManager\n"
106-
+ "hbase.procedure.regionserver.classes=YOUR_CLASSES,"
107-
+ "org.apache.hadoop.hbase.backup.regionserver.LogRollRegionServerProcedureManager\n"
108-
+ "hbase.coprocessor.region.classes=YOUR_CLASSES,"
109-
+ "org.apache.hadoop.hbase.backup.BackupObserver\n" + "and restart the cluster\n"
110-
+ "For more information please see http://hbase.apache.org/book.html#backuprestore\n";
102+
String BACKUP_CONFIG_STRING = BackupRestoreConstants.BACKUP_ENABLE_KEY + "=true\n"
103+
+ "hbase.master.logcleaner.plugins="
104+
+ "YOUR_PLUGINS,org.apache.hadoop.hbase.backup.master.BackupLogCleaner\n"
105+
+ "hbase.procedure.master.classes=YOUR_CLASSES,"
106+
+ "org.apache.hadoop.hbase.backup.master.LogRollMasterProcedureManager\n"
107+
+ "hbase.procedure.regionserver.classes=YOUR_CLASSES,"
108+
+ "org.apache.hadoop.hbase.backup.regionserver.LogRollRegionServerProcedureManager\n"
109+
+ CoprocessorHost.REGION_COPROCESSOR_CONF_KEY + "=YOUR_CLASSES,"
110+
+ BackupObserver.class.getSimpleName() + "\n" + CoprocessorHost.MASTER_COPROCESSOR_CONF_KEY
111+
+ "=YOUR_CLASSES," + BackupMasterObserver.class.getSimpleName() + "\nand restart the cluster\n"
112+
+ "For more information please see http://hbase.apache.org/book.html#backuprestore\n";
111113
String ENABLE_BACKUP = "Backup is not enabled. To enable backup, " + "in hbase-site.xml, set:\n "
112114
+ BACKUP_CONFIG_STRING;
113115

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

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -119,11 +119,17 @@ public static void decorateMasterConfiguration(Configuration conf) {
119119
plugins = conf.get(HFileCleaner.MASTER_HFILE_CLEANER_PLUGINS);
120120
conf.set(HFileCleaner.MASTER_HFILE_CLEANER_PLUGINS,
121121
(plugins == null ? "" : plugins + ",") + BackupHFileCleaner.class.getName());
122+
123+
String observerClass = BackupObserver.class.getName();
124+
String masterCoProc = conf.get(CoprocessorHost.MASTER_COPROCESSOR_CONF_KEY);
125+
conf.set(CoprocessorHost.MASTER_COPROCESSOR_CONF_KEY,
126+
(masterCoProc == null ? "" : masterCoProc + ",") + observerClass);
127+
122128
if (LOG.isDebugEnabled()) {
123129
LOG.debug(
124130
"Added log cleaner: {}. Added master procedure manager: {}."
125-
+ "Added master procedure manager: {}",
126-
cleanerClass, masterProcedureClass, BackupHFileCleaner.class.getName());
131+
+ " Added master procedure manager: {}. Added master observer: {}",
132+
cleanerClass, masterProcedureClass, BackupHFileCleaner.class.getName(), observerClass);
127133
}
128134
}
129135

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

Lines changed: 9 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -411,25 +411,24 @@ public void registerBulkLoad(TableName tableName, byte[] region,
411411
try (BufferedMutator bufferedMutator = connection.getBufferedMutator(bulkLoadTableName)) {
412412
List<Put> puts = BackupSystemTable.createPutForBulkLoad(tableName, region, cfToHfilePath);
413413
bufferedMutator.mutate(puts);
414-
LOG.debug("Written {} rows for bulk load of {}", puts.size(), tableName);
414+
LOG.debug("Written {} rows for bulk load of table {}", puts.size(), tableName);
415415
}
416416
}
417417

418-
/*
419-
* Removes rows recording bulk loaded hfiles from backup table
420-
* @param lst list of table names
421-
* @param rows the rows to be deleted
418+
/**
419+
* Removes entries from the table that tracks all bulk loaded hfiles.
420+
* @param rows the row keys of the entries to be deleted
422421
*/
423422
public void deleteBulkLoadedRows(List<byte[]> rows) throws IOException {
424423
try (BufferedMutator bufferedMutator = connection.getBufferedMutator(bulkLoadTableName)) {
425-
List<Delete> lstDels = new ArrayList<>();
424+
List<Delete> deletes = new ArrayList<>();
426425
for (byte[] row : rows) {
427426
Delete del = new Delete(row);
428-
lstDels.add(del);
429-
LOG.debug("orig deleting the row: " + Bytes.toString(row));
427+
deletes.add(del);
428+
LOG.debug("Deleting bulk load entry with key: {}", Bytes.toString(row));
430429
}
431-
bufferedMutator.mutate(lstDels);
432-
LOG.debug("deleted " + rows.size() + " original bulkload rows");
430+
bufferedMutator.mutate(deletes);
431+
LOG.debug("Deleted {} bulk load entries.", rows.size());
433432
}
434433
}
435434

@@ -1522,16 +1521,6 @@ public static void deleteSnapshot(Connection conn) throws IOException {
15221521
}
15231522
}
15241523

1525-
public static List<Delete> createDeleteForOrigBulkLoad(List<TableName> lst) {
1526-
List<Delete> lstDels = new ArrayList<>(lst.size());
1527-
for (TableName table : lst) {
1528-
Delete del = new Delete(rowkey(BULK_LOAD_PREFIX, table.toString(), BLK_LD_DELIM));
1529-
del.addFamily(BackupSystemTable.META_FAMILY);
1530-
lstDels.add(del);
1531-
}
1532-
return lstDels;
1533-
}
1534-
15351524
private Put createPutForDeleteOperation(String[] backupIdList) {
15361525
byte[] value = Bytes.toBytes(StringUtils.join(backupIdList, ","));
15371526
Put put = new Put(DELETE_OP_ROW);

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

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
import java.io.IOException;
2727
import java.util.ArrayList;
2828
import java.util.HashMap;
29+
import java.util.List;
2930
import java.util.Map;
3031
import org.apache.hadoop.hbase.TableName;
3132
import org.apache.hadoop.hbase.backup.BackupCopyJob;
@@ -152,6 +153,11 @@ public void execute() throws IOException {
152153
// the snapshot.
153154
LOG.info("Execute roll log procedure for full backup ...");
154155

156+
// Gather the bulk loads being tracked by the system, which can be deleted (since their data
157+
// will be part of the snapshot being taken). We gather this list before taking the actual
158+
// snapshots for the same reason as the log rolls.
159+
List<BulkLoad> bulkLoadsToDelete = backupManager.readBulkloadRows(tableList);
160+
155161
Map<String, String> props = new HashMap<>();
156162
props.put("backupRoot", backupInfo.getBackupRootDir());
157163
admin.execProcedure(LogRollMasterProcedureManager.ROLLLOG_PROCEDURE_SIGNATURE,
@@ -192,6 +198,9 @@ public void execute() throws IOException {
192198
BackupUtils.getMinValue(BackupUtils.getRSLogTimestampMins(newTableSetTimestampMap));
193199
backupManager.writeBackupStartCode(newStartCode);
194200

201+
backupManager
202+
.deleteBulkLoadedRows(bulkLoadsToDelete.stream().map(BulkLoad::getRowKey).toList());
203+
195204
// backup complete
196205
completeBackup(conn, backupInfo, BackupType.FULL, conf);
197206
} catch (Exception e) {

0 commit comments

Comments
 (0)