HBASE-28659 Fix NPE in hmaster RegionStates.setServerState when the ServerStateNode is missing

[SwaraliJoshi]

Problem statement

A ServerCrashProcedure (SCP) can crash the PEWorker with a NullPointerException while processing a dead RegionServer, after a Master restart:

ERROR [PEWorker-15] procedure2.ProcedureExecutor: CODE-BUG: Uncaught runtime exception:
  pid=48, state=RUNNABLE:SERVER_CRASH_SPLIT_LOGS, hasLock=true;
  ServerCrashProcedure hregion1,16020,1715424228375, splitWal=true, meta=true
java.lang.NullPointerException: null
  at o.a.h.h.master.assignment.RegionStates.setServerState(RegionStates.java:409)
  at o.a.h.h.master.assignment.RegionStates.logSplitting(RegionStates.java:435)
  at o.a.h.h.master.procedure.ServerCrashProcedure.executeFromState(ServerCrashProcedure.java:226)

setServerState dereferences the result of getServerNode(serverName) inside synchronized (serverNode), but getServerNode is documented to return null when no node exists:

private void setServerState(ServerName serverName, ServerState state) {
  ServerStateNode serverNode = getServerNode(serverName);
  synchronized (serverNode) {   // NPE when serverNode == null
    serverNode.setState(state);
  }
}

This is worse than a single failed procedure: the SCP does not support rollback, so the uncaught exception triggers an unsupported rollback (... does not support rollback but the execution failed and try to rollback, code bug?). The crashed server's WAL split and region reassignment never complete — and since this SCP carried meta, meta recovery is abandoned, which can wedge the cluster until manual intervention.

Root cause

ServerStateNodes live only in the in-memory RegionStates.serverMap; they are never persisted. On Master startup the map is rebuilt only from:

• RegionServerTracker.upgrade() -> createServer() for the live set: rsListStorage.getAll() ∪ MasterWalManager.getLiveServersFromWALDir() (WAL dirs that do not end in -splitting),
• RegionServers re-registering (ServerManager.recordNewServerWithLock),
• region locations loaded from hbase:meta (AssignmentManager.loadMeta -> createServer(regionLocation) for any region whose persisted location is that server).

Servers that only have an outstanding SCP are added to the deadservers list by findDeadServersAndProcess, but no ServerStateNode is created for them. So if a persisted SCP resumes for a server that:

1. has its WAL dir already renamed to -splitting (counted as "splitting", explicitly excluded from "live"),
2. has come back with a new start code (so re-registration creates a node for a different ServerName), and
3. is no longer referenced by any region in meta (so loadMeta does not recreate the node),

…then getServerNode(oldServerName) returns null, and the next setServerState call (logSplitting at SERVER_CRASH_SPLIT_LOGS) NPEs.

This is deterministic given that on-disk/in-memory state — it is not a timing race. It is "rare" only because reaching that state requires a Master restart to land inside an SCP's WAL-splitting window. A 2.5.8 -> 3.0 upgrade (rolling Master failovers while RegionServers bounce) widens that window dramatically, which is why the reporter only saw it during migration.

Timeline (from the attached hbase--master log)

Time (UTC)	Event
10:43:51	hregion1,16020,1715424228375 registers -> createServer() -> ServerStateNode created
10:45:02	hregion1 znode deleted -> expiration -> SCP pid=48 scheduled (carrying meta); runs through SERVER_CRASH_SPLIT_META_LOGS while the node still exists
10:45:05	hregion2 (now hosting meta) aborts: Failed to open region hbase:meta ... can not recover -> SCP pid=51 scheduled
10:45:08	Terminating master — the Master process itself goes down (external/harness event, unrelated to the SCP)
10:45:11	STARTING service HMaster — fresh Master boots; in-memory serverMap is empty
10:45:42	Upgrading RegionServerTracker ... 2 existing ServerCrashProcedures, 0 possibly 'live' servers, and 2 'splitting' — old hregion1 is in the splitting set, not live, so no node is recreated
10:45:49	hregion1 re-registers with a new start code ...343997 -> node created for the new name only
10:45:57	SCP pid=48 resumes for old ...228375 -> logSplitting -> getServerNode returns null -> NPE -> SCP fails -> unsupported rollback

The Master restart and the SCP are independently triggered events (a RegionServer crash created the SCP; the Master process was terminated separately). The bug is the coincidence of the two.

Solution

Recreate the ServerStateNode in setServerState when it is missing, instead of NPEing:

private void setServerState(ServerName serverName, ServerState state) {
  ServerStateNode serverNode =
    serverMap.computeIfAbsent(serverName, key -> new ServerStateNode(key));
  synchronized (serverNode) {
    serverNode.setState(state);
  }
}

This single change covers all four split helpers that route through setServerState: metaLogSplitting, metaLogSplit, logSplitting, and logSplit.

These split helpers are only ever called from ServerCrashProcedure for a crashed server, and the SCP removes the node again via removeServer at SERVER_CRASH_FINISH. So creating the node on demand is the correct, self-cleaning behavior: the resumed SCP records its split state as usual, and the node is torn down when the SCP completes. This also follows the reviewer's suggestion — a missing node here is not an error condition, so a warning would only confuse operators; instead we just (re)create it, consistent with how createServer already builds nodes with the same computeIfAbsent.

Impact

• Eliminates the NullPointerException in setServerState during SCP replay after a Master restart/failover.
• Prevents the more damaging downstream effect: an SCP failure -> unsupported rollback -> abandoned WAL split / meta recovery -> stuck regions requiring manual recovery.
• No behavior change on the normal path (when the ServerStateNode exists). When it is absent, the node is recreated and cleaned up by the SCP at SERVER_CRASH_FINISH.
• Low-risk and easily backportable (single localized change), which matters because this bug surfaces during version upgrades.

Testing

• Updated the regression test in TestRegionStates:
  • testLogSplittingCreatesNodeWhenServerNodeMissing — reproduces the post-restart scenario (a server with no ServerStateNode) and asserts all four split helpers (metaLogSplitting, metaLogSplit, logSplitting, logSplit) run without throwing, recreate the node, and record the final split state. Before the fix this threw NPE.
  • testLogSplittingOkWhenServerNodePresent — contrast case confirming normal behavior when the node already exists.
• Verified the SCP integration suites pass: TestSCP, TestSCPWithMeta (meta-carrying scenario from the JIRA), TestRollbackSCP.

[Apache9]

So this could only happen where is actually no regions on the region server? Otherwise when loading regions in AssignmentManger, we will create the server node?

And I think we should create the server node here since this is not something incorrect, a warning message here will confusing the users...

[SwaraliJoshi]

So this could only happen where is actually no regions on the region server? Otherwise when loading regions in AssignmentManger, we will create the server node?

And I think we should create the server node here since this is not something incorrect, a warning message here will confusing the users...

@Apache9 I have made the changes. Please take a look at it.

[Umeshkumar9414]

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.

[Umeshkumar9414]

left a nit, otherwise fix looks good to me.

[SwaraliJoshi]

@Apache9 Requesting you to kindly take a look at this

[SwaraliJoshi]

@Apache9 Please review this fix.

[virajjasani]

+1

[virajjasani]

@SwaraliJoshi do we have backport PRs ready for branch-2 and branch-2.6? I can merge them all together.

[SwaraliJoshi]

@virajjasani No sir, I have not created the backport PRs yet. I thought once this is merged I can create those as well.
I will create them now.

[Apache9]

I think we have a method for creating ServerStateNode? Let's only create it when necessary.
Always creating it here may hide other bugs...

[SwaraliJoshi]

We will only create the serverNode when it is absent. I have updated the call with method createServer which does the exact same computeIfAbsent call as earlier (since this is cleaner and more readable).
Without this, it will throw NPE so we need to check if it exists and create if it doesn't.

@Apache9 Please take a look now.

[Apache9]

What I mean is that, we should change the code in SCP? If there is no server state node, we should create it before calling setServerState, but we'd better not creating server state node automatically inside setServerState.