Skip to content

Commit 91bea7c

Browse files
authored
[server] Add per-task timeout for RebalanceManager to prevent indefinite blocking (#3429)
* [server] Add per-task timeout for RebalanceManager to prevent indefinite blocking * address hongshun's comments * address gujian's comments
1 parent 0a699b3 commit 91bea7c

6 files changed

Lines changed: 358 additions & 5 deletions

File tree

fluss-common/src/main/java/org/apache/fluss/cluster/rebalance/RebalanceStatus.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,10 +34,11 @@ public enum RebalanceStatus {
3434
REBALANCING(1),
3535
FAILED(2),
3636
COMPLETED(3),
37-
CANCELED(4);
37+
CANCELED(4),
38+
TIMEOUT(5);
3839

3940
public static final Set<RebalanceStatus> FINAL_STATUSES =
40-
new HashSet<>(Arrays.asList(COMPLETED, CANCELED, FAILED));
41+
new HashSet<>(Arrays.asList(COMPLETED, CANCELED, FAILED, TIMEOUT));
4142

4243
private final int code;
4344

fluss-server/src/main/java/org/apache/fluss/server/coordinator/CoordinatorEventProcessor.java

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,7 @@
8585
import org.apache.fluss.server.coordinator.event.NotifyLakeTableOffsetEvent;
8686
import org.apache.fluss.server.coordinator.event.NotifyLeaderAndIsrResponseReceivedEvent;
8787
import org.apache.fluss.server.coordinator.event.RebalanceEvent;
88+
import org.apache.fluss.server.coordinator.event.RebalanceTaskTimeoutEvent;
8889
import org.apache.fluss.server.coordinator.event.RemoveServerTagEvent;
8990
import org.apache.fluss.server.coordinator.event.SchemaChangeEvent;
9091
import org.apache.fluss.server.coordinator.event.TableRegistrationChangeEvent;
@@ -122,6 +123,7 @@
122123
import org.apache.fluss.server.zk.data.lake.LakeTableHelper;
123124
import org.apache.fluss.server.zk.data.lake.LakeTableSnapshot;
124125
import org.apache.fluss.utils.AutoPartitionStrategy;
126+
import org.apache.fluss.utils.clock.SystemClock;
125127
import org.apache.fluss.utils.types.Tuple2;
126128

127129
import org.slf4j.Logger;
@@ -249,7 +251,9 @@ public CoordinatorEventProcessor(
249251
this.lakeTableTieringManager = lakeTableTieringManager;
250252
this.coordinatorMetricGroup = coordinatorMetricGroup;
251253
this.internalListenerName = conf.getString(ConfigOptions.INTERNAL_LISTENER_NAME);
252-
this.rebalanceManager = new RebalanceManager(this, zooKeeperClient);
254+
this.rebalanceManager =
255+
new RebalanceManager(
256+
this, zooKeeperClient, coordinatorEventManager, SystemClock.getInstance());
253257
this.ioExecutor = ioExecutor;
254258
this.lakeTableHelper =
255259
new LakeTableHelper(zooKeeperClient, conf.getString(ConfigOptions.REMOTE_DATA_DIR));
@@ -302,6 +306,7 @@ public void startup() {
302306

303307
// start rebalance manager.
304308
rebalanceManager.startup();
309+
rebalanceManager.start();
305310
}
306311

307312
public void shutdown() {
@@ -645,6 +650,13 @@ public void process(CoordinatorEvent event) {
645650
completeFromCallable(
646651
cancelRebalanceEvent.getRespCallback(),
647652
() -> processCancelRebalance(cancelRebalanceEvent));
653+
} else if (event instanceof RebalanceTaskTimeoutEvent) {
654+
RebalanceTaskTimeoutEvent timeoutEvent = (RebalanceTaskTimeoutEvent) event;
655+
LOG.warn(
656+
"Rebalance task for {} timed out. Treating as timeout.",
657+
timeoutEvent.getTableBucket());
658+
rebalanceManager.finishRebalanceTask(
659+
timeoutEvent.getTableBucket(), RebalanceStatus.TIMEOUT);
648660
} else if (event instanceof ListRebalanceProgressEvent) {
649661
ListRebalanceProgressEvent listRebalanceProgressEvent =
650662
(ListRebalanceProgressEvent) event;
Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one or more
3+
* contributor license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright ownership.
5+
* The ASF licenses this file to You under the Apache License, Version 2.0
6+
* (the "License"); you may not use this file except in compliance with
7+
* the License. You may obtain a copy of the License at
8+
*
9+
* http://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*/
17+
18+
package org.apache.fluss.server.coordinator.event;
19+
20+
import org.apache.fluss.metadata.TableBucket;
21+
22+
/** An event fired when a rebalance task exceeds the timeout without completing. */
23+
public class RebalanceTaskTimeoutEvent implements CoordinatorEvent {
24+
25+
private final TableBucket tableBucket;
26+
27+
public RebalanceTaskTimeoutEvent(TableBucket tableBucket) {
28+
this.tableBucket = tableBucket;
29+
}
30+
31+
public TableBucket getTableBucket() {
32+
return tableBucket;
33+
}
34+
35+
@Override
36+
public String toString() {
37+
return "RebalanceTaskTimeoutEvent{tableBucket=" + tableBucket + "}";
38+
}
39+
}

fluss-server/src/main/java/org/apache/fluss/server/coordinator/rebalance/RebalanceManager.java

Lines changed: 114 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,8 @@
2727
import org.apache.fluss.metadata.TableBucket;
2828
import org.apache.fluss.server.coordinator.CoordinatorContext;
2929
import org.apache.fluss.server.coordinator.CoordinatorEventProcessor;
30+
import org.apache.fluss.server.coordinator.event.EventManager;
31+
import org.apache.fluss.server.coordinator.event.RebalanceTaskTimeoutEvent;
3032
import org.apache.fluss.server.coordinator.rebalance.goal.Goal;
3133
import org.apache.fluss.server.coordinator.rebalance.goal.GoalOptimizer;
3234
import org.apache.fluss.server.coordinator.rebalance.model.ClusterModel;
@@ -36,6 +38,9 @@
3638
import org.apache.fluss.server.zk.ZooKeeperClient;
3739
import org.apache.fluss.server.zk.data.LeaderAndIsr;
3840
import org.apache.fluss.server.zk.data.RebalanceTask;
41+
import org.apache.fluss.utils.clock.Clock;
42+
import org.apache.fluss.utils.clock.SystemClock;
43+
import org.apache.fluss.utils.concurrent.ExecutorThreadFactory;
3944

4045
import org.slf4j.Logger;
4146
import org.slf4j.LoggerFactory;
@@ -53,6 +58,9 @@
5358
import java.util.TreeSet;
5459
import java.util.UUID;
5560
import java.util.concurrent.ConcurrentHashMap;
61+
import java.util.concurrent.Executors;
62+
import java.util.concurrent.ScheduledExecutorService;
63+
import java.util.concurrent.TimeUnit;
5664

5765
import static org.apache.fluss.cluster.rebalance.RebalanceStatus.CANCELED;
5866
import static org.apache.fluss.cluster.rebalance.RebalanceStatus.COMPLETED;
@@ -70,8 +78,17 @@
7078
public class RebalanceManager {
7179
private static final Logger LOG = LoggerFactory.getLogger(RebalanceManager.class);
7280

81+
/** Hardcoded timeout for an in-flight rebalance task: 2 minutes. */
82+
private static final long REBALANCE_TASK_TIMEOUT_MS = 2 * 60 * 1000L;
83+
84+
/** Hardcoded interval for the periodic timeout check: 30 seconds. */
85+
private static final long TIMEOUT_CHECK_INTERVAL_MS = 30 * 1000L;
86+
7387
private final ZooKeeperClient zkClient;
7488
private final CoordinatorEventProcessor eventProcessor;
89+
private final EventManager eventManager;
90+
private final Clock clock;
91+
private final ScheduledExecutorService timeoutChecker;
7592

7693
/** A queue of in progress table bucket to rebalance. */
7794
private final Queue<TableBucket> inProgressRebalanceTasksQueue = new ArrayDeque<>();
@@ -90,9 +107,46 @@ public class RebalanceManager {
90107
private volatile @Nullable String currentRebalanceId;
91108
private volatile boolean isClosed = false;
92109

93-
public RebalanceManager(CoordinatorEventProcessor eventProcessor, ZooKeeperClient zkClient) {
110+
/**
111+
* Timestamp when the current in-flight task was started, or -1 if idle.
112+
*
113+
* <p>Write ordering contract (volatile publication idiom): always write {@code
114+
* inflightTaskStartMs} BEFORE {@code inflightTaskBucket} when setting, and clear {@code
115+
* inflightTaskBucket} BEFORE {@code inflightTaskStartMs} when resetting. The timeout checker
116+
* reads in reverse order (bucket first, then startMs), ensuring it never observes a stale
117+
* startMs paired with a new bucket.
118+
*/
119+
private volatile long inflightTaskStartMs = -1;
120+
121+
/** The bucket of the current in-flight task, or null if idle. Acts as the "gate" variable. */
122+
private volatile @Nullable TableBucket inflightTaskBucket;
123+
124+
public RebalanceManager(
125+
CoordinatorEventProcessor eventProcessor,
126+
ZooKeeperClient zkClient,
127+
EventManager eventManager,
128+
Clock clock) {
129+
this(
130+
eventProcessor,
131+
zkClient,
132+
eventManager,
133+
clock,
134+
Executors.newScheduledThreadPool(
135+
1, new ExecutorThreadFactory("rebalance-timeout")));
136+
}
137+
138+
@VisibleForTesting
139+
RebalanceManager(
140+
CoordinatorEventProcessor eventProcessor,
141+
ZooKeeperClient zkClient,
142+
EventManager eventManager,
143+
Clock clock,
144+
ScheduledExecutorService timeoutChecker) {
94145
this.eventProcessor = eventProcessor;
95146
this.zkClient = zkClient;
147+
this.eventManager = eventManager;
148+
this.clock = clock == null ? SystemClock.getInstance() : clock;
149+
this.timeoutChecker = timeoutChecker;
96150
this.goalOptimizer = new GoalOptimizer();
97151
}
98152

@@ -101,6 +155,19 @@ public void startup() {
101155
initialize();
102156
}
103157

158+
/** Starts the periodic timeout checker. Call after {@link #startup()}. */
159+
public void start() {
160+
timeoutChecker.scheduleWithFixedDelay(
161+
this::checkTimeoutSafely,
162+
TIMEOUT_CHECK_INTERVAL_MS,
163+
TIMEOUT_CHECK_INTERVAL_MS,
164+
TimeUnit.MILLISECONDS);
165+
LOG.info(
166+
"RebalanceManager timeout checker started: timeoutMs={}, checkIntervalMs={}",
167+
REBALANCE_TASK_TIMEOUT_MS,
168+
TIMEOUT_CHECK_INTERVAL_MS);
169+
}
170+
104171
public @Nullable String getRebalanceId() {
105172
return currentRebalanceId;
106173
}
@@ -132,6 +199,9 @@ public void registerRebalance(
132199
inProgressRebalanceTasks.clear();
133200
inProgressRebalanceTasksQueue.clear();
134201
finishedRebalanceTasks.clear();
202+
// Clear gate (bucket) first, then data (startMs).
203+
inflightTaskBucket = null;
204+
inflightTaskStartMs = -1;
135205

136206
currentRebalanceId = rebalanceId;
137207
if (rebalancePlan.isEmpty()) {
@@ -170,6 +240,9 @@ public void finishRebalanceTask(TableBucket tableBucket, RebalanceStatus statusF
170240
finishedRebalanceTasks.put(
171241
tableBucket,
172242
RebalanceResultForBucket.of(resultForBucket.plan(), statusForBucket));
243+
// Clear gate (bucket) first, then data (startMs).
244+
inflightTaskBucket = null;
245+
inflightTaskStartMs = -1;
173246
LOG.info(
174247
"Rebalance task {} in progress: {} tasks pending, {} completed.",
175248
currentRebalanceId,
@@ -250,6 +323,9 @@ public void cancelRebalance(@Nullable String rebalanceId) {
250323
rebalanceStatus = CANCELED;
251324
inProgressRebalanceTasksQueue.clear();
252325
inProgressRebalanceTasks.clear();
326+
// Clear gate (bucket) first, then data (startMs).
327+
inflightTaskBucket = null;
328+
inflightTaskStartMs = -1;
253329
// Here, it will not clear finishedRebalanceTasks, because it will be used by
254330
// listRebalanceProgress. It will be cleared when next register.
255331

@@ -302,6 +378,9 @@ public RebalanceTask generateRebalanceTask(List<Goal> goalsByPriority) {
302378
private void processNewRebalanceTask() {
303379
TableBucket tableBucket = inProgressRebalanceTasksQueue.peek();
304380
if (tableBucket != null && inProgressRebalanceTasks.containsKey(tableBucket)) {
381+
// Write data (startMs) first, then publish gate (bucket).
382+
inflightTaskStartMs = clock.milliseconds();
383+
inflightTaskBucket = tableBucket;
305384
RebalanceResultForBucket resultForBucket = inProgressRebalanceTasks.get(tableBucket);
306385
RebalanceResultForBucket rebalanceResultForBucket =
307386
RebalanceResultForBucket.of(resultForBucket.plan(), REBALANCING);
@@ -400,12 +479,46 @@ private ClusterModel initialClusterModel(Map<Integer, ServerModel> serverModelMa
400479
return new ClusterModel(servers);
401480
}
402481

482+
private void checkTimeoutSafely() {
483+
try {
484+
checkTimeout();
485+
} catch (Throwable t) {
486+
LOG.error("Unexpected error in RebalanceManager timeout check.", t);
487+
}
488+
}
489+
490+
@VisibleForTesting
491+
void checkTimeout() {
492+
// Read gate (bucket) first, then data (startMs).
493+
// If bucket is non-null, happens-before guarantees startMs is at least as
494+
// fresh as the value written before bucket was published.
495+
TableBucket bucket = inflightTaskBucket;
496+
long startMs = inflightTaskStartMs;
497+
if (bucket == null || startMs < 0) {
498+
return;
499+
}
500+
long elapsed = clock.milliseconds() - startMs;
501+
if (elapsed > REBALANCE_TASK_TIMEOUT_MS) {
502+
LOG.warn(
503+
"In-flight rebalance task for {} timed out after {}ms. "
504+
+ "Treating it as timed out and advancing to the next task.",
505+
bucket,
506+
elapsed);
507+
// Clear gate (bucket) first, then data (startMs), matching the
508+
// publication idiom so the next checkTimeout sees bucket==null.
509+
inflightTaskBucket = null;
510+
inflightTaskStartMs = -1;
511+
eventManager.put(new RebalanceTaskTimeoutEvent(bucket));
512+
}
513+
}
514+
403515
private void checkNotClosed() {
404516
checkArgument(!isClosed, "RebalanceManager is already closed.");
405517
}
406518

407519
public void close() {
408520
isClosed = true;
521+
timeoutChecker.shutdownNow();
409522
}
410523

411524
@VisibleForTesting

0 commit comments

Comments
 (0)