Skip to content

Commit 57d214b

Browse files
gengjun-gitclaude
andcommitted
[Enhancement] Stop leader daemons via interrupt on demotion; disconnect checkpoint HTTP on stop
Rework leader-demotion daemon shutdown from the no-interrupt + cooperative onStopRequested model to interrupt-based fast-cancel, and make in-flight checkpoint HTTP disconnectable so it does not block the drain. LeaderDaemon framework: - stopGracefully() interrupts the worker by default via a new interruptOnStop() hook (default true), so a cycle blocked in an interruptible primitive (queue poll, latch/future await, sleep, interruptible lock) unblocks at once. - Interrupt-unsafe daemons that call BDBJE/JE directly opt out with interruptOnStop()=false (CheckpointController, ClusterSnapshotJobScheduler) and stop cooperatively. Fix task-level interrupt mishandlers so shutdownNow() is safe: - ExportExportingTask: an interrupt during the sub-task await no longer cancels a healthy job as TIMEOUT; it leaves the job EXPORTING for the next leader and always clears doExportingThread (finally). - TabletScheduler.blockingAddTabletCtxToScheduler and RoutineLoadTaskScheduler no longer swallow InterruptedException (re-assert / propagate). - LeaderTaskExecutor / PriorityLeaderTaskExecutor close() uses shutdownNow() to cancel in-flight tasks fast. HTTP fast-fail: - CheckpointController holds its own in-flight HttpURLConnection and disconnects it in onStopRequested() to break out of an otherwise-uninterruptible socket read; it only ever disconnects its own connection (no collateral damage). - MetaHelper only publishes the connection handle (new Consumer<HttpURLConnection> overloads) and no longer owns any cancel logic; cancelInFlight() removed. Update LeaderDaemonTest (default-interrupt + opt-out paths) and CheckpointControllerTest (disconnect-on-stop). Signed-off-by: gengjun-git <gengjun@starrocks.com> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> (cherry picked from commit 42d82ca)
1 parent 35fdfb0 commit 57d214b

11 files changed

Lines changed: 279 additions & 106 deletions

File tree

fe/fe-core/src/main/java/com/starrocks/clone/TabletScheduler.java

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -346,7 +346,11 @@ public Pair<Boolean, Long> blockingAddTabletCtxToScheduler(TabletSchedCtx tablet
346346
result.first = (res == AddResult.ADDED);
347347
} catch (InterruptedException e) {
348348
// heldLock has already been re-acquired by sleepUnlocked().
349-
LOG.warn("Failed to execute blockingAddTabletCtxToScheduler", e);
349+
// Re-assert the interrupt so the caller (e.g. TabletChecker / ColocateTableBalancer
350+
// leader daemon being stopped on demotion) unwinds its cycle promptly instead of
351+
// silently swallowing the cancel.
352+
Thread.currentThread().interrupt();
353+
LOG.warn("Interrupted while executing blockingAddTabletCtxToScheduler", e);
350354
}
351355

352356
return result;

fe/fe-core/src/main/java/com/starrocks/common/util/LeaderDaemon.java

Lines changed: 35 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,9 @@ private void requestStop(boolean interruptWorker) {
136136

137137
/**
138138
* Coordinated stop for leader demotion:
139-
* 1. mark stopped and wake interval waits without interrupting the current cycle;
139+
* 1. mark stopped, wake interval waits, and (unless {@link #interruptOnStop()} is overridden
140+
* to {@code false}) interrupt the worker so it breaks out of any blocking primitive
141+
* (queue poll, latch/future await, sleep, interruptible lock) at once;
140142
* 2. join up to {@code timeoutMs};
141143
* 3. on timeout, invoke {@link #onJoinTimeout()} (default: terminate the JVM) and return
142144
* without clearing {@code worker}/{@code isRunning} - a subsequent {@link #start()}
@@ -145,7 +147,7 @@ private void requestStop(boolean interruptWorker) {
145147
* Idempotent.
146148
*/
147149
public final void stopGracefully(long timeoutMs) {
148-
requestStop(false);
150+
requestStop(interruptOnStop());
149151
Thread t = worker;
150152
if (t != null) {
151153
try {
@@ -231,9 +233,19 @@ protected void runOneCycle() throws InterruptedException {
231233

232234
/**
233235
* The body of each iteration. Runs only after FE is ready and the captured leader lease
234-
* is still valid. Subclasses must not block indefinitely; leader demotion waits for the
235-
* current cycle to finish and terminates the process on timeout instead of interrupting the
236-
* worker inside storage or journal code that may treat interruption as fatal.
236+
* is still valid. Subclasses must not block indefinitely.
237+
*
238+
* By default leader demotion INTERRUPTS the worker (see {@link #stopGracefully(long)}), so
239+
* subclasses should block only in interruptible primitives and must let an
240+
* {@link InterruptedException} propagate (or re-check {@link #isStopped()} and return) - they
241+
* MUST NOT map it to a business outcome (e.g. cancel a healthy job as "timeout"). If the cycle
242+
* cannot finish within {@code leader_demotion_drain_timeout_sec} the process is terminated via
243+
* {@link #onJoinTimeout()} as a last resort.
244+
*
245+
* A subclass that runs interrupt-unsafe work on its own thread - a direct BDBJE/JE call
246+
* (interrupting it can invalidate the environment) or an uninterruptible native/socket read -
247+
* must override {@link #interruptOnStop()} to return {@code false} and cooperatively bail out
248+
* by polling {@link #isStopped()} and/or waking its wait in {@link #onStopRequested()}.
237249
*/
238250
protected abstract void runAfterLeaseValid() throws InterruptedException;
239251

@@ -255,12 +267,28 @@ protected void onStopped() {
255267
}
256268

257269
/**
258-
* Hook called immediately after a stop request is accepted and before {@link #stopGracefully(long)}
259-
* waits for the worker thread. Subclasses should use this to wake their own cancellable waits.
270+
* Optional hook called immediately after a stop request is accepted and before
271+
* {@link #stopGracefully(long)} waits for the worker thread. Most daemons do not need it:
272+
* the default stop interrupts the worker, which already breaks any interruptible wait.
273+
* It matters only for daemons that override {@link #interruptOnStop()} to {@code false} and
274+
* therefore need to cooperatively wake their own uninterruptible wait (e.g. offer a sentinel
275+
* to a result queue, or disconnect an in-flight HTTP connection).
260276
*/
261277
protected void onStopRequested() {
262278
}
263279

280+
/**
281+
* Whether {@link #stopGracefully(long)} may interrupt the worker thread. Default {@code true}:
282+
* interrupt is the fast, standard way to cancel a blocked cycle. Override to return
283+
* {@code false} ONLY for daemons whose worker executes interrupt-unsafe work directly on its
284+
* own thread - a raw BDBJE/JE operation (an interrupt can invalidate the environment) or an
285+
* uninterruptible native/socket read - and instead bail out cooperatively via
286+
* {@link #isStopped()} polling and {@link #onStopRequested()}.
287+
*/
288+
protected boolean interruptOnStop() {
289+
return true;
290+
}
291+
264292
/**
265293
* Invoked when the worker thread fails to exit within the stop timeout. Default:
266294
* {@link System#exit(int)} - a stuck worker combined with a later {@link #start()} would run

fe/fe-core/src/main/java/com/starrocks/lake/snapshot/ClusterSnapshotJobScheduler.java

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -126,4 +126,16 @@ protected void runAfterLeaseValid() {
126126
CheckpointController.exclusiveUnlock();
127127
}
128128
}
129+
130+
/**
131+
* Interrupt-unsafe: the worker calls BDBJE/JE directly (getJournal().getMaxJournalId()) and
132+
* drives a full checkpoint (journal maintenance + image push) inline, where an interrupt can
133+
* invalidate the BDB environment. It stops cooperatively instead - the driven checkpoint polls
134+
* isStopped() between phases, backed by the stopGracefully() join-timeout -> onJoinTimeout()
135+
* process-exit backstop.
136+
*/
137+
@Override
138+
protected boolean interruptOnStop() {
139+
return false;
140+
}
129141
}

fe/fe-core/src/main/java/com/starrocks/leader/CheckpointController.java

Lines changed: 47 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,15 @@ public class CheckpointController extends LeaderDaemon {
107107
// save the cluster snapshot info getted from the checkpoint worker.
108108
private volatile ClusterSnapshotInfo clusterSnapshotInfo;
109109

110+
// The HTTP connection this controller's worker thread is currently blocked on (image
111+
// push / download / journal_id probe), or null between calls. Held so onStopRequested()
112+
// can disconnect OUR OWN connection on demotion to break out of an otherwise-uninterruptible
113+
// socket read (a push can block up to PUT_TIMEOUT_SECOND). Scoped to this controller, so it
114+
// never touches another caller's connection. Only one HTTP op runs at a time on the
115+
// single-threaded worker, so a single reference is sufficient.
116+
// Package-private so same-package tests can set it to verify the disconnect-on-stop path.
117+
volatile HttpURLConnection inFlightConnection;
118+
110119
public CheckpointController(String name, Journal journal, String subDir) {
111120
super(name, FeConstants.checkpoint_interval_second * 1000L);
112121
this.journal = journal;
@@ -139,22 +148,29 @@ protected void runAfterLeaseValid() {
139148
* pushImage, deleteOldJournals) are idempotent at the system level - worker FE
140149
* keeps writing the image regardless of this leader's lifecycle, and the next
141150
* leader will re-orchestrate pushImage / deleteOldJournals from scratch. So we
142-
* only have to wake the current checkpoint wait, release leader-session bookkeeping,
143-
* and ask MetaHelper to break out of any in-flight HTTP, then let the daemon thread exit.
151+
* only have to wake the current checkpoint wait, break out of any in-flight HTTP,
152+
* and let the daemon thread exit; onStopped() clears the leader-session bookkeeping.
144153
*
145-
* Today {@link MetaHelper#cancelInFlight()} is a no-op (see TODO there), so a
146-
* push/download stuck in HttpURLConnection.read will block this drain up to its
147-
* own connect/read timeout (up to 3600s per node for push). The bookkeeping
148-
* fields below are still cleared so the next leader does not inherit stale
149-
* "pending push" / "last-failed-worker" state.
154+
* Bare {@link HttpURLConnection} socket reads honor neither {@link Thread#interrupt()}
155+
* nor any cooperative flag, so we disconnect the connection this controller's own worker
156+
* is currently blocked on. That unblocks its read within the drain budget instead of
157+
* waiting out the per-node push/download timeout (up to 3600s). We only ever disconnect
158+
* OUR OWN connection, so no other caller's request is affected.
150159
*/
151160
@Override
152161
protected void onStopRequested() {
153162
BlockingQueue<CheckpointCompletionStatus> currentResult = result;
154163
if (currentResult != null) {
155164
currentResult.offer(new CheckpointCompletionStatus(false, "leader checkpoint controller is stopping", null));
156165
}
157-
MetaHelper.cancelInFlight();
166+
HttpURLConnection conn = inFlightConnection;
167+
if (conn != null) {
168+
try {
169+
conn.disconnect();
170+
} catch (Throwable t) {
171+
LOG.warn("failed to disconnect in-flight checkpoint connection on stop", t);
172+
}
173+
}
158174
}
159175

160176
@Override
@@ -169,6 +185,18 @@ protected void onStopped() {
169185
lastFailedTime.clear();
170186
}
171187

188+
/**
189+
* Interrupt-unsafe: the worker calls BDBJE/JE directly (getFinalizedJournalId /
190+
* deleteJournals -> removeDatabase) where an interrupt can invalidate the environment, and
191+
* blocks in uninterruptible HttpURLConnection reads. It stops cooperatively instead: the
192+
* isStopped() polling between phases + onStopRequested() (waking the result queue and
193+
* disconnecting the in-flight HTTP connection).
194+
*/
195+
@Override
196+
protected boolean interruptOnStop() {
197+
return false;
198+
}
199+
172200
protected void runCheckpointController() {
173201
// ignore return value in normal checkpoint controller
174202
runCheckpointControllerWithIds(getImageJournalId(), getCheckpointJournalId(), false);
@@ -326,7 +354,12 @@ private void downloadImage(ImageFormatVersion imageFormatVersion, String imageDi
326354
"/image?version=" + journalId
327355
+ "&subdir=" + subDir
328356
+ "&image_format_version=" + imageFormatVersion;
329-
MetaHelper.downloadImageFile(url, MetaService.DOWNLOAD_TIMEOUT_SECOND * 1000, String.valueOf(journalId), dir);
357+
try {
358+
MetaHelper.downloadImageFile(url, MetaService.DOWNLOAD_TIMEOUT_SECOND * 1000, String.valueOf(journalId), dir,
359+
conn -> inFlightConnection = conn);
360+
} finally {
361+
inFlightConnection = null;
362+
}
330363

331364
// clean the old images
332365
MetaCleaner cleaner = new MetaCleaner(imageDir);
@@ -472,7 +505,7 @@ private void pushImage(long imageVersion) {
472505
+ "&for_global_state=" + belongToGlobalStateMgr
473506
+ "&image_format_version=" + formatVersion.toString();
474507
try {
475-
MetaHelper.httpGet(url, PUT_TIMEOUT_SECOND * 1000);
508+
MetaHelper.httpGet(url, PUT_TIMEOUT_SECOND * 1000, conn -> inFlightConnection = conn);
476509

477510
LOG.info("push image successfully, url = {}", url);
478511
if (MetricRepo.hasInit) {
@@ -481,6 +514,8 @@ private void pushImage(long imageVersion) {
481514
} catch (IOException e) {
482515
pushSuccess = false;
483516
LOG.error("Exception when pushing image file. url = {}", url, e);
517+
} finally {
518+
inFlightConnection = null;
484519
}
485520
if (pushSuccess) {
486521
iterator.remove();
@@ -508,6 +543,7 @@ private long getMinReplayedJournalId() {
508543
*/
509544
idURL = new URL("http://" + NetUtils.getHostPortInAccessibleFormat(host, port) + "/journal_id?prefix=" + journal.getPrefix());
510545
conn = (HttpURLConnection) idURL.openConnection();
546+
inFlightConnection = conn;
511547
conn.setConnectTimeout(CONNECT_TIMEOUT_SECOND * 1000);
512548
conn.setReadTimeout(READ_TIMEOUT_SECOND * 1000);
513549
String idString = conn.getHeaderField("id");
@@ -521,6 +557,7 @@ private long getMinReplayedJournalId() {
521557
minReplayedJournalId = 0;
522558
break;
523559
} finally {
560+
inFlightConnection = null;
524561
if (conn != null) {
525562
conn.disconnect();
526563
}

fe/fe-core/src/main/java/com/starrocks/leader/MetaHelper.java

Lines changed: 30 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,7 @@
6262
import java.nio.file.Files;
6363
import java.nio.file.Path;
6464
import java.nio.file.Paths;
65+
import java.util.function.Consumer;
6566
import java.util.stream.Stream;
6667

6768
public class MetaHelper {
@@ -77,26 +78,6 @@ public static int getLimit() {
7778
return CHECKPOINT_LIMIT_BYTES;
7879
}
7980

80-
/**
81-
* Best-effort cancel for any blocking HTTP operation currently issued by MetaHelper on
82-
* behalf of leader-only daemons (e.g. CheckpointController push/download). Today this is
83-
* a no-op: {@link #httpGet} and {@link #downloadImageFile} use bare {@link HttpURLConnection},
84-
* which honors neither {@link Thread#interrupt()} nor any other external cancel signal for
85-
* socket reads. The only way a stuck call exits is its own connect/read timeout, which
86-
* may be up to an hour for push and {@code MetaService.DOWNLOAD_TIMEOUT_SECOND} for
87-
* download.
88-
*
89-
* TODO(cancel-infra): wire this up to actively disconnect the active {@link HttpURLConnection}
90-
* (or switch to a cancellable HTTP client) so leader-demotion {@code onStopped()} can
91-
* break out of these calls within the {@code leader_demotion_drain_timeout_sec} budget.
92-
* This is tracked alongside the planned {@code Locker.lockInterruptibly} cleanup, since
93-
* both block the same goal of making demotion drain deterministic for non-interruptible
94-
* blocking operations.
95-
*/
96-
public static void cancelInFlight() {
97-
// intentionally empty - see TODO(cancel-infra) above
98-
}
99-
10081
// rename the .PART_SUFFIX file to filename
10182
public static File complete(String filename, File dir) throws IOException {
10283
File file = new File(dir, filename + MetaHelper.PART_SUFFIX);
@@ -109,6 +90,18 @@ public static File complete(String filename, File dir) throws IOException {
10990

11091
public static void downloadImageFile(String urlStr, int timeout, String journalId, File destDir)
11192
throws IOException {
93+
downloadImageFile(urlStr, timeout, journalId, destDir, null);
94+
}
95+
96+
/**
97+
* Same as {@link #downloadImageFile(String, int, String, File)} but publishes the freshly
98+
* opened {@link HttpURLConnection} to {@code onConnect} so the *calling* leader daemon can
99+
* hold its own reference and {@link HttpURLConnection#disconnect() disconnect} it on demotion
100+
* to break out of a stuck socket read. MetaHelper itself performs no cancellation.
101+
*/
102+
public static void downloadImageFile(String urlStr, int timeout, String journalId, File destDir,
103+
Consumer<HttpURLConnection> onConnect)
104+
throws IOException {
112105
HttpURLConnection conn = null;
113106
String checksum = null;
114107
String destFilename = Storage.IMAGE + "." + journalId;
@@ -117,6 +110,9 @@ public static void downloadImageFile(String urlStr, int timeout, String journalI
117110
try (FileOutputStream out = new FileOutputStream(partFile)) {
118111
URL url = new URL(urlStr);
119112
conn = (HttpURLConnection) url.openConnection();
113+
if (onConnect != null) {
114+
onConnect.accept(conn);
115+
}
120116
conn.setConnectTimeout(timeout);
121117
conn.setReadTimeout(timeout);
122118

@@ -169,11 +165,25 @@ public static OutputStream getOutputStream(String filename, File dir)
169165
}
170166

171167
public static void httpGet(String urlStr, int timeout) throws IOException {
168+
httpGet(urlStr, timeout, null);
169+
}
170+
171+
/**
172+
* Same as {@link #httpGet(String, int)} but publishes the freshly opened
173+
* {@link HttpURLConnection} to {@code onConnect} so the *calling* leader daemon can hold its
174+
* own reference and {@link HttpURLConnection#disconnect() disconnect} it on demotion to break
175+
* out of a stuck socket read (e.g. a checkpoint push that can otherwise block up to an hour).
176+
* MetaHelper itself performs no cancellation.
177+
*/
178+
public static void httpGet(String urlStr, int timeout, Consumer<HttpURLConnection> onConnect) throws IOException {
172179
URL url = new URL(urlStr);
173180
HttpURLConnection conn = null;
174181

175182
try {
176183
conn = (HttpURLConnection) url.openConnection();
184+
if (onConnect != null) {
185+
onConnect.accept(conn);
186+
}
177187
conn.setConnectTimeout(timeout);
178188
conn.setReadTimeout(timeout);
179189

fe/fe-core/src/main/java/com/starrocks/load/routineload/RoutineLoadTaskScheduler.java

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -206,8 +206,12 @@ private void process() throws InterruptedException {
206206
}
207207

208208
submitToSchedule(routineLoadTaskInfo);
209+
} catch (InterruptedException e) {
210+
// Propagate so the LeaderDaemon loop breaks promptly when the scheduler is being
211+
// stopped (e.g. on leader demotion) instead of swallowing the cancel and re-polling.
212+
throw e;
209213
} catch (Exception e) {
210-
LOG.warn("Taking routine load task from queue has been interrupted", e);
214+
LOG.warn("Failed to take/schedule routine load task from queue", e);
211215
return;
212216
}
213217
}

0 commit comments

Comments
 (0)