Dieter De Paepe created HBASE-30480:
---------------------------------------

             Summary: Backup code mixes up region server casing
                 Key: HBASE-30480
                 URL: https://issues.apache.org/jira/browse/HBASE-30480
             Project: HBase
          Issue Type: Bug
          Components: backup&restore
            Reporter: Dieter De Paepe


This issue was originally discovered while reviewing HBASE-29776 
(https://github.com/apache/hbase/pull/8694), so some references in the ticket
might point to code of that PR. The core issue also affects master, see below. 
The description below is created with help from AI.

The backup code keys region servers by host:port strings. Those strings come 
from two sources that disagree on the hostname's case:

- ServerName#toString() lowercases the hostname (ServerName.java:240). WAL file 
names and WAL directory names are built from it, so they are always
  lowercase.
- ServerName#getAddress()#toString() keeps the hostname's original case. 
Log-roll results are stored under this string (BackupUtils#logRollV2 uses
  serverName.getAddress().toString()).

The hostname dieter-Precision therefore shows up as both dieter-Precision:37979 
(roll results and stored boundaries) and
dieter-precision:37979 (parsed from WAL paths). The maps holding these keys are 
plain HashMap<String, Long>, so a lookup with one spelling
misses an entry stored under the other.

Two details make the result vary between runs:

- ServerName instances are interned in a weak interner (ServerName.java:100, 
:138, :159), and ServerName#equals ignores case (ServerName.java:262,
  :280). ServerName.valueOf(<lowercase dir name>) can return an older, 
mixed-case instance if one is still alive, so getAddress() comes back in
  either case.
- Address#equals is case-sensitive (Address.java:119-125).

*Observed effects on branch HBASE-29776*

- Stored boundaries mix both spellings. After a full backup, 
readLogTimestampMap returned
  \{dieter-precision:35257=…, dieter-Precision:34739=…, 
dieter-precision:35491=…, dieter-precision:45447=…}.
- Lookups miss and the code falls back to the oldest boundary. In 
IncrementalBackupManager#getLogFilesForNewBackup, parseHostFromOldLog returns 
the
  lowercase key for archived WALs. olderTimestamps.get(host) then returns null. 
The code treats the host as unknown and only filters against
  prevBackupStartTs, so WALs from before the previous full backup get into the 
incremental.
- The boundary loop adds a phantom host. The new boundary-update loop in 
getIncrBackupLogFileMap writes a second, lowercase key for the same live
  server and logs it as an "inactive host".
- TestIncrementalBackupRestore fails. The baseline (8315e2fac3) failed 3 of 4 
runs with
  Unmatched family names found: unmatched family names in HFiles to be 
bulkloaded: [f3].
  The incremental image picked up a WAL that was written before a full backup 
and still holds edits for a column family dropped since then.
- TestBackupOfflineRS fails 4/4. The tests build the expected key from 
getServerName().getHostname(), and that spelling doesn't match the stored
  key.

Both test classes pass consistently with this workaround in the test 
configuration:
conf1.set("hbase.unsafe.regionserver.hostname", "localhost");
conf1.set("hbase.master.hostname", "localhost");

*Observed effects on master:*

1. Incremental backups skip archived WALs (data loss)

Archived WAL names come from ServerName#toString(), which lowercases the 
hostname. BackupUtils#parseHostFromOldLog therefore returns
dieter-precision:37533. The log-roll results and stored boundaries use 
ServerName#getAddress()#toString(), which keeps the case:
dieter-Precision:37533.

In IncrementalBackupManager#getLogFilesForNewBackup, newestTimestamps.get(host) 
misses for every archived WAL. The newTimestamp == null ||
currentLogTS > newTimestamp branch then excludes the WAL. The same lookup 
misses in every later incremental, the WAL is never backed up.

Test TestIncrementalBackupArchivedWAL:
1. Take a full backup.
2. Write 50 rows.
3. Flush and roll until the WAL holding those rows is in oldWALs.
4. Take an incremental backup, restore it, and compare row counts.

Results:
- Mixed-case hostname: fails. The restored table has 99 rows, expected 149. The 
incremental only contains the active WAL,
  WALs/dieter-precision,37533,…/….1792056346432.
- localhost: passes. The incremental also contains 
oldWALs/localhost%2C43943%2C….1792056464569.

2. BackupLogCleaner misses the boundary for archived WALs (WALs kept too long, 
no early deletion)

BackupBoundaries#isDeletable turns the host parsed from the WAL path into an 
Address and looks it up in a map keyed by Address.fromString(<stored
key>). Address#equals and Address#hashCode use the hostname as-is, so the 
lookup misses. The cleaner then judges the WAL against defaultBoundary,
which is the oldest boundary of any server in the newest backup per root (or 
the start time of a running backup, if older), minus
hbase.backup.log.cleaner.timestamp.buffer.ms (default 1 hour).

The default boundary is never newer than a server's own boundary, so the miss 
can't make the cleaner delete a WAL that a backup still needs. The
effect is over-retention in oldWALs. How long depends on the oldest boundary of 
any server.

Not verified, from reading the code: on master, the stored boundaries contain 
every log-roll result ever written for the backup root, and that list
is never pruned (newTimestamps = readRegionServerLastLogRollResult()). If a 
server that left the cluster long ago keeps an old entry,
defaultBoundary stays old. Archived WALs of mixed-case hosts would then build 
up in oldWALs.

Interaction between the two: an archived WAL that the incremental backup 
skipped is still eventually deleted. That happens once defaultBoundary
moves past its timestamp. The cleaner doesn't cause the data loss, but it does 
make it permanent.

Not tested: clusters with hbase.separate.oldlogdir.by.regionserver=true. In 
that layout, parseHostFromOldLog takes the host from the parent
directory through ServerName.valueOf(...). ServerName interning can then return 
either case, so the behaviour may vary between runs.

*Impact*

This affects clusters whose region server hostnames contain uppercase letters. 
DNS treats hostnames case-insensitively, so such names are valid and
do occur (for example, default machine names like dieter-Precision). The 
confirmed result is incremental backups containing the wrong WALs. The
possible result is WALs missing from backups or deleted too early.

*Suggested direction*

Normalize the host:port key in one place, for example by lowercasing it in a 
single helper. Use that helper wherever the backup code writes or looks
up per-server state:
- log-roll results (BackupUtils#logRollV2 and the legacy roll procedure);
- WAL path parsing (BackupUtils#parseHostNameFromLogFile, #parseHostFromOldLog);
- stored boundaries and BackupBoundaries.

Existing backup system tables may already hold mixed-case keys, so reads should 
normalize as well.

*Reproduction*

Run TestIncrementalBackupRestore or TestBackupOfflineRS (hbase-backup) on a 
machine whose hostname contains uppercase letters, without the localhost
workaround.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

Reply via email to