[
https://issues.apache.org/jira/browse/HBASE-30480?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Dieter De Paepe updated HBASE-30480:
------------------------------------
Priority: Critical (was: Major)
> 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
> Priority: Critical
>
> 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)
