Skip to content

HBASE-XXXXX Tolerate stale recovered.edits below durable seqid in split/merge - #8599

Draft
nirdosh0110 wants to merge 1 commit into
apache:masterfrom
nirdosh0110:drop-stale-recovered-edits
Draft

HBASE-XXXXX Tolerate stale recovered.edits below durable seqid in split/merge#8599
nirdosh0110 wants to merge 1 commit into
apache:masterfrom
nirdosh0110:drop-stale-recovered-edits

Conversation

@nirdosh0110

Copy link
Copy Markdown
Contributor

Context

Follow-up to the discussion on #8584 (HBASE-30335). @Apache9 suggested in #8584 (comment):

In MergeTableRegionsProcedure, when we have a recovered.edits file, we should check if the edits are all below the persistent seqNum, if so we are OK to remove the directory and go on.

This PR implements that check for both MergeTableRegionsProcedure and SplitTableRegionProcedure (both call AssignmentManagerUtil.checkClosedRegion).

Draft: JIRA not yet filed — will fill in the number and force-push the title once assigned.

The incident this addresses

  1. Region 112d9f08 was gracefully moved from RS-A → RS-B.
  2. RS-B opened the region with openSeqNum=4997750282; all prior edits were durable in HFiles.
  3. ~25 s later, RS-A was declared dead and its WAL was split.
  4. The split worker produced a recovered.edits file for this region containing seqId=4997750280 — an edit already flushed on HFile before RS-A closed.
  5. MergeTableRegionsProcedure later hit MERGE_TABLE_REGIONS_CHECK_CLOSED_REGIONS, saw the recovered.edits file, and threw. The region sat in CLOSED/RIT for ~49 min until master failover cleared it.

Change

AssignmentManagerUtil.checkClosedRegion now, when hasRecoveredEdits is true:

  1. Reads the region's lastFlushedSequenceId from ServerManager.
  2. For each recovered.edits file in every candidate location (WAL region dir, root region dir, legacy "wrong" WAL region dir), parses the filename — which is formatRecoveredEditsFileName(maxEditWALSeqNum) — to get the file's max seqid.
  3. If every file's max seqid ≤ lastFlushedSequenceId, deletes those specific files and returns.
  4. Otherwise the pre-existing abort behavior is preserved as a safe fallback (unknown/no durable info, or a file that could contain non-durable edits).

Filename parsing avoids opening/reading the WAL edits; the writer contract already encodes the max seqid in the file name via WALSplitUtil.getCompletedRecoveredEditsFilePath.

Files

  • hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/AssignmentManagerUtil.java — the tolerance logic.
  • hbase-server/src/test/java/org/apache/hadoop/hbase/master/assignment/TestAssignmentManagerUtil.java — new test testCheckClosedRegionDropsStaleRecoveredEdits verifies (a) a stale file is dropped and the check passes, (b) a file with a fresh seqid still causes the abort.

Local run

TestAssignmentManagerUtil - 3 tests / 0 failures / 0 errors / 0 skipped - 15.14 s

Notes / open questions

  • The improvement only takes effect when ServerManager has an authoritative lastFlushedSequenceId for the region. With HBASE-30335 landing, this will be the case immediately after region OPEN. Without HBASE-30335 it kicks in after the first flush heartbeat. In either case, the fallback matches today's behavior.
  • Not implemented here (Apache9's other suggested item): proactively removing stale recovered.edits on region OPEN. Happy to file/pick that up separately if reviewers agree it should be in scope.

…it/merge

MergeTableRegionsProcedure and SplitTableRegionProcedure invoke
AssignmentManagerUtil.checkClosedRegion during MERGE/SPLIT_CHECK_CLOSED_REGIONS.
Today the check aborts the procedure if any recovered.edits file exists in the
region directory, retaining the region lock and leaving the region CLOSED/RIT
until the next master failover.

This can happen on a graceful region move followed by the source RS's WAL split
completing after the region has already reopened - and flushed - on the target
RS. The recovered.edits file contains edits already durable in HFiles; the
existence of the file is harmless but the check treats it as data-loss risk.

Change checkClosedRegion so that, when recovered.edits are present, it consults
ServerManager.getLastFlushedSequenceId for the region and inspects each
recovered.edits filename (whose numeric name is the max seqid of edits in the
file). If every file's max seqid is <= the region's durable seqid the files are
removed and the procedure proceeds. Otherwise the previous abort behavior is
preserved as a safe fallback.

Follow-up to the discussion on PR apache#8584 (HBASE-30335).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant