nirdosh0110 opened a new pull request, #8599: URL: https://github.com/apache/hbase/pull/8599
## Context Follow-up to the discussion on #8584 (HBASE-30335). @Apache9 suggested in https://github.com/apache/hbase/pull/8584#issuecomment-5487364435: > 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. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
