Skip to content

Commit 3853d38

Browse files
authored
Fix driver scheduler ready queue reservation leak (#17919)
1 parent 1a3548e commit 3853d38

6 files changed

Lines changed: 121 additions & 7 deletions

File tree

‎iotdb-core/calc-commons/src/main/java/org/apache/iotdb/calc/execution/schedule/queue/IndexedBlockingReserveQueue.java‎

Lines changed: 33 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ public synchronized E poll() throws InterruptedException {
4949
E output = pollFirst();
5050
size--;
5151
reservedSize++;
52+
markReserved(output);
5253
return output;
5354
}
5455

@@ -68,15 +69,44 @@ public synchronized void repush(E element) {
6869
throw new NullPointerException(CalcMessages.PUSHED_ELEMENT_IS_NULL);
6970
}
7071
pushToQueue(element);
71-
reservedSize--;
72+
decreaseReservedSizeIfNecessary(element);
7273
size++;
7374
this.notifyAll();
7475
}
7576

7677
/**
7778
* For task that is not in readyQueue when it's cleared, it won't be added into the queue again.
7879
*/
79-
public synchronized void decreaseReservedSize() {
80-
this.reservedSize--;
80+
public synchronized boolean decreaseReservedSize(E element) {
81+
if (element == null) {
82+
throw new NullPointerException(CalcMessages.PUSHED_ELEMENT_IS_NULL);
83+
}
84+
return decreaseReservedSizeIfNecessary(element);
85+
}
86+
87+
public final synchronized int getReservedSize() {
88+
return reservedSize;
89+
}
90+
91+
@Override
92+
public synchronized void clear() {
93+
super.clear();
94+
this.reservedSize = 0;
95+
}
96+
97+
protected void markReserved(E element) {
98+
// Do nothing by default.
99+
}
100+
101+
protected boolean releaseReserved(E element) {
102+
return true;
103+
}
104+
105+
private boolean decreaseReservedSizeIfNecessary(E element) {
106+
if (!releaseReserved(element) || reservedSize <= 0) {
107+
return false;
108+
}
109+
reservedSize--;
110+
return true;
81111
}
82112
}

‎iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/execution/schedule/DriverScheduler.java‎

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -347,16 +347,18 @@ private void clearDriverTask(DriverTask task) {
347347
return;
348348
case READY:
349349
task.setStatus(DriverTaskStatus.ABORTED);
350-
readyQueue.remove(task.getDriverTaskId());
350+
if (readyQueue.remove(task.getDriverTaskId()) == null) {
351+
readyQueue.decreaseReservedSize(task);
352+
}
351353
break;
352354
case BLOCKED:
353355
task.setStatus(DriverTaskStatus.ABORTED);
354356
blockedTasks.remove(task);
355-
readyQueue.decreaseReservedSize();
357+
readyQueue.decreaseReservedSize(task);
356358
break;
357359
case RUNNING:
358360
task.setStatus(DriverTaskStatus.ABORTED);
359-
readyQueue.decreaseReservedSize();
361+
readyQueue.decreaseReservedSize(task);
360362
break;
361363
case FINISHED:
362364
break;
@@ -422,6 +424,10 @@ public long getReadyQueueTaskCount() {
422424
return readyQueue.size();
423425
}
424426

427+
public long getReadyQueueReservedTaskCount() {
428+
return readyQueue.getReservedSize();
429+
}
430+
425431
public long getBlockQueueTaskCount() {
426432
return blockedTasks.size();
427433
}
@@ -497,6 +503,7 @@ public boolean readyToRunning(DriverTask task) {
497503
task.lock();
498504
try {
499505
if (task.getStatus() != DriverTaskStatus.READY) {
506+
readyQueue.decreaseReservedSize(task);
500507
return false;
501508
}
502509

@@ -553,7 +560,7 @@ public void runningToFinished(DriverTask task, ExecutionContext context) {
553560
}
554561
task.updateSchedulePriority(context);
555562
task.setStatus(DriverTaskStatus.FINISHED);
556-
readyQueue.decreaseReservedSize();
563+
readyQueue.decreaseReservedSize(task);
557564
} finally {
558565
task.unlock();
559566
}

‎iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/execution/schedule/queue/multilevelqueue/MultilevelPriorityQueue.java‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -191,6 +191,16 @@ protected DriverTask get(DriverTask driverTask) {
191191
"MultilevelPriorityQueue does not support access element by get.");
192192
}
193193

194+
@Override
195+
protected void markReserved(DriverTask task) {
196+
task.markReservedInReadyQueue();
197+
}
198+
199+
@Override
200+
protected boolean releaseReserved(DriverTask task) {
201+
return task.releaseReservedInReadyQueue();
202+
}
203+
194204
@Override
195205
protected void clearAllElements() {
196206
highestPriorityLevelQueue.clear();

‎iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/execution/schedule/task/DriverTask.java‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@ public class DriverTask implements IDIndexedAccessible {
6060
private final DriverTaskHandle driverTaskHandle;
6161
private long lastEnterReadyQueueTime;
6262
private long lastEnterBlockQueueTime;
63+
private boolean reservedInReadyQueue;
6364

6465
private long estimatedMemorySize;
6566

@@ -213,6 +214,18 @@ public void setLastEnterBlockQueueTime(long lastEnterBlockQueueTime) {
213214
this.lastEnterBlockQueueTime = lastEnterBlockQueueTime;
214215
}
215216

217+
public void markReservedInReadyQueue() {
218+
reservedInReadyQueue = true;
219+
}
220+
221+
public boolean releaseReservedInReadyQueue() {
222+
if (!reservedInReadyQueue) {
223+
return false;
224+
}
225+
reservedInReadyQueue = false;
226+
return true;
227+
}
228+
216229
/** a comparator of ddl, the less the ddl is, the low order it has. */
217230
public static class TimeoutComparator implements Comparator<DriverTask> {
218231

‎iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/metric/DriverSchedulerMetricSet.java‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ private DriverSchedulerMetricSet() {
3939
public static final String READY_QUEUED_TIME = "ready_queued_time";
4040
public static final String BLOCK_QUEUED_TIME = "block_queued_time";
4141
public static final String READY_QUEUE_TASK_COUNT = "ready_queue_task_count";
42+
public static final String READY_QUEUE_RESERVED_TASK_COUNT = "ready_queue_reserved_task_count";
4243
public static final String BLOCK_QUEUE_TASK_COUNT = "block_queue_task_count";
4344
private static final String TIMEOUT_QUEUE_SIZE = "timeout_queue_task_count";
4445
private static final String QUERY_MAP_SIZE = "query_map_size";
@@ -67,6 +68,13 @@ public void bindTo(AbstractMetricService metricService) {
6768
DriverScheduler::getReadyQueueTaskCount,
6869
Tag.NAME.toString(),
6970
READY_QUEUE_TASK_COUNT);
71+
metricService.createAutoGauge(
72+
Metric.DRIVER_SCHEDULER.toString(),
73+
MetricLevel.IMPORTANT,
74+
DriverScheduler.getInstance(),
75+
DriverScheduler::getReadyQueueReservedTaskCount,
76+
Tag.NAME.toString(),
77+
READY_QUEUE_RESERVED_TASK_COUNT);
7078
metricService.createAutoGauge(
7179
Metric.DRIVER_SCHEDULER.toString(),
7280
MetricLevel.IMPORTANT,
@@ -109,6 +117,11 @@ public void unbindFrom(AbstractMetricService metricService) {
109117
Metric.DRIVER_SCHEDULER.toString(),
110118
Tag.NAME.toString(),
111119
READY_QUEUE_TASK_COUNT);
120+
metricService.remove(
121+
MetricType.AUTO_GAUGE,
122+
Metric.DRIVER_SCHEDULER.toString(),
123+
Tag.NAME.toString(),
124+
READY_QUEUE_RESERVED_TASK_COUNT);
112125
metricService.remove(
113126
MetricType.AUTO_GAUGE,
114127
Metric.DRIVER_SCHEDULER.toString(),

‎iotdb-core/datanode/src/test/java/org/apache/iotdb/db/queryengine/execution/schedule/DefaultDriverSchedulerTest.java‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -197,6 +197,47 @@ public void testReadyToRunning() {
197197
clear();
198198
}
199199

200+
@Test
201+
public void testAbortReadyTaskAfterPollReleasesReadyQueueReservation() throws Exception {
202+
IMPPDataExchangeManager mockMPPDataExchangeManager =
203+
Mockito.mock(IMPPDataExchangeManager.class);
204+
manager.setBlockManager(mockMPPDataExchangeManager);
205+
ITaskScheduler defaultScheduler = manager.getScheduler();
206+
IDriver mockDriver = Mockito.mock(IDriver.class);
207+
DriverTaskHandle driverTaskHandle =
208+
new DriverTaskHandle(
209+
1,
210+
(MultilevelPriorityQueue) manager.getReadyQueue(),
211+
OptionalInt.of(Integer.MAX_VALUE));
212+
213+
QueryId queryId = new QueryId("test");
214+
FragmentInstanceId instanceId =
215+
new FragmentInstanceId(new PlanFragmentId(queryId, 0), "inst-0");
216+
DriverTaskId driverTaskID = new DriverTaskId(instanceId, 0);
217+
Mockito.when(mockDriver.getDriverTaskId()).thenReturn(driverTaskID);
218+
DriverTask testTask =
219+
new DriverTask(mockDriver, 100L, DriverTaskStatus.READY, driverTaskHandle, 0, false);
220+
221+
try {
222+
manager.registerTaskToQueryMap(queryId, testTask);
223+
manager.submitTaskToReadyQueue(testTask);
224+
225+
DriverTask polledTask = manager.getReadyQueue().poll();
226+
Assert.assertSame(testTask, polledTask);
227+
Assert.assertEquals(1, manager.getReadyQueueReservedTaskCount());
228+
229+
manager.abortFragmentInstance(instanceId, new RuntimeException("test abort"));
230+
231+
Assert.assertEquals(DriverTaskStatus.ABORTED, testTask.getStatus());
232+
Assert.assertEquals(0, manager.getReadyQueue().size());
233+
Assert.assertEquals(0, manager.getReadyQueueReservedTaskCount());
234+
Assert.assertFalse(defaultScheduler.readyToRunning(polledTask));
235+
Assert.assertEquals(0, manager.getReadyQueueReservedTaskCount());
236+
} finally {
237+
clear();
238+
}
239+
}
240+
200241
@Test
201242
public void testRunningToReady() {
202243
IMPPDataExchangeManager mockMPPDataExchangeManager =

0 commit comments

Comments
 (0)