Apache9 commented on code in PR #5774:
URL: https://github.com/apache/hbase/pull/5774#discussion_r1537006507


##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/ServerManager.java:
##########
@@ -324,8 +324,19 @@ public void regionServerReport(ServerName sn, 
ServerMetrics sl) throws YouAreDea
       // the ServerName to use. Here we presume a master has already done
       // that so we'll press on with whatever it gave us for ServerName.
       if (!checkAndRecordNewServer(sn, sl)) {
-        LOG.info("RegionServerReport ignored, could not record the server: " + 
sn);
-        return; // Not recorded, so no need to move on
+        // Master already registered server with same (host + port) and higher 
startcode.

Review Comment:
   I checked the code, now I understand why we need to throw an exception here. 
Your comment totally missed the most important part...
   
   At least your comment should include these two points:
   
   1. The exception thrown here is not meant to tell the region server it is 
dead because if there is a new server on the same host port, the old server 
should have already been dead.
   2. The exception thrown here is to skip the later steps of the whole 
regionServerReport request processing. Usually, after recording it in 
ServerManager, we will call the related methods in AssignmentManager to record 
region states. If the region server is already dead, we should not do these 
steps any more, so here we throw an exception to let the upper layer know that 
they should not continue processing any more.



-- 
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]

Reply via email to