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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
