Apache9 commented on code in PR #7075:
URL: https://github.com/apache/hbase/pull/7075#discussion_r2217107979
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/wal/AbstractRecoveredEditsOutputSink.java:
##########
@@ -111,6 +128,13 @@ protected Path
closeRecoveredEditsWriterAndFinalizeEdits(RecoveredEditsWriter ed
// TestHLogSplit#testThreading is an example.
Review Comment:
Not your fault, but we'd better change the test implementation instead of
adding more checks in the normal code path, since this will lead to a request
to NN...
Can be a separated issue.
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/wal/AbstractRecoveredEditsOutputSink.java:
##########
@@ -111,6 +128,13 @@ protected Path
closeRecoveredEditsWriterAndFinalizeEdits(RecoveredEditsWriter ed
// TestHLogSplit#testThreading is an example.
if (walSplitter.walFS.exists(editsWriter.path)) {
Review Comment:
I think we should use a loop inside the try block.
First, we try renaming, if it returns false, then we check which one has
less entries.
If it is the dst, we delete the dst, and then continue to the next loop.
If it is us, the tmp file, then we delete the tmp file, and return. I guess
we should marked the split task succeeded in this case but I'm not sure, please
double confirm.
And I think in this way, we could also reduce the pressure on HDFS's
namenode, in the currect implementation we do bunch of existence check...
--
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]
