Skip to content

Commit ba062ef

Browse files
gengjun-gitclaude
andcommitted
[Enhancement] Reject agent-task enqueue on a demoted (non-leader) node
The addTask() demotion guard only checked isLeaderDemoting(), leaving a window AFTER the demotion completed: a straggling leader-session thread (e.g. a user DDL that passed its admission checks before the demotion began) could still enqueue on the now-follower node. The waiter would then sit out its full timeout, and the stale queue entry would survive into a later re-election where it shadows a same-signature task (addTask de-dups by signature, so BE finish reports resolve to the stale object and the new waiter never completes). Extend the guard to also reject when feType is FOLLOWER / OBSERVER / UNKNOWN (the demotion target roles). Verified every legitimate enqueue happens with feType == LEADER: activation flips feType to LEADER before the leader-only daemons start, DDL threads pass leader admission first, and the replay paths never enqueue (e.g. OlapTable.onDrop guards sendDropAutoIncrementMapTask with !replay). INIT stays admitted so bootstrap, checkpoint-image GlobalStateMgr instances, and plain unit tests are unaffected; a fully mocked GlobalStateMgr (null feType) is also admitted. Add AgentTaskTest.testAddTaskRejectedAfterDemotionCompleted. Signed-off-by: gengjun-git <gengjun@starrocks.com> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 1697260 commit ba062ef

2 files changed

Lines changed: 36 additions & 6 deletions

File tree

fe/fe-core/src/main/java/com/starrocks/task/AgentTaskQueue.java

Lines changed: 19 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@
4141
import com.google.common.collect.Multimap;
4242
import com.google.common.collect.Table;
4343
import com.starrocks.common.Status;
44+
import com.starrocks.ha.FrontendNodeType;
4445
import com.starrocks.memory.estimate.Estimator;
4546
import com.starrocks.server.GlobalStateMgr;
4647
import com.starrocks.thrift.TPushType;
@@ -76,13 +77,25 @@ public static synchronized void addTaskList(List<AgentTask> taskList) {
7677
}
7778

7879
public static synchronized boolean addTask(AgentTask task) {
79-
// Source guard for leader demotion: once demoting, refuse to enqueue new agent tasks.
80-
// Together with abandonInFlightTasks() (which drains what is already queued) this closes the
81-
// TOCTOU where a create-tablet started just after the drain would otherwise wait out its
82-
// timeout. The throw fails the doomed operation fast (its journal write would be fenced anyway).
83-
if (GlobalStateMgr.getCurrentState().isLeaderDemoting()) {
80+
// Source guard for leader demotion: refuse to enqueue new agent tasks once this node is
81+
// demoting OR has already finished demoting to a non-leader role. Together with
82+
// abandonInFlightTasks() (which drains what is already queued) this closes both windows
83+
// where a straggling leader-session thread (e.g. a user DDL that passed its admission
84+
// checks before the demotion began) would otherwise enqueue after the drain and wait out
85+
// its full timeout, leaving a stale entry in a non-leader's queue that could shadow a
86+
// same-signature task after re-election. The throw fails the doomed operation fast (its
87+
// journal write would be fenced anyway). Every legitimate enqueue happens with
88+
// feType == LEADER (activation flips feType to LEADER before leader-only daemons start,
89+
// and the replay paths never enqueue); INIT stays admitted so bootstrap, checkpoint-image
90+
// GlobalStateMgr instances, and plain unit tests are unaffected.
91+
GlobalStateMgr globalStateMgr = GlobalStateMgr.getCurrentState();
92+
FrontendNodeType feType = globalStateMgr.getFeType();
93+
if (globalStateMgr.isLeaderDemoting()
94+
|| feType == FrontendNodeType.FOLLOWER
95+
|| feType == FrontendNodeType.OBSERVER
96+
|| feType == FrontendNodeType.UNKNOWN) {
8497
throw new IllegalStateException(
85-
"leader is demoting, refuse to enqueue agent task: " + task);
98+
"node is demoting or not the leader (" + feType + "), refuse to enqueue agent task: " + task);
8699
}
87100
long backendId = task.getBackendId();
88101
TTaskType type = task.getTaskType();

fe/fe-core/src/test/java/com/starrocks/task/AgentTaskTest.java

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -529,6 +529,23 @@ public boolean isLeaderDemoting() {
529529
Assertions.assertEquals(0, AgentTaskQueue.getTaskNum());
530530
}
531531

532+
@Test
533+
public void testAddTaskRejectedAfterDemotionCompleted() {
534+
AgentTaskQueue.clearAllTasks();
535+
new MockUp<GlobalStateMgr>() {
536+
@Mock
537+
public com.starrocks.ha.FrontendNodeType getFeType() {
538+
return com.starrocks.ha.FrontendNodeType.FOLLOWER;
539+
}
540+
};
541+
// A straggling leader-session thread (e.g. a DDL that passed admission before the
542+
// demotion) must not enqueue AFTER the demotion completed either - otherwise it waits
543+
// out its full timeout and leaves a stale entry in the follower's queue that could
544+
// shadow a same-signature task after re-election.
545+
Assertions.assertThrows(IllegalStateException.class, () -> AgentTaskQueue.addTask(createReplicaTask));
546+
Assertions.assertEquals(0, AgentTaskQueue.getTaskNum());
547+
}
548+
532549
@Test
533550
public void testCancelPendingWaiterReleasesLatchOfEveryLatchHolder() {
534551
Status demoting = new Status(TStatusCode.CANCELLED, "leader is demoting");

0 commit comments

Comments
 (0)