Skip to content

Commit 701d98e

Browse files
gengjun-gitclaude
andcommitted
[Enhancement] Drop abandoned agent tasks from AgentTaskQueue on leader demotion
The demotion drain previously only failed the in-flight agent tasks' latches; the tasks themselves stayed in AgentTaskQueue. A demoting/follower leader no longer processes the BE reports that normally remove them, so they would leak (including latch-less tasks such as drop/clone) across a demote/re-elect cycle. Clear the queue as part of the drain, mirroring how the old System.exit model wiped it via process restart. Rename AgentTaskQueue.failAllPendingWaiters -> abandonInFlightTasks (and the GlobalStateMgr demotion stage failInFlightAgentTasks -> abandonInFlightAgentTasks) to reflect that it now both fails latch waiters and drops all tasks. Safe to clear the whole queue because addTask() rejects new tasks while demoting, so nothing live is discarded. AgentTaskTest asserts the queue is emptied. Signed-off-by: gengjun-git <gengjun@starrocks.com> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 579b63f commit 701d98e

4 files changed

Lines changed: 28 additions & 21 deletions

File tree

fe/fe-core/src/main/java/com/starrocks/server/GlobalStateMgr.java

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2149,7 +2149,7 @@ void executeLeaderDemotionStages(FrontendNodeType targetType) {
21492149
LOG.info("leader demotion to {} starting", targetType);
21502150
long startMs = System.currentTimeMillis();
21512151
runDemotionStage("beginLeaderDemotion", () -> beginLeaderDemotion(targetType));
2152-
runDemotionStage("failInFlightAgentTasks", this::failInFlightAgentTasks);
2152+
runDemotionStage("abandonInFlightAgentTasks", this::abandonInFlightAgentTasks);
21532153
runDemotionStage("sealJournalWriter", this::sealJournalWriter);
21542154
runDemotionStage("stopLeaderOnlyDaemonThreads", this::stopLeaderOnlyDaemonThreads);
21552155
runDemotionStage("flushLeaderSessionState", this::flushLeaderSessionState);
@@ -2175,11 +2175,12 @@ private void runDemotionStage(String stageName, Runnable stage) {
21752175
* Abandon every in-flight BE agent task the (demoting) leader issued: fail their completion
21762176
* latches so waiters (e.g. create-tablet in TabletTaskExecutor.waitForFinished, which can run on
21772177
* a user connection thread that demotion never interrupts) unblock immediately and release their
2178-
* db locks, instead of waiting out tablet_create_timeout for an operation whose journal write is
2179-
* already fenced. AgentTaskQueue.addTask() rejects new tasks once demoting, closing the TOCTOU.
2178+
* db locks instead of waiting out tablet_create_timeout for an operation whose journal write is
2179+
* already fenced, and drop the tasks from the queue so they do not leak across a demote/re-elect
2180+
* cycle. AgentTaskQueue.addTask() rejects new tasks once demoting, closing the TOCTOU.
21802181
*/
2181-
private void failInFlightAgentTasks() {
2182-
com.starrocks.task.AgentTaskQueue.failAllPendingWaiters(
2182+
private void abandonInFlightAgentTasks() {
2183+
com.starrocks.task.AgentTaskQueue.abandonInFlightTasks(
21832184
new com.starrocks.common.Status(TStatusCode.CANCELLED, "leader is demoting"));
21842185
}
21852186

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -172,7 +172,7 @@ public void setFailed(boolean isFailed) {
172172
* Release any waiter blocked on this task's completion latch, failing it with {@code status}.
173173
* Default no-op: most agent tasks have no waiter. Latch-holding subclasses (create-replica,
174174
* push, tablet-metadata-update, drop-auto-increment-map) override this so a leader demotion can
175-
* abandon all in-flight agent tasks at once (see {@link AgentTaskQueue#failAllPendingWaiters})
175+
* abandon all in-flight agent tasks at once (see {@link AgentTaskQueue#abandonInFlightTasks})
176176
* and unblock their waiters immediately, instead of each wait site polling for demotion.
177177
* Distinct from {@link #setFailed(boolean)} (per-task status flag consumed by ALTER jobs) and
178178
* from the per-mark countdown (per-replica success) - overriding those would break retry/quorum.

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

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ public static synchronized void addTaskList(List<AgentTask> taskList) {
7777

7878
public static synchronized boolean addTask(AgentTask task) {
7979
// Source guard for leader demotion: once demoting, refuse to enqueue new agent tasks.
80-
// Together with failAllPendingWaiters() (which drains what is already queued) this closes the
80+
// Together with abandonInFlightTasks() (which drains what is already queued) this closes the
8181
// TOCTOU where a create-tablet started just after the drain would otherwise wait out its
8282
// timeout. The throw fails the doomed operation fast (its journal write would be fenced anyway).
8383
if (GlobalStateMgr.getCurrentState().isLeaderDemoting()) {
@@ -272,15 +272,17 @@ public static synchronized void clearAllTasks() {
272272
}
273273

274274
/**
275-
* Leader-demotion drain: fail the completion latch of every in-flight agent task so any waiter
276-
* (e.g. TabletTaskExecutor.waitForFinished waiting on a create-tablet latch) unblocks at once
277-
* with {@code status} instead of waiting out its timeout. Snapshots under the queue lock, then
278-
* cancels outside it, so we do not hold the global queue monitor across N latch operations
279-
* (which would stall concurrent BE-response processing). Tasks without a latch are no-ops.
280-
* Pairs with the addTask() demoting guard, which prevents new tasks from being enqueued after
281-
* this runs.
275+
* Leader-demotion drain: abandon every in-flight agent task. First fail each task's completion
276+
* latch (if any) so waiters (e.g. TabletTaskExecutor.waitForFinished on a create-tablet latch)
277+
* unblock at once with {@code status} and release their locks instead of waiting out a timeout;
278+
* then drop all tasks from the queue so they do not leak across a demote/re-elect cycle - a
279+
* demoting/follower leader no longer processes the BE reports that would normally remove them.
280+
* Snapshots under the queue lock and cancels outside it, so the global queue monitor is not held
281+
* across N latch operations (which would stall concurrent BE-response processing). Clearing the
282+
* whole queue is safe because addTask() rejects new tasks while demoting, so nothing live is
283+
* discarded here.
282284
*/
283-
public static void failAllPendingWaiters(Status status) {
285+
public static void abandonInFlightTasks(Status status) {
284286
List<AgentTask> snapshot = Lists.newArrayList();
285287
synchronized (AgentTaskQueue.class) {
286288
for (Map<Long, AgentTask> signatureMap : tasks.values()) {
@@ -294,6 +296,10 @@ public static void failAllPendingWaiters(Status status) {
294296
LOG.warn("failed to cancel pending waiter for agent task {}", task, t);
295297
}
296298
}
299+
synchronized (AgentTaskQueue.class) {
300+
tasks.clear();
301+
taskNum = 0;
302+
}
297303
}
298304

299305
public static synchronized int getTaskNum() {

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

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -495,21 +495,21 @@ public void testCreateReplicaTaskBuilderPreservesRange() throws AnalysisExceptio
495495
}
496496

497497
@Test
498-
public void testFailAllPendingWaitersReleasesLatch() {
498+
public void testAbandonInFlightTasksReleasesLatchAndClearsQueue() {
499499
AgentTaskQueue.clearAllTasks();
500500
MarkedCountDownLatch<Long, Long> l = new MarkedCountDownLatch<>(1);
501501
l.addMark(backendId1, tabletId1);
502502
((CreateReplicaTask) createReplicaTask).setLatch(l);
503503
AgentTaskQueue.addTask(createReplicaTask);
504504

505-
// Leader-demotion drain: fail every in-flight agent task latch so a waiter
506-
// (e.g. TabletTaskExecutor.waitForFinished) unblocks at once with the demotion reason
507-
// instead of waiting out its timeout.
508-
AgentTaskQueue.failAllPendingWaiters(new Status(TStatusCode.CANCELLED, "leader is demoting"));
505+
// Leader-demotion drain: fail every in-flight agent task latch (so a waiter such as
506+
// TabletTaskExecutor.waitForFinished unblocks with the demotion reason instead of waiting out
507+
// its timeout) AND drop the tasks from the queue so they do not leak across a demote/re-elect.
508+
AgentTaskQueue.abandonInFlightTasks(new Status(TStatusCode.CANCELLED, "leader is demoting"));
509509

510510
Assertions.assertEquals(0, l.getCount());
511511
Assertions.assertFalse(l.getStatus().ok());
512-
AgentTaskQueue.clearAllTasks();
512+
Assertions.assertEquals(0, AgentTaskQueue.getTaskNum());
513513
}
514514

515515
@Test

0 commit comments

Comments
 (0)