You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@hbase.apache.org by "nirdosh0110 (via GitHub)" <gi...@apache.org> on 2026/09/01 04:39:37 UTC
[PR] HBASE-XXXXX Tolerate stale recovered.edits below durable seqid in split/merge [hbase]
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: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-30352: Tolerate stale recovered.edits below durable seqid in split/merge [hbase]
Posted by "Apache9 (via GitHub)" <gi...@apache.org>.
Apache9 commented on code in PR #8599:
URL: https://github.com/apache/hbase/pull/8599#discussion_r3999870623
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/AssignmentManagerUtil.java:
##########
@@ -294,10 +303,79 @@ static void removeNonDefaultReplicas(MasterProcedureEnv env, Stream<RegionInfo>
}
static void checkClosedRegion(MasterProcedureEnv env, RegionInfo regionInfo) throws IOException {
- if (WALSplitUtil.hasRecoveredEdits(env.getMasterConfiguration(), regionInfo)) {
- throw new IOException("Recovered.edits are found in Region: " + regionInfo
- + ", abort split/merge to prevent data loss");
+ if (!WALSplitUtil.hasRecoveredEdits(env.getMasterConfiguration(), regionInfo)) {
+ return;
}
+ // A recovered.edits file whose max seqid is <= the region's last flushed seqid is stale:
+ // its edits are already durable in HFiles. This happens e.g. when a graceful region move
+ // is followed by a WAL split of the source RS - the split creates recovered.edits for a
+ // region that has already been reopened elsewhere and flushed. Cleaning up such files here
+ // (rather than aborting split/merge) matches the tolerance HRegion itself applies at open.
+ if (tryDropStaleRecoveredEdits(env, regionInfo)) {
+ return;
+ }
+ throw new IOException("Recovered.edits are found in Region: " + regionInfo
+ + ", abort split/merge to prevent data loss");
+ }
+
+ /**
+ * Try to remove recovered.edits files that are provably below the region's last flushed seqid.
+ * @return true if, after cleanup, no recovered.edits remain for the region
+ */
+ private static boolean tryDropStaleRecoveredEdits(MasterProcedureEnv env, RegionInfo regionInfo) {
+ long durableSeqId = env.getMasterServices().getServerManager()
+ .getLastFlushedSequenceId(regionInfo.getEncodedNameAsBytes()).getLastFlushedSequenceId();
+ if (durableSeqId <= 0L) {
+ // No authoritative durability info at the master; play safe and let the caller abort.
+ return false;
+ }
+ try {
+ Configuration conf = env.getMasterConfiguration();
+ Path regionWALDir =
+ CommonFSUtils.getWALRegionDir(conf, regionInfo.getTable(), regionInfo.getEncodedName());
+ Path regionDir = FSUtils.getRegionDirFromRootDir(CommonFSUtils.getRootDir(conf), regionInfo);
+ Path wrongRegionWALDir = CommonFSUtils.getWrongWALRegionDir(conf, regionInfo.getTable(),
+ regionInfo.getEncodedName());
+ FileSystem walFs = CommonFSUtils.getWALFileSystem(conf);
+ FileSystem rootFs = CommonFSUtils.getRootDirFileSystem(conf);
+ return dropStaleEditsUnder(walFs, regionWALDir, durableSeqId, regionInfo)
+ && dropStaleEditsUnder(rootFs, regionDir, durableSeqId, regionInfo)
+ && dropStaleEditsUnder(walFs, wrongRegionWALDir, durableSeqId, regionInfo);
+ } catch (IOException e) {
+ LOG.warn("Failed to inspect recovered.edits for {}; falling back to abort", regionInfo, e);
+ return false;
+ }
+ }
+
+ private static boolean dropStaleEditsUnder(FileSystem fs, Path regionDir, long durableSeqId,
+ RegionInfo regionInfo) throws IOException {
+ NavigableSet<Path> files = WALSplitUtil.getSplitEditFilesSorted(fs, regionDir);
+ if (files.isEmpty()) {
+ return true;
+ }
+ for (Path p : files) {
+ long fileMaxSeqId;
+ try {
+ fileMaxSeqId = Long.parseLong(p.getName());
+ } catch (NumberFormatException e) {
+ LOG.warn("Non-numeric recovered.edits filename {} for {}; not dropping", p, regionInfo);
Review Comment:
Does this really hurt? When loading recovered.edits, we will abort or just skip these files?
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/AssignmentManagerUtil.java:
##########
@@ -294,10 +303,79 @@ static void removeNonDefaultReplicas(MasterProcedureEnv env, Stream<RegionInfo>
}
static void checkClosedRegion(MasterProcedureEnv env, RegionInfo regionInfo) throws IOException {
- if (WALSplitUtil.hasRecoveredEdits(env.getMasterConfiguration(), regionInfo)) {
- throw new IOException("Recovered.edits are found in Region: " + regionInfo
- + ", abort split/merge to prevent data loss");
+ if (!WALSplitUtil.hasRecoveredEdits(env.getMasterConfiguration(), regionInfo)) {
+ return;
}
+ // A recovered.edits file whose max seqid is <= the region's last flushed seqid is stale:
Review Comment:
I prefer we do not mention the details about how we can generate stale recoverd.edits, as in general, we should fix the problem.
Just add comment to say that this is for making the process more robust, there may be some corner cases where we left stale recovered.edits.
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/AssignmentManagerUtil.java:
##########
@@ -294,10 +303,79 @@ static void removeNonDefaultReplicas(MasterProcedureEnv env, Stream<RegionInfo>
}
static void checkClosedRegion(MasterProcedureEnv env, RegionInfo regionInfo) throws IOException {
- if (WALSplitUtil.hasRecoveredEdits(env.getMasterConfiguration(), regionInfo)) {
- throw new IOException("Recovered.edits are found in Region: " + regionInfo
- + ", abort split/merge to prevent data loss");
+ if (!WALSplitUtil.hasRecoveredEdits(env.getMasterConfiguration(), regionInfo)) {
+ return;
}
+ // A recovered.edits file whose max seqid is <= the region's last flushed seqid is stale:
+ // its edits are already durable in HFiles. This happens e.g. when a graceful region move
+ // is followed by a WAL split of the source RS - the split creates recovered.edits for a
+ // region that has already been reopened elsewhere and flushed. Cleaning up such files here
+ // (rather than aborting split/merge) matches the tolerance HRegion itself applies at open.
+ if (tryDropStaleRecoveredEdits(env, regionInfo)) {
+ return;
+ }
+ throw new IOException("Recovered.edits are found in Region: " + regionInfo
+ + ", abort split/merge to prevent data loss");
+ }
+
+ /**
+ * Try to remove recovered.edits files that are provably below the region's last flushed seqid.
+ * @return true if, after cleanup, no recovered.edits remain for the region
+ */
+ private static boolean tryDropStaleRecoveredEdits(MasterProcedureEnv env, RegionInfo regionInfo) {
+ long durableSeqId = env.getMasterServices().getServerManager()
+ .getLastFlushedSequenceId(regionInfo.getEncodedNameAsBytes()).getLastFlushedSequenceId();
+ if (durableSeqId <= 0L) {
Review Comment:
We can get this value by reading all the HFiles' metadata?
--
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: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-30335 Tolerate stale recovered.edits below durable seqid in split/merge [hbase]
Posted by "nirdosh0110 (via GitHub)" <gi...@apache.org>.
nirdosh0110 commented on PR #8599:
URL: https://github.com/apache/hbase/pull/8599#issuecomment-5541403489
@Apache9 @apurtell @virajjasani Pleasse help with review coments on this PR. TIA.
--
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: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-30335 Tolerate stale recovered.edits below durable seqid in split/merge [hbase]
Posted by "virajjasani (via GitHub)" <gi...@apache.org>.
virajjasani commented on PR #8599:
URL: https://github.com/apache/hbase/pull/8599#issuecomment-5636928253
@nirdosh0110 I just realized this PR also points to HBASE-30335, we will need new Jira for this PR, given the 1:1 mapping that we follow for each Jira/PR.
--
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: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-30352: Tolerate stale recovered.edits below durable seqid in split/merge [hbase]
Posted by "virajjasani (via GitHub)" <gi...@apache.org>.
virajjasani commented on PR #8599:
URL: https://github.com/apache/hbase/pull/8599#issuecomment-5853151760
@nirdosh0110 if we want this in branch-2.4 as well, we will need one more PR
--
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: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-30352: Tolerate stale recovered.edits below durable seqid in split/merge [hbase]
Posted by "nirdosh0110 (via GitHub)" <gi...@apache.org>.
nirdosh0110 commented on PR #8599:
URL: https://github.com/apache/hbase/pull/8599#issuecomment-5818229633
> @nirdosh0110 could you also prepare branch-2 PR?
@virajjasani Raised a PR: https://github.com/apache/hbase/pull/8699 → target branch apache/branch-2.
--
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: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-30352: Tolerate stale recovered.edits below durable seqid in split/merge [hbase]
Posted by "nirdosh0110 (via GitHub)" <gi...@apache.org>.
nirdosh0110 commented on PR #8599:
URL: https://github.com/apache/hbase/pull/8599#issuecomment-5637206532
> > @nirdosh0110 I just realized this PR also points to [HBASE-30335](https://issues.apache.org/jira/browse/HBASE-30335), we will need new Jira for this PR, given the 1:1 mapping that we follow for each Jira/PR.
>
> Thanks @virajjasani. Some how this PR is not auto linked to open jira. Below are the details. I've updated the PR title.
>
> * [HBASE-30352](https://issues.apache.org/jira/browse/HBASE-30352) PR: [HBASE-30352: Tolerate stale recovered.edits below durable seqid in split/merge #8599](https://github.com/apache/hbase/pull/8599)
Now it is linked properly.
--
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: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-30352: Tolerate stale recovered.edits below durable seqid in split/merge [hbase]
Posted by "nirdosh0110 (via GitHub)" <gi...@apache.org>.
nirdosh0110 commented on code in PR #8599:
URL: https://github.com/apache/hbase/pull/8599#discussion_r4000393841
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/AssignmentManagerUtil.java:
##########
@@ -294,10 +303,79 @@ static void removeNonDefaultReplicas(MasterProcedureEnv env, Stream<RegionInfo>
}
static void checkClosedRegion(MasterProcedureEnv env, RegionInfo regionInfo) throws IOException {
- if (WALSplitUtil.hasRecoveredEdits(env.getMasterConfiguration(), regionInfo)) {
- throw new IOException("Recovered.edits are found in Region: " + regionInfo
- + ", abort split/merge to prevent data loss");
+ if (!WALSplitUtil.hasRecoveredEdits(env.getMasterConfiguration(), regionInfo)) {
+ return;
}
+ // A recovered.edits file whose max seqid is <= the region's last flushed seqid is stale:
Review Comment:
Done — reframed as a generic robustness note without the incident-specific detail in c7e4623.
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/AssignmentManagerUtil.java:
##########
@@ -294,10 +303,79 @@ static void removeNonDefaultReplicas(MasterProcedureEnv env, Stream<RegionInfo>
}
static void checkClosedRegion(MasterProcedureEnv env, RegionInfo regionInfo) throws IOException {
- if (WALSplitUtil.hasRecoveredEdits(env.getMasterConfiguration(), regionInfo)) {
- throw new IOException("Recovered.edits are found in Region: " + regionInfo
- + ", abort split/merge to prevent data loss");
+ if (!WALSplitUtil.hasRecoveredEdits(env.getMasterConfiguration(), regionInfo)) {
+ return;
}
+ // A recovered.edits file whose max seqid is <= the region's last flushed seqid is stale:
+ // its edits are already durable in HFiles. This happens e.g. when a graceful region move
+ // is followed by a WAL split of the source RS - the split creates recovered.edits for a
+ // region that has already been reopened elsewhere and flushed. Cleaning up such files here
+ // (rather than aborting split/merge) matches the tolerance HRegion itself applies at open.
+ if (tryDropStaleRecoveredEdits(env, regionInfo)) {
+ return;
+ }
+ throw new IOException("Recovered.edits are found in Region: " + regionInfo
+ + ", abort split/merge to prevent data loss");
+ }
+
+ /**
+ * Try to remove recovered.edits files that are provably below the region's last flushed seqid.
+ * @return true if, after cleanup, no recovered.edits remain for the region
+ */
+ private static boolean tryDropStaleRecoveredEdits(MasterProcedureEnv env, RegionInfo regionInfo) {
+ long durableSeqId = env.getMasterServices().getServerManager()
+ .getLastFlushedSequenceId(regionInfo.getEncodedNameAsBytes()).getLastFlushedSequenceId();
+ if (durableSeqId <= 0L) {
Review Comment:
Good point. The reason for going with `ServerManager` here was consistency — SCP consults the same map when deciding what enters `recovered.edits` in the first place, so both decisions come from one source and cannot disagree. That said, HFile metadata is stronger: it is the on-disk truth and would decouple this check from HBASE-30335's seed timing (and from a cold `ServerManager` cache right after master restart).
Reading it correctly means `min across families of max HFile seqid` (the same fence `HRegion.replayRecoveredEditsIfAny` uses when skipping already-flushed edits). Roughly: `HRegionFileSystem.openRegionFromFileSystem` → per family `StoreFileTracker.load()` → wrap each `StoreFileInfo` in `HStoreFile` with `CacheConfig.DISABLED` → `getReader().getSequenceID()`. Same pattern `MergeTableRegionsProcedure.mergeStoreFiles` already uses in this package.
Would you prefer I switch entirely to the HFile-metadata source in this PR, or keep `ServerManager` primary with HFile-metadata as a fallback when the cache is empty? Happy to do either.
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/AssignmentManagerUtil.java:
##########
@@ -294,10 +303,79 @@ static void removeNonDefaultReplicas(MasterProcedureEnv env, Stream<RegionInfo>
}
static void checkClosedRegion(MasterProcedureEnv env, RegionInfo regionInfo) throws IOException {
- if (WALSplitUtil.hasRecoveredEdits(env.getMasterConfiguration(), regionInfo)) {
- throw new IOException("Recovered.edits are found in Region: " + regionInfo
- + ", abort split/merge to prevent data loss");
+ if (!WALSplitUtil.hasRecoveredEdits(env.getMasterConfiguration(), regionInfo)) {
+ return;
}
+ // A recovered.edits file whose max seqid is <= the region's last flushed seqid is stale:
+ // its edits are already durable in HFiles. This happens e.g. when a graceful region move
+ // is followed by a WAL split of the source RS - the split creates recovered.edits for a
+ // region that has already been reopened elsewhere and flushed. Cleaning up such files here
+ // (rather than aborting split/merge) matches the tolerance HRegion itself applies at open.
+ if (tryDropStaleRecoveredEdits(env, regionInfo)) {
+ return;
+ }
+ throw new IOException("Recovered.edits are found in Region: " + regionInfo
+ + ", abort split/merge to prevent data loss");
+ }
+
+ /**
+ * Try to remove recovered.edits files that are provably below the region's last flushed seqid.
+ * @return true if, after cleanup, no recovered.edits remain for the region
+ */
+ private static boolean tryDropStaleRecoveredEdits(MasterProcedureEnv env, RegionInfo regionInfo) {
+ long durableSeqId = env.getMasterServices().getServerManager()
+ .getLastFlushedSequenceId(regionInfo.getEncodedNameAsBytes()).getLastFlushedSequenceId();
+ if (durableSeqId <= 0L) {
+ // No authoritative durability info at the master; play safe and let the caller abort.
+ return false;
+ }
+ try {
+ Configuration conf = env.getMasterConfiguration();
+ Path regionWALDir =
+ CommonFSUtils.getWALRegionDir(conf, regionInfo.getTable(), regionInfo.getEncodedName());
+ Path regionDir = FSUtils.getRegionDirFromRootDir(CommonFSUtils.getRootDir(conf), regionInfo);
+ Path wrongRegionWALDir = CommonFSUtils.getWrongWALRegionDir(conf, regionInfo.getTable(),
+ regionInfo.getEncodedName());
+ FileSystem walFs = CommonFSUtils.getWALFileSystem(conf);
+ FileSystem rootFs = CommonFSUtils.getRootDirFileSystem(conf);
+ return dropStaleEditsUnder(walFs, regionWALDir, durableSeqId, regionInfo)
+ && dropStaleEditsUnder(rootFs, regionDir, durableSeqId, regionInfo)
+ && dropStaleEditsUnder(walFs, wrongRegionWALDir, durableSeqId, regionInfo);
+ } catch (IOException e) {
+ LOG.warn("Failed to inspect recovered.edits for {}; falling back to abort", regionInfo, e);
+ return false;
+ }
+ }
+
+ private static boolean dropStaleEditsUnder(FileSystem fs, Path regionDir, long durableSeqId,
+ RegionInfo regionInfo) throws IOException {
+ NavigableSet<Path> files = WALSplitUtil.getSplitEditFilesSorted(fs, regionDir);
+ if (files.isEmpty()) {
+ return true;
+ }
+ for (Path p : files) {
+ long fileMaxSeqId;
+ try {
+ fileMaxSeqId = Long.parseLong(p.getName());
+ } catch (NumberFormatException e) {
+ LOG.warn("Non-numeric recovered.edits filename {} for {}; not dropping", p, regionInfo);
Review Comment:
You're right — `WALSplitUtil.getSplitEditFilesSorted` already restricts filenames via `EDITFILES_NAME_PATTERN` (`-?[0-9]+`), so `Long.parseLong` cannot throw here. Removed the guard in c7e4623 and left a short note pointing at the pattern.
--
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: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-30352: Tolerate stale recovered.edits below durable seqid in split/merge [hbase]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #8599:
URL: https://github.com/apache/hbase/pull/8599#discussion_r4029735272
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/AssignmentManagerUtil.java:
##########
@@ -294,10 +303,72 @@ static void removeNonDefaultReplicas(MasterProcedureEnv env, Stream<RegionInfo>
}
static void checkClosedRegion(MasterProcedureEnv env, RegionInfo regionInfo) throws IOException {
- if (WALSplitUtil.hasRecoveredEdits(env.getMasterConfiguration(), regionInfo)) {
- throw new IOException("Recovered.edits are found in Region: " + regionInfo
- + ", abort split/merge to prevent data loss");
+ if (!WALSplitUtil.hasRecoveredEdits(env.getMasterConfiguration(), regionInfo)) {
+ return;
}
+ // Robustness: corner cases can leave behind recovered.edits whose max seqid is already
+ // covered by the region's durable seqid. Drop those and proceed instead of aborting.
+ if (tryDropStaleRecoveredEdits(env, regionInfo)) {
+ return;
+ }
+ throw new IOException("Recovered.edits are found in Region: " + regionInfo
+ + ", abort split/merge to prevent data loss");
+ }
+
+ /**
+ * Try to remove recovered.edits files that are provably below the region's last flushed seqid.
+ * @return true if, after cleanup, no recovered.edits remain for the region
+ */
+ private static boolean tryDropStaleRecoveredEdits(MasterProcedureEnv env, RegionInfo regionInfo) {
+ long durableSeqId = env.getMasterServices().getServerManager()
+ .getLastFlushedSequenceId(regionInfo.getEncodedNameAsBytes()).getLastFlushedSequenceId();
+ if (durableSeqId <= 0L) {
+ // No authoritative durability info at the master; play safe and let the caller abort.
+ return false;
+ }
+ try {
+ Configuration conf = env.getMasterConfiguration();
+ Path regionWALDir =
+ CommonFSUtils.getWALRegionDir(conf, regionInfo.getTable(), regionInfo.getEncodedName());
+ Path regionDir = FSUtils.getRegionDirFromRootDir(CommonFSUtils.getRootDir(conf), regionInfo);
+ Path wrongRegionWALDir = CommonFSUtils.getWrongWALRegionDir(conf, regionInfo.getTable(),
+ regionInfo.getEncodedName());
+ FileSystem walFs = CommonFSUtils.getWALFileSystem(conf);
+ FileSystem rootFs = CommonFSUtils.getRootDirFileSystem(conf);
+ return dropStaleEditsUnder(walFs, regionWALDir, durableSeqId, regionInfo)
+ && dropStaleEditsUnder(rootFs, regionDir, durableSeqId, regionInfo)
+ && dropStaleEditsUnder(walFs, wrongRegionWALDir, durableSeqId, regionInfo);
+ } catch (IOException e) {
+ LOG.warn("Failed to inspect recovered.edits for {}; falling back to abort", regionInfo, e);
+ return false;
+ }
+ }
+
+ private static boolean dropStaleEditsUnder(FileSystem fs, Path regionDir, long durableSeqId,
+ RegionInfo regionInfo) throws IOException {
+ NavigableSet<Path> files = WALSplitUtil.getSplitEditFilesSorted(fs, regionDir);
+ if (files.isEmpty()) {
+ return true;
+ }
+ for (Path p : files) {
+ // getSplitEditFilesSorted restricts filenames to WALSplitUtil.EDITFILES_NAME_PATTERN
+ // (`-?[0-9]+`), so parseLong cannot throw here.
+ long fileMaxSeqId = Long.parseLong(p.getName());
Review Comment:
`EDITFILES_NAME_PATTERN` only restricts the filename to digits; it does not guarantee that the value fits in a `long`. An oversized numeric filename therefore reaches this line and throws an unchecked `NumberFormatException`, bypassing the intended safe fallback and failing the split/merge procedure unexpectedly. Treat an unparseable sequence ID as non-stale and return `false`.
--
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: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-30352: Tolerate stale recovered.edits below durable seqid in split/merge [hbase]
Posted by "virajjasani (via GitHub)" <gi...@apache.org>.
virajjasani merged PR #8599:
URL: https://github.com/apache/hbase/pull/8599
--
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: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-30352: Tolerate stale recovered.edits below durable seqid in split/merge [hbase]
Posted by "nirdosh0110 (via GitHub)" <gi...@apache.org>.
nirdosh0110 commented on PR #8599:
URL: https://github.com/apache/hbase/pull/8599#issuecomment-5637186109
> @nirdosh0110 I just realized this PR also points to [HBASE-30335](https://issues.apache.org/jira/browse/HBASE-30335), we will need new Jira for this PR, given the 1:1 mapping that we follow for each Jira/PR.
Thanks @virajjasani. Some how this PR is not auto linked to open jira. Below are the details. I've updated the PR title.
- [HBASE-30352](https://issues.apache.org/jira/browse/HBASE-30352) PR: https://github.com/apache/hbase/pull/8599
--
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: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-30352: Tolerate stale recovered.edits below durable seqid in split/merge [hbase]
Posted by "virajjasani (via GitHub)" <gi...@apache.org>.
virajjasani commented on PR #8599:
URL: https://github.com/apache/hbase/pull/8599#issuecomment-5817190129
@nirdosh0110 could you also prepare branch-2 PR?
--
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: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org