Umeshkumar9414 commented on code in PR #8339:
URL: https://github.com/apache/hbase/pull/8339#discussion_r3434927157
##########
hbase-server/src/test/java/org/apache/hadoop/hbase/master/assignment/TestRegionStates.java:
##########
@@ -208,4 +213,60 @@ public void testPerfSingleThread() {
LOG.info(String.format("PERF SingleThread: %s %s/sec",
StringUtils.humanTimeDiff(et - st),
StringUtils.humanSize(NRUNS / ((et - st) / 1000.0f))));
}
+
+ // ==========================================================================
+ // HBASE-28659: NPE in setServerState when the ServerStateNode is missing
+ // ==========================================================================
+
+ /**
+ * Regression test for HBASE-28659. After a Master restart, the serverMap
inside RegionStates is
+ * rebuilt only for the "live" servers (RegionServerTracker#upgrade ->
createServer) and for
+ * servers that re-register / are loaded from meta. A ServerCrashProcedure
that was persisted
+ * before the restart resumes for the *old* crashed ServerName, but no
ServerStateNode was ever
+ * recreated for it. When the resumed SCP reaches SERVER_CRASH_SPLIT_LOGS it
calls
+ * {@link RegionStates#logSplitting(ServerName)} -> setServerState ->
getServerNode, which returns
+ * null. Before the fix, {@code synchronized (serverNode)} threw NPE
(failing the SCP and
+ * triggering an unsupported rollback). After the fix, setServerState
recreates the missing node
+ * (these helpers only ever run for a crashed server inside an SCP), so the
split helpers proceed
+ * without throwing and the state is recorded.
+ */
+ @Test
+ public void testLogSplittingCreatesNodeWhenServerNodeMissing() {
+ final RegionStates stateMap = new RegionStates();
+ // The crashed server from the JIRA log: hregion1,16020,1715424228375
+ final ServerName crashedServer = ServerName.valueOf("hregion1", 16020,
1715424228375L);
+
+ // Simulate the post-restart state: the ServerStateNode for the crashed
server was never
+ // recreated (it was not in liveServersBeforeRestart and the RS came back
with a new startcode).
+ assertNull(stateMap.getServerNode(crashedServer),
+ "precondition: no ServerStateNode should exist for the crashed server
after restart");
+
+ // The resumed SCP advances through the split states. None of these should
throw now; the node
+ // is recreated on demand. Exercise every entry point into setServerState.
+ assertDoesNotThrow(() -> stateMap.metaLogSplitting(crashedServer));
+ assertDoesNotThrow(() -> stateMap.metaLogSplit(crashedServer));
+ assertDoesNotThrow(() -> stateMap.logSplitting(crashedServer));
+ assertDoesNotThrow(() -> stateMap.logSplit(crashedServer));
+
+ // The node was recreated and reflects the last split state set above.
+ ServerStateNode serverNode = stateMap.getServerNode(crashedServer);
+ assertNotNull(serverNode, "the ServerStateNode should be recreated by
setServerState");
+ assertTrue(serverNode.isInState(ServerState.OFFLINE));
Review Comment:
nit: It is hard but it would be great if in thist test we can also validate
that serverNode will get removed from the map after complition of SCP.
--
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]
