Apache9 commented on code in PR #6129:
URL: https://github.com/apache/hbase/pull/6129#discussion_r1712643892
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/handler/UnassignRegionHandler.java:
##########
@@ -63,20 +63,25 @@ public class UnassignRegionHandler extends EventHandler {
private boolean evictCache;
+ // active time of the master that sent this unassign request
+ private final long initiatingMasterActiveTime;
+
public UnassignRegionHandler(HRegionServer server, String encodedName, long
closeProcId,
boolean abort, @Nullable ServerName destination, EventType eventType) {
- this(server, encodedName, closeProcId, abort, destination, eventType,
false);
+ this(server, encodedName, closeProcId, abort, destination, eventType, -1,
false);
Review Comment:
Where do we call this constructor? Is it safe to just use -1 here?
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterRpcServices.java:
##########
@@ -2567,6 +2583,18 @@ public ReportProcedureDoneResponse
reportProcedureDone(RpcController controller,
return ReportProcedureDoneResponse.getDefaultInstance();
}
+ private void throwOnOldMasterStartCode(long procId, long
initiatingMasterActiveTime)
Review Comment:
Just name it throwOnOldMaster? Now we use master active time, not master
start code.
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/handler/AssignRegionHandler.java:
##########
@@ -59,15 +59,20 @@ public class AssignRegionHandler extends EventHandler {
private final long masterSystemTime;
+ // active time of the master that sent this assign request
Review Comment:
Please also say "used for fencing, please see HBASE-28690 for more details".
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/RSProcedureDispatcher.java:
##########
@@ -471,9 +474,10 @@ public ServerOperation(RemoteProcedure remoteProcedure,
long procId, Class<?> rs
this.rsProcData = rsProcData;
}
- public RemoteProcedureRequest buildRequest() {
+ public RemoteProcedureRequest buildRequest(long
initiatingMasterActiveTime) {
Review Comment:
I prefer we add initiatingMasterActiveTime as a field of the operation,
instead of a method parameter.We can add a initiatingMasterActiveTime in
ServerOperation and RegionOperation class.
--
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]
