Apache9 commented on code in PR #7617:
URL: https://github.com/apache/hbase/pull/7617#discussion_r3142213317
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSourceShipper.java:
##########
@@ -215,6 +275,9 @@ private void shipEdits(WALEntryBatch entryBatch) {
entryBatch.getNbOperations(), (endTimeNs - startTimeNs) / 1000000);
}
break;
+ } catch (IOException ioe) {
+ throw new ReplicationRuntimeException(
Review Comment:
Why not just throw IOException out? This is a private method, we are free to
change the method signature.
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSourceShipper.java:
##########
@@ -215,6 +275,9 @@ private void shipEdits(WALEntryBatch entryBatch) {
entryBatch.getNbOperations(), (endTimeNs - startTimeNs) / 1000000);
}
break;
+ } catch (IOException ioe) {
+ throw new ReplicationRuntimeException(
Review Comment:
OK, checked locally, cleanUpHFileRefs also throws IOException...
So here the implementation is incorrect.
Before the PR there is no critical exception thrown out(in updateLogPosition
we will call abort directly), so we just catch Exception and retry.
In the new logic, we need to make try catch section smaller, for example,
only wrap cleanUpHFileRefs and retry, and for persistLogPosition, just throw
the IOException out, and the upper layer decide what to do.
--
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]
