Skip to content

Commit 579b63f

Browse files
gengjun-gitclaude
andcommitted
[Enhancement] Cancel in-flight agent tasks at the source on leader demotion
Replace the per-wait-site isLeaderDemoting() poll in TabletTaskExecutor with a single source-level drain: on leader demotion, fail the completion latch of every in-flight agent task in AgentTaskQueue so all latch-based waiters (create-tablet in waitForFinished, and the other agent-task waiters) unblock at once and release their db locks, instead of each wait site polling and every new waiter needing its own check. - AgentTask.cancelPendingWaiter(Status): default no-op; overridden by the latch-holding subclasses (CreateReplicaTask, PushTask, TabletMetadataUpdateAgentTask, DropAutoIncrementMapTask) to countDownToZero their latch. Kept distinct from setFailed(boolean) (per-task status flag consumed by ALTER-job polling) and per-mark countdown (per-replica success) so retry/quorum are not broken. - AgentTaskQueue.failAllPendingWaiters(Status): snapshot under the queue lock, then cancel outside it so the global queue monitor is not held across N latch operations. - AgentTaskQueue.addTask(): reject new tasks once demoting, closing the TOCTOU where a create-tablet started just after the drain would otherwise wait out its timeout. - GlobalStateMgr: new demotion stage failInFlightAgentTasks right after beginLeaderDemotion, so waiters release their locks before the leader-only daemons are stopped. - TabletTaskExecutor.waitForFinished: drop the now-redundant per-iteration demotion poll. Add AgentTaskTest coverage for the drain and the addTask demoting guard. Signed-off-by: gengjun-git <gengjun@starrocks.com> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 57d214b commit 579b63f

9 files changed

Lines changed: 125 additions & 8 deletions

File tree

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

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2149,6 +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);
21522153
runDemotionStage("sealJournalWriter", this::sealJournalWriter);
21532154
runDemotionStage("stopLeaderOnlyDaemonThreads", this::stopLeaderOnlyDaemonThreads);
21542155
runDemotionStage("flushLeaderSessionState", this::flushLeaderSessionState);
@@ -2170,6 +2171,18 @@ private void runDemotionStage(String stageName, Runnable stage) {
21702171
LOG.info("leader demotion stage '{}' completed in {}ms", stageName, System.currentTimeMillis() - startMs);
21712172
}
21722173

2174+
/**
2175+
* Abandon every in-flight BE agent task the (demoting) leader issued: fail their completion
2176+
* latches so waiters (e.g. create-tablet in TabletTaskExecutor.waitForFinished, which can run on
2177+
* 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.
2180+
*/
2181+
private void failInFlightAgentTasks() {
2182+
com.starrocks.task.AgentTaskQueue.failAllPendingWaiters(
2183+
new com.starrocks.common.Status(TStatusCode.CANCELLED, "leader is demoting"));
2184+
}
2185+
21732186
@VisibleForTesting
21742187
void sealJournalWriter() {
21752188
if (journalWriter == null) {

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

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
package com.starrocks.task;
3636

3737
import com.starrocks.common.Config;
38+
import com.starrocks.common.Status;
3839
import com.starrocks.thrift.TResourceInfo;
3940
import com.starrocks.thrift.TTaskType;
4041

@@ -167,6 +168,19 @@ public void setFailed(boolean isFailed) {
167168
this.isFailed = isFailed;
168169
}
169170

171+
/**
172+
* Release any waiter blocked on this task's completion latch, failing it with {@code status}.
173+
* Default no-op: most agent tasks have no waiter. Latch-holding subclasses (create-replica,
174+
* 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})
176+
* and unblock their waiters immediately, instead of each wait site polling for demotion.
177+
* Distinct from {@link #setFailed(boolean)} (per-task status flag consumed by ALTER jobs) and
178+
* from the per-mark countdown (per-replica success) - overriding those would break retry/quorum.
179+
*/
180+
public void cancelPendingWaiter(Status status) {
181+
// no-op by default
182+
}
183+
170184
public boolean shouldResend(long currentTimeMillis) {
171185
return createTime == -1 || currentTimeMillis - createTime > Config.agent_task_resend_wait_time_ms;
172186
}

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

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,9 @@
4040
import com.google.common.collect.Maps;
4141
import com.google.common.collect.Multimap;
4242
import com.google.common.collect.Table;
43+
import com.starrocks.common.Status;
4344
import com.starrocks.memory.estimate.Estimator;
45+
import com.starrocks.server.GlobalStateMgr;
4446
import com.starrocks.thrift.TPushType;
4547
import com.starrocks.thrift.TTaskType;
4648
import org.apache.logging.log4j.LogManager;
@@ -74,6 +76,14 @@ public static synchronized void addTaskList(List<AgentTask> taskList) {
7476
}
7577

7678
public static synchronized boolean addTask(AgentTask task) {
79+
// 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
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()) {
84+
throw new IllegalStateException(
85+
"leader is demoting, refuse to enqueue agent task: " + task);
86+
}
7787
long backendId = task.getBackendId();
7888
TTaskType type = task.getTaskType();
7989

@@ -261,6 +271,31 @@ public static synchronized void clearAllTasks() {
261271
taskNum = 0;
262272
}
263273

274+
/**
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.
282+
*/
283+
public static void failAllPendingWaiters(Status status) {
284+
List<AgentTask> snapshot = Lists.newArrayList();
285+
synchronized (AgentTaskQueue.class) {
286+
for (Map<Long, AgentTask> signatureMap : tasks.values()) {
287+
snapshot.addAll(signatureMap.values());
288+
}
289+
}
290+
for (AgentTask task : snapshot) {
291+
try {
292+
task.cancelPendingWaiter(status);
293+
} catch (Throwable t) {
294+
LOG.warn("failed to cancel pending waiter for agent task {}", task, t);
295+
}
296+
}
297+
}
298+
264299
public static synchronized int getTaskNum() {
265300
return taskNum;
266301
}

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -164,6 +164,13 @@ public void countDownToZero(String errMsg) {
164164
}
165165
}
166166

167+
@Override
168+
public void cancelPendingWaiter(Status status) {
169+
if (this.latch != null) {
170+
latch.countDownToZero(status);
171+
}
172+
}
173+
167174
public void setLatch(MarkedCountDownLatch<Long, Long> latch) {
168175
this.latch = latch;
169176
}

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,13 @@ public void countDownToZero(String errMsg) {
5959
}
6060
}
6161

62+
@Override
63+
public void cancelPendingWaiter(Status status) {
64+
if (this.latch != null) {
65+
latch.countDownToZero(status);
66+
}
67+
}
68+
6269
public void setLatch(MarkedCountDownLatch<Long, Long> latch) {
6370
this.latch = latch;
6471
}

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -229,6 +229,13 @@ public void countDownLatch(long backendId, long tabletId) {
229229
}
230230
}
231231

232+
@Override
233+
public void cancelPendingWaiter(Status status) {
234+
if (this.latch != null) {
235+
latch.countDownToZero(status);
236+
}
237+
}
238+
232239
public void countDownLatch(long backendId, long tabletId, String errMsg) {
233240
if (this.latch != null) {
234241
if (latch.markedCountDown(backendId, tabletId, new Status(TStatusCode.INTERNAL_ERROR, errMsg))) {

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,13 @@ public void countDownToZero(String errMsg) {
100100
}
101101
}
102102

103+
@Override
104+
public void cancelPendingWaiter(Status status) {
105+
if (this.latch != null) {
106+
latch.countDownToZero(status);
107+
}
108+
}
109+
103110
public abstract Set<Long> getTablets();
104111

105112
public abstract List<TTabletMetaInfo> getTTabletMetaInfoList();

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

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -543,14 +543,6 @@ static void waitForFinished(MarkedCountDownLatch<Long, Long> countDownLatch, lon
543543
}
544544
}
545545

546-
GlobalStateMgr globalStateMgr = GlobalStateMgr.getCurrentState();
547-
if (globalStateMgr.isLeaderDemoting() || !globalStateMgr.isLeaderWorkAdmissionOpen()) {
548-
String errMsg = "Create tablet cancelled because leader work admission is closed";
549-
LOG.warn(errMsg);
550-
countDownLatch.countDownToZero(new Status(TStatusCode.CANCELLED, errMsg));
551-
throw new DdlException(errMsg);
552-
}
553-
554546
timeLeft -= waitInterval;
555547
}
556548
// timed out

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

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@
4444
import com.starrocks.common.AnalysisException;
4545
import com.starrocks.common.Pair;
4646
import com.starrocks.common.Range;
47+
import com.starrocks.common.Status;
4748
import com.starrocks.common.jmockit.Deencapsulation;
4849
import com.starrocks.common.util.concurrent.MarkedCountDownLatch;
4950
import com.starrocks.server.GlobalStateMgr;
@@ -58,6 +59,7 @@
5859
import com.starrocks.thrift.TBackend;
5960
import com.starrocks.thrift.TCompressionType;
6061
import com.starrocks.thrift.TCreateTabletReq;
62+
import com.starrocks.thrift.TStatusCode;
6163
import com.starrocks.thrift.TStorageMedium;
6264
import com.starrocks.thrift.TStorageType;
6365
import com.starrocks.thrift.TTabletRange;
@@ -491,4 +493,37 @@ public void testCreateReplicaTaskBuilderPreservesRange() throws AnalysisExceptio
491493
TCreateTabletReq req = task.toThrift();
492494
Assertions.assertTrue(req.isSetRange());
493495
}
496+
497+
@Test
498+
public void testFailAllPendingWaitersReleasesLatch() {
499+
AgentTaskQueue.clearAllTasks();
500+
MarkedCountDownLatch<Long, Long> l = new MarkedCountDownLatch<>(1);
501+
l.addMark(backendId1, tabletId1);
502+
((CreateReplicaTask) createReplicaTask).setLatch(l);
503+
AgentTaskQueue.addTask(createReplicaTask);
504+
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"));
509+
510+
Assertions.assertEquals(0, l.getCount());
511+
Assertions.assertFalse(l.getStatus().ok());
512+
AgentTaskQueue.clearAllTasks();
513+
}
514+
515+
@Test
516+
public void testAddTaskRejectedWhenLeaderDemoting() {
517+
AgentTaskQueue.clearAllTasks();
518+
new MockUp<GlobalStateMgr>() {
519+
@Mock
520+
public boolean isLeaderDemoting() {
521+
return true;
522+
}
523+
};
524+
// Source guard: once demoting, no new agent task may be enqueued, so a create-tablet
525+
// started after the drain fails fast instead of hanging on a never-marked latch.
526+
Assertions.assertThrows(IllegalStateException.class, () -> AgentTaskQueue.addTask(createReplicaTask));
527+
Assertions.assertEquals(0, AgentTaskQueue.getTaskNum());
528+
}
494529
}

0 commit comments

Comments
 (0)