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