HBASE-30480 Backup code mixes up region server casing by DieterDP-ng · Pull Request #8784 · apache/hbase · GitHub
Skip to content

HBASE-30480 Backup code mixes up region server casing #8784

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
DieterDP-ng wants to merge 1 commit into
base: master
Choose a base branch
from
Open

HBASE-30480 Backup code mixes up region server casing #8784

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 10 additions & 2 deletions hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/impl/BackupSystemTable.java
View file
Open in desktop
4 changes: 2 additions & 2 deletions hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/impl/FullTableBackupClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -312,8 +312,8 @@ static Map<String, Long> computeLogBoundaries(FileSystem fs, Path walRootDir,
// was listed is found. If the live servers do not include every host that took part in the
// roll, the master's server list is incomplete, or a host stopped after the roll.
Set<ServerName> live = new HashSet<>(admin.getRegionServers());
Set<String> liveAddresses =
live.stream().map(sn -> sn.getAddress().toString()).collect(Collectors.toSet());
Set<String> liveAddresses = live.stream()
.map(sn -> BackupUtils.toHostKey(sn.getAddress().toString())).collect(Collectors.toSet());
if (!liveAddresses.containsAll(rolledHosts.keySet())) {
throw new IOException("Live region servers " + live + " do not include every host that took"
+ " part in this backup's log roll " + rolledHosts.keySet() + ". The master's server list"
Expand Down
5 changes: 3 additions & 2 deletions .../src/main/java/org/apache/hadoop/hbase/backup/regionserver/LogRollBackupSubprocedure.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import java.util.concurrent.Callable;
import org.apache.hadoop.hbase.backup.impl.BackupSystemTable;
import org.apache.hadoop.hbase.backup.master.LogRollMasterProcedureManager;
import org.apache.hadoop.hbase.backup.util.BackupUtils;
import org.apache.hadoop.hbase.client.Connection;
import org.apache.hadoop.hbase.errorhandling.ForeignException;
import org.apache.hadoop.hbase.errorhandling.ForeignExceptionDispatcher;
Expand Down Expand Up @@ -107,8 +108,8 @@ public Void call() throws Exception {
table.readRegionServerLastLogRollResult(backupRoot);
String host = rss.getServerName().getHostname();
int port = rss.getServerName().getPort();
String server = host + ":" + port;
Long sts = serverTimestampMap.get(host);
String server = BackupUtils.toHostKey(host + ":" + port);
Long sts = serverTimestampMap.get(server);
if (sts != null && sts > highest) {
LOG
.warn("Won't update server's last roll log result: current=" + sts + " new=" + highest);
Expand Down
3 changes: 2 additions & 1 deletion hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/util/BackupBoundaries.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,8 @@ public void update(BackupInfo backupInfo) {
for (TableName table : backupInfo.getTableSetTimestampMap().keySet()) {
for (Map.Entry<String, Long> entry : backupInfo.getTableSetTimestampMap().get(table)
.entrySet()) {
Address regionServerAddress = Address.fromString(entry.getKey());
Address regionServerAddress =
Address.fromString(BackupUtils.toHostKey(entry.getKey()));
Long logRollTs = entry.getValue();

Long storedTs = boundaries.get(regionServerAddress);
Expand Down
21 changes: 17 additions & 4 deletions hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/util/BackupUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
import java.util.Date;
import java.util.HashMap;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.Map.Entry;
import java.util.Set;
Expand Down Expand Up @@ -107,6 +108,18 @@ private BackupUtils() {
throw new AssertionError("Instantiating utility class...");
}

/**
* Returns the key under which the backup code tracks a region server. The key is the server's
* host:port with the hostname lowercased. WAL file names contain the lowercased hostname (see
* {@link ServerName#toString()}), while {@link ServerName#getAddress()} keeps the hostname's
* original case, so both spellings must map to the same key.
* @param hostAndPort host:port of a region server, in any case
* @return the lowercased host:port
*/
public static String toHostKey(String hostAndPort) {
return hostAndPort.toLowerCase(Locale.ROOT);
}

/**
* Loop through the RS log timestamp map for the tables, for each RS, find the min timestamp value
* for the RS among the tables.
Expand Down Expand Up @@ -196,7 +209,7 @@ public static String parseHostNameFromLogFile(Path p) {
} else {
ServerName sname = AbstractFSWALProvider.getServerNameFromWALDirectoryName(p);
if (sname != null) {
return sname.getAddress().toString();
return toHostKey(sname.getAddress().toString());
} else {
LOG.error("Skip log file (can't parse): " + p);
return null;
Expand Down Expand Up @@ -452,14 +465,14 @@ public static String parseHostFromOldLog(Path p) {
}
Path parent = p.getParent();
if (parent != null && ServerName.isFullServerName(parent.getName())) {
return ServerName.valueOf(parent.getName()).getAddress().toString();
return toHostKey(ServerName.valueOf(parent.getName()).getAddress().toString());
}
try {
String urlDecodedName = URLDecoder.decode(p.getName(), "UTF8");
Iterable<String> nameSplitsOnComma = Splitter.on(",").split(urlDecodedName);
String host = Iterables.get(nameSplitsOnComma, 0);
String port = Iterables.get(nameSplitsOnComma, 1);
return host + ":" + port;
return toHostKey(host + ":" + port);
} catch (Exception e) {
LOG.warn("Skip log file (can't parse): {}", p);
return null;
Expand Down Expand Up @@ -858,7 +871,7 @@ private static void logRollV2(Connection conn, String backupRootDir) throws IOEx
ServerName serverName = entry.getKey();
long newHighestWALFilenum = entry.getValue();

String address = serverName.getAddress().toString();
String address = toHostKey(serverName.getAddress().toString());
Long lastHighestWALFilenum = lastLogRollResult.get(address);
if (lastHighestWALFilenum != null && lastHighestWALFilenum > newHighestWALFilenum) {
LOG.warn("Won't update last roll log result for server {}: current = {}, new = {}",
Expand Down
12 changes: 8 additions & 4 deletions hbase-backup/src/test/java/org/apache/hadoop/hbase/backup/TestBackupOfflineRS.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@
import org.apache.hadoop.hbase.regionserver.HRegionServer;
import org.apache.hadoop.hbase.testclassification.LargeTests;
import org.apache.hadoop.hbase.util.Bytes;
import org.apache.hadoop.hbase.util.DNS;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Tag;
import org.junit.jupiter.api.Test;
Expand Down Expand Up @@ -81,6 +82,9 @@ public static void setUp() throws Exception {
conf1 = TEST_UTIL.getConfiguration();
conf1.setInt("hbase.regionserver.info.port", -1);
conf1.setInt(HMaster.HBASE_MASTER_CLEANER_INTERVAL, Integer.MAX_VALUE);
// Forces a hostname with a capital letter. This effectively repurposes the test to also
// verify that the backup system is not affected by case sensitivity issues in hostnames.
conf1.set(DNS.UNSAFE_RS_HOSTNAME_KEY, "LocalHost");
autoRestoreOnFailure = true;
useSecondCluster = false;
setUpHelper();
Expand Down Expand Up @@ -172,7 +176,7 @@ public void testBackupWithOfflineRS() throws Exception {
}

String offlineHost =
rsBeforeStop.getServerName().getHostname() + ":" + rsBeforeStop.getServerName().getPort();
BackupUtils.toHostKey(rsBeforeStop.getServerName().getAddress().toString());
LOG.info("Stopping RS: {}", offlineHost);

stopRegionServerAndWait(rsBeforeStop.getServerName());
Expand Down Expand Up @@ -229,7 +233,7 @@ public void testRSGoesOfflineAfterFullBackupBeforeIncremental() throws Exception
rsBeforeStop.getWalRoller().requestRollAll();
rsBeforeStop.getWalRoller().waitUntilWalRollFinished();
String offlineHost =
rsBeforeStop.getServerName().getHostname() + ":" + rsBeforeStop.getServerName().getPort();
BackupUtils.toHostKey(rsBeforeStop.getServerName().getAddress().toString());

stopRegionServerAndWait(rsBeforeStop.getServerName());

Expand Down Expand Up @@ -291,7 +295,7 @@ public void testTransientRSBeforeFullBackup() throws Exception {
transientRS.getWalRoller().requestRollAll();
transientRS.getWalRoller().waitUntilWalRollFinished();
String transientHost =
transientRS.getServerName().getHostname() + ":" + transientRS.getServerName().getPort();
BackupUtils.toHostKey(transientRS.getServerName().getAddress().toString());

stopRegionServerAndWait(transientRS.getServerName());

Expand Down Expand Up @@ -337,7 +341,7 @@ public void testTransientRSAfterFullBackupBeforeIncremental() throws Exception {
transientRS.getWalRoller().requestRollAll();
transientRS.getWalRoller().waitUntilWalRollFinished();
String transientHost =
transientRS.getServerName().getHostname() + ":" + transientRS.getServerName().getPort();
BackupUtils.toHostKey(transientRS.getServerName().getAddress().toString());

stopRegionServerAndWait(transientRS.getServerName());

Expand Down
16 changes: 16 additions & 0 deletions hbase-backup/src/test/java/org/apache/hadoop/hbase/backup/TestBackupSystemTable.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,22 @@ public void testRegionServerLastLogRollResults() throws IOException {
cleanBackupTable();
}

@Test
public void testMixedCaseHostKeysAreReadLowercase() throws IOException {
table.writeRegionServerLastLogRollResult("Host-A:1", 102L, "root");
table.writeRegionServerLastLogRollResult("host-a:1", 100L, "root");
assertEquals(Map.of("host-a:1", 102L), table.readRegionServerLastLogRollResult("root"));

Map<String, Long> rsTimestampMap = new HashMap<>();
rsTimestampMap.put("Host-A:1", 100L);
rsTimestampMap.put("host-a:1", 102L);
TableName t1 = TableName.valueOf("t1");
table.writeRegionServerLogTimestamp(Set.of(t1), rsTimestampMap, "root");
assertEquals(Map.of(t1, Map.of("host-a:1", 100L)), table.readLogTimestampMap("root"));

cleanBackupTable();
}

@Test
public void testIncrementalBackupTableSet() throws IOException {
TreeSet<TableName> tables1 = new TreeSet<>();
Expand Down
23 changes: 15 additions & 8 deletions hbase-backup/src/test/java/org/apache/hadoop/hbase/backup/TestBackupUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import java.time.ZoneId;
import java.util.ArrayList;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.Set;
import java.util.TimeZone;
Expand Down Expand Up @@ -104,20 +105,28 @@ public Path run() {

@Test
public void testFilesystemWalHostNameParsing() throws IOException {
String[] hosts =
new String[] { "10.20.30.40", "127.0.0.1", "localhost", "a-region-server.domain.com" };
String[] hosts = new String[] { "10.20.30.40", "127.0.0.1", "localhost",
"a-region-server.domain.com", "Other-Region-Server.Domain.com" };

Path walRootDir = CommonFSUtils.getWALRootDir(conf);
Path logDir = new Path(walRootDir, HConstants.HREGION_LOGDIR_NAME);
Path oldLogDir = new Path(walRootDir, HConstants.HREGION_OLDLOGDIR_NAME);

int port = 60030;
for (String host : hosts) {
// ServerName interning makes parsing the server's directory name return this instance, which
// keeps the hostname's case. The parsed key must be lowercase anyway.
ServerName serverName = ServerName.valueOf(host, port, 1234);
String expectedKey =
host.toLowerCase(Locale.ROOT) + Addressing.HOSTNAME_PORT_SEPARATOR + port;

Path testWalPath = new Path(new Path(logDir, serverName.toString()),
"wal" + BackupUtils.LOGNAME_SEPARATOR + EnvironmentEdgeManager.currentTime());
assertEquals(expectedKey, BackupUtils.parseHostNameFromLogFile(testWalPath));

Path testOldWalPath = new Path(oldLogDir,
serverName + BackupUtils.LOGNAME_SEPARATOR + EnvironmentEdgeManager.currentTime());
assertEquals(host + Addressing.HOSTNAME_PORT_SEPARATOR + port,
BackupUtils.parseHostFromOldLog(testOldWalPath));
assertEquals(expectedKey, BackupUtils.parseHostFromOldLog(testOldWalPath));

Path testMasterWalPath =
new Path(oldLogDir, testOldWalPath.getName() + MasterRegionFactory.ARCHIVED_WAL_SUFFIX);
Expand All @@ -127,13 +136,11 @@ public void testFilesystemWalHostNameParsing() throws IOException {
Path testOldWalWithRegionGroupingPath = new Path(oldLogDir,
serverName + BackupUtils.LOGNAME_SEPARATOR + serverName + BackupUtils.LOGNAME_SEPARATOR
+ "regiongroup-0" + BackupUtils.LOGNAME_SEPARATOR + EnvironmentEdgeManager.currentTime());
assertEquals(host + Addressing.HOSTNAME_PORT_SEPARATOR + port,
BackupUtils.parseHostFromOldLog(testOldWalWithRegionGroupingPath));
assertEquals(expectedKey, BackupUtils.parseHostFromOldLog(testOldWalWithRegionGroupingPath));

Path testOldWalInServerDirPath = new Path(new Path(oldLogDir, serverName.toString()),
"wal" + BackupUtils.LOGNAME_SEPARATOR + EnvironmentEdgeManager.currentTime());
assertEquals(host + Addressing.HOSTNAME_PORT_SEPARATOR + port,
BackupUtils.parseHostFromOldLog(testOldWalInServerDirPath));
assertEquals(expectedKey, BackupUtils.parseHostFromOldLog(testOldWalInServerDirPath));
}
}

Expand Down
3 changes: 2 additions & 1 deletion hbase-backup/src/test/java/org/apache/hadoop/hbase/backup/TestIncrementalBackupManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,8 @@ private void testCollectWALFiles(boolean separateOldLogDir) throws Exception {
Map<String, Long> previousTimestamps =
BackupUtils.getRSLogTimestampMins(manager.readLogTimestampMap());
ServerName serverName = TEST_UTIL.getMiniHBaseCluster().getRegionServer(0).getServerName();
Long previousTimestamp = previousTimestamps.get(serverName.getAddress().toString());
Long previousTimestamp =
previousTimestamps.get(BackupUtils.toHostKey(serverName.getAddress().toString()));
assertNotNull(previousTimestamp);

TEST_UTIL.waitFor(30_000,
Expand Down
16 changes: 16 additions & 0 deletions hbase-backup/src/test/java/org/apache/hadoop/hbase/backup/master/TestBackupLogCleaner.java
View file
Open in desktop
Loading