Skip to content

Commit ca9a8a7

Browse files
committed
Fix interrupt
1 parent 556cd66 commit ca9a8a7

6 files changed

Lines changed: 160 additions & 13 deletions

File tree

iotdb-core/datanode/src/main/java/org/apache/iotdb/db/pipe/source/dataregion/realtime/assigner/DisruptorQueue.java

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -87,16 +87,29 @@ public DisruptorQueue(
8787
}
8888

8989
public void publish(final PipeRealtimeEvent event) {
90+
publishOrDrop(event);
91+
}
92+
93+
public boolean publishOrDrop(final PipeRealtimeEvent event) {
9094
final EnrichedEvent innerEvent = event.getEvent();
9195
if (innerEvent instanceof PipeHeartbeatEvent) {
9296
((PipeHeartbeatEvent) innerEvent).recordDisruptorSize(ringBuffer);
9397
}
94-
ringBuffer.publishEvent((container, sequence, o) -> container.setEvent(event), event);
95-
mayPrintExceedingLog();
98+
final boolean published =
99+
ringBuffer.publishEvent(
100+
(container, sequence, o) -> container.setEvent(event), event, this::isClosed);
101+
if (published) {
102+
mayPrintExceedingLog();
103+
}
104+
return published;
96105
}
97106

98-
public void shutdown() {
107+
public void closeInput() {
99108
isClosed = true;
109+
}
110+
111+
public void shutdown() {
112+
closeInput();
100113
// use shutdown instead of halt to ensure all published events have been handled
101114
disruptor.shutdown();
102115
allocatedMemoryBlock.close();

iotdb-core/datanode/src/main/java/org/apache/iotdb/db/pipe/source/dataregion/realtime/assigner/PipeDataRegionAssigner.java

Lines changed: 36 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@ public class PipeDataRegionAssigner implements Closeable {
6767
private Boolean isTableModel;
6868

6969
private final PipeEventCounter eventCounter = new PipeDataRegionEventCounter();
70+
private int inFlightPublishCount = 0;
7071

7172
public int getDataRegionId() {
7273
return dataRegionId;
@@ -101,14 +102,28 @@ public void publishToAssign(final PipeRealtimeEvent event) {
101102
((PipeHeartbeatEvent) innerEvent).onPublished();
102103
}
103104

104-
// use synchronized here for completely preventing reference count leaks under extreme thread
105-
// scheduling when closing
106105
synchronized (this) {
107-
if (!disruptor.isClosed()) {
108-
disruptor.publish(event);
109-
} else {
106+
if (disruptor.isClosed()) {
110107
onAssignedHook(event);
108+
return;
111109
}
110+
inFlightPublishCount++;
111+
}
112+
113+
boolean isPublished = false;
114+
try {
115+
isPublished = disruptor.publishOrDrop(event);
116+
} finally {
117+
synchronized (this) {
118+
inFlightPublishCount--;
119+
if (inFlightPublishCount == 0) {
120+
notifyAll();
121+
}
122+
}
123+
}
124+
125+
if (!isPublished) {
126+
onAssignedHook(event);
112127
}
113128
}
114129

@@ -254,9 +269,25 @@ public boolean notMoreSourceNeededToBeAssigned() {
254269
public synchronized void close() {
255270
PipeAssignerMetrics.getInstance().deregister(dataRegionId);
256271

272+
boolean interrupted = false;
273+
disruptor.closeInput();
274+
while (inFlightPublishCount > 0) {
275+
try {
276+
wait();
277+
} catch (final InterruptedException e) {
278+
interrupted = true;
279+
LOGGER.warn(
280+
"Interrupted while waiting for in-flight publishes to finish when closing assigner on data region {}.",
281+
dataRegionId);
282+
}
283+
}
284+
257285
final long startTime = System.currentTimeMillis();
258286
disruptor.shutdown();
259287
matcher.clear();
288+
if (interrupted) {
289+
Thread.currentThread().interrupt();
290+
}
260291
LOGGER.info(
261292
"Pipe: Assigner on data region {} shutdown internal disruptor within {} ms",
262293
dataRegionId,

iotdb-core/datanode/src/main/java/org/apache/iotdb/db/pipe/source/dataregion/realtime/disruptor/BatchEventProcessor.java

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -80,11 +80,14 @@ public void run() {
8080
nextSequence = processAvailableEvents(nextSequence, availableSequence);
8181

8282
} catch (final InterruptedException ex) {
83-
if (running) {
84-
Thread.currentThread().interrupt();
85-
LOGGER.info("Processor interrupted");
83+
if (!running) {
84+
break;
8685
}
87-
break;
86+
// A transient interrupt should not permanently stop the consumer thread. Otherwise the
87+
// gating sequence will stop advancing and producers may block forever on a full ring
88+
// buffer, making the later close path appear stuck.
89+
Thread.interrupted();
90+
LOGGER.warn("Processor interrupted unexpectedly, continue running");
8891
} catch (final Throwable ex) {
8992
exceptionHandler.handleEventException(ex, nextSequence, ringBuffer.get(nextSequence));
9093
sequence.set(nextSequence);

iotdb-core/datanode/src/main/java/org/apache/iotdb/db/pipe/source/dataregion/realtime/disruptor/MultiProducerSequencer.java

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
import java.util.concurrent.atomic.AtomicIntegerArray;
2424
import java.util.concurrent.atomic.AtomicReference;
2525
import java.util.concurrent.locks.LockSupport;
26+
import java.util.function.BooleanSupplier;
2627

2728
/**
2829
* Multi-producer sequencer for coordinating concurrent publishers
@@ -110,14 +111,31 @@ public MultiProducerSequencer(int bufferSize, Sequence[] gatingSequences) {
110111
* @return highest claimed sequence number
111112
*/
112113
public long next(int n) {
114+
return next(n, () -> false);
115+
}
116+
117+
/**
118+
* Claim next n sequences for publishing, or abort if the caller is closing.
119+
*
120+
* @param n number of sequences to claim
121+
* @param abortCondition returns {@code true} if the claim should be abandoned
122+
* @return highest claimed sequence number, or {@link Sequence#INITIAL_VALUE} if aborted
123+
*/
124+
public long next(final int n, final BooleanSupplier abortCondition) {
113125
if (n < 1) {
114126
throw new IllegalArgumentException("n must be > 0");
115127
}
116128

129+
final BooleanSupplier effectiveAbortCondition =
130+
abortCondition != null ? abortCondition : () -> false;
117131
long current;
118132
long next;
119133

120134
do {
135+
if (effectiveAbortCondition.getAsBoolean()) {
136+
return Sequence.INITIAL_VALUE;
137+
}
138+
121139
current = cursor.get();
122140
next = current + n;
123141

@@ -128,6 +146,9 @@ public long next(int n) {
128146
long gatingSequence = Sequence.getMinimumSequence(gatingSequences.get(), current);
129147

130148
if (wrapPoint > gatingSequence) {
149+
if (effectiveAbortCondition.getAsBoolean()) {
150+
return Sequence.INITIAL_VALUE;
151+
}
131152
LockSupport.parkNanos(1);
132153
continue;
133154
}

iotdb-core/datanode/src/main/java/org/apache/iotdb/db/pipe/source/dataregion/realtime/disruptor/RingBuffer.java

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@
1919

2020
package org.apache.iotdb.db.pipe.source.dataregion.realtime.disruptor;
2121

22+
import java.util.function.BooleanSupplier;
23+
2224
/**
2325
* Left-hand side padding for cache line alignment
2426
*
@@ -205,8 +207,28 @@ public void publish(long lo, long hi) {
205207
* @param <A> argument type
206208
*/
207209
public <A> void publishEvent(EventTranslator<E, A> translator, A arg0) {
208-
final long sequence = sequencer.next(1);
210+
publishEvent(translator, arg0, () -> false);
211+
}
212+
213+
/**
214+
* Publish event using a translator function, or abort if the caller is closing.
215+
*
216+
* @param translator function to populate the event
217+
* @param arg0 argument passed to translator
218+
* @param abortCondition returns {@code true} if the publish should be abandoned
219+
* @param <A> argument type
220+
* @return {@code true} if the event is published, {@code false} if the publish is aborted
221+
*/
222+
public <A> boolean publishEvent(
223+
final EventTranslator<E, A> translator,
224+
final A arg0,
225+
final BooleanSupplier abortCondition) {
226+
final long sequence = sequencer.next(1, abortCondition);
227+
if (sequence == Sequence.INITIAL_VALUE) {
228+
return false;
229+
}
209230
translateAndPublish(translator, sequence, arg0);
231+
return true;
210232
}
211233

212234
/**

iotdb-core/datanode/src/test/java/org/apache/iotdb/db/pipe/source/dataregion/realtime/disruptor/DisruptorShutdownTest.java

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
import java.util.concurrent.CountDownLatch;
2626
import java.util.concurrent.ThreadFactory;
2727
import java.util.concurrent.TimeUnit;
28+
import java.util.concurrent.atomic.AtomicBoolean;
2829
import java.util.concurrent.atomic.AtomicInteger;
2930
import java.util.concurrent.atomic.AtomicReference;
3031

@@ -113,6 +114,62 @@ public void testDisruptorShutdownInterruptsWaitingProcessor() throws Exception {
113114
Assert.assertFalse(processorThread.isAlive());
114115
}
115116

117+
@Test
118+
public void testUnexpectedInterruptDoesNotStopProcessor() throws Exception {
119+
final AtomicReference<Thread> processorThreadReference = new AtomicReference<>();
120+
final ThreadFactory threadFactory =
121+
runnable -> {
122+
final Thread thread =
123+
new Thread(runnable, "pipe-disruptor-unexpected-interrupt-test");
124+
processorThreadReference.set(thread);
125+
return thread;
126+
};
127+
128+
final CountDownLatch handled = new CountDownLatch(1);
129+
final Disruptor<TestEvent> disruptor = new Disruptor<>(TestEvent::new, 32, threadFactory);
130+
final RingBuffer<TestEvent> ringBuffer =
131+
disruptor.handleEventsWith((event, sequence, endOfBatch) -> handled.countDown()).start();
132+
133+
final Thread processorThread = processorThreadReference.get();
134+
Assert.assertNotNull(processorThread);
135+
136+
TimeUnit.MILLISECONDS.sleep(50);
137+
processorThread.interrupt();
138+
139+
ringBuffer.publishEvent((event, sequence, value) -> event.value = value, 1);
140+
Assert.assertTrue(handled.await(5, TimeUnit.SECONDS));
141+
Assert.assertTrue(processorThread.isAlive());
142+
143+
disruptor.shutdown();
144+
Assert.assertFalse(processorThread.isAlive());
145+
}
146+
147+
@Test
148+
public void testPublishEventCanAbortWhenClosingWhileBufferIsFull() throws Exception {
149+
final RingBuffer<TestEvent> ringBuffer = RingBuffer.createMultiProducer(TestEvent::new, 1);
150+
final Sequence gatingSequence = new Sequence();
151+
ringBuffer.addGatingSequences(gatingSequence);
152+
ringBuffer.publishEvent((event, sequence, value) -> event.value = value, 1);
153+
154+
final AtomicBoolean isClosed = new AtomicBoolean(false);
155+
final AtomicBoolean published = new AtomicBoolean(true);
156+
final Thread publisherThread =
157+
new Thread(
158+
() ->
159+
published.set(
160+
ringBuffer.publishEvent(
161+
(event, sequence, value) -> event.value = value, 2, isClosed::get)),
162+
"pipe-disruptor-publish-abort-test");
163+
164+
publisherThread.start();
165+
TimeUnit.MILLISECONDS.sleep(50);
166+
isClosed.set(true);
167+
publisherThread.join(TimeUnit.SECONDS.toMillis(5));
168+
169+
Assert.assertFalse(publisherThread.isAlive());
170+
Assert.assertFalse(published.get());
171+
}
172+
116173
private static class TestEvent {
117174
private int value;
118175
}

0 commit comments

Comments
 (0)