-
Notifications
You must be signed in to change notification settings - Fork 214
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Fixes an issue with the end-to-end acknowledgements where the the sch…
…eduled monitor thread holds a user thread and prevents Data Prepper from shutting down correctly. The monitor now runs in a dedicated daemon thread and the callback methods are submitted to a distinct executor service with a lower bound of available threads. Includes various test improvements as well. (#2483) Signed-off-by: David Venable <[email protected]>
- Loading branch information
Showing
10 changed files
with
340 additions
and
115 deletions.
There are no files selected for viewing
29 changes: 29 additions & 0 deletions
29
...e/src/main/java/org/opensearch/dataprepper/acknowledgements/AcknowledgementAppConfig.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,29 @@ | ||
| /* | ||
| * Copyright OpenSearch Contributors | ||
| * SPDX-License-Identifier: Apache-2.0 | ||
| */ | ||
|
|
||
| package org.opensearch.dataprepper.acknowledgements; | ||
|
|
||
| import org.springframework.context.annotation.Bean; | ||
| import org.springframework.context.annotation.Configuration; | ||
|
|
||
| import java.util.concurrent.ExecutorService; | ||
| import java.util.concurrent.Executors; | ||
| import java.util.concurrent.ThreadFactory; | ||
|
|
||
| @Configuration | ||
| class AcknowledgementAppConfig { | ||
| private static final int MAX_THREADS = 12; | ||
|
|
||
| @Bean | ||
| CallbackTheadFactory callbackTheadFactory() { | ||
| final ThreadFactory defaultThreadFactory = Executors.defaultThreadFactory(); | ||
| return new CallbackTheadFactory(defaultThreadFactory); | ||
| } | ||
|
|
||
| @Bean(name = "acknowledgementCallbackExecutor") | ||
| ExecutorService acknowledgementCallbackExecutor(final CallbackTheadFactory callbackTheadFactory) { | ||
| return Executors.newFixedThreadPool(MAX_THREADS, callbackTheadFactory); | ||
| } | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
47 changes: 47 additions & 0 deletions
47
...ain/java/org/opensearch/dataprepper/acknowledgements/AcknowledgementSetMonitorThread.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,47 @@ | ||
| /* | ||
| * Copyright OpenSearch Contributors | ||
| * SPDX-License-Identifier: Apache-2.0 | ||
| */ | ||
|
|
||
| package org.opensearch.dataprepper.acknowledgements; | ||
|
|
||
| import java.time.Duration; | ||
|
|
||
| class AcknowledgementSetMonitorThread { | ||
| private final Thread monitorThread; | ||
| private final AcknowledgementSetMonitor acknowledgementSetMonitor; | ||
| private final Duration delayTime; | ||
| private volatile boolean isStopped = false; | ||
|
|
||
| public AcknowledgementSetMonitorThread( | ||
| final AcknowledgementSetMonitor acknowledgementSetMonitor, | ||
| final Duration delayTime) { | ||
| this.acknowledgementSetMonitor = acknowledgementSetMonitor; | ||
| this.delayTime = delayTime; | ||
| monitorThread = new Thread(new Monitor()); | ||
| monitorThread.setDaemon(true); | ||
| monitorThread.setName("acknowledgement-monitor"); | ||
| } | ||
|
|
||
| public void start() { | ||
| monitorThread.start(); | ||
| } | ||
|
|
||
| public void stop() { | ||
| isStopped = true; | ||
| } | ||
|
|
||
| private class Monitor implements Runnable { | ||
| @Override | ||
| public void run() { | ||
| while (!isStopped) { | ||
| acknowledgementSetMonitor.run(); | ||
| try { | ||
| Thread.sleep(delayTime.toMillis()); | ||
| } catch (final InterruptedException e) { | ||
| throw new RuntimeException(e); | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } |
26 changes: 26 additions & 0 deletions
26
...-core/src/main/java/org/opensearch/dataprepper/acknowledgements/CallbackTheadFactory.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,26 @@ | ||
| /* | ||
| * Copyright OpenSearch Contributors | ||
| * SPDX-License-Identifier: Apache-2.0 | ||
| */ | ||
|
|
||
| package org.opensearch.dataprepper.acknowledgements; | ||
|
|
||
| import java.util.Objects; | ||
| import java.util.concurrent.ThreadFactory; | ||
| import java.util.concurrent.atomic.AtomicInteger; | ||
|
|
||
| public class CallbackTheadFactory implements ThreadFactory { | ||
| private final ThreadFactory delegateFactory; | ||
| private final AtomicInteger threadNumber = new AtomicInteger(1); | ||
|
|
||
| public CallbackTheadFactory(final ThreadFactory delegateFactory) { | ||
| this.delegateFactory = Objects.requireNonNull(delegateFactory); | ||
| } | ||
|
|
||
| @Override | ||
| public Thread newThread(final Runnable runnable) { | ||
| final Thread thread = delegateFactory.newThread(runnable); | ||
| thread.setName("acknowledgement-callback-" + threadNumber.getAndIncrement()); | ||
| return thread; | ||
| } | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
49 changes: 49 additions & 0 deletions
49
...java/org/opensearch/dataprepper/acknowledgements/AcknowledgementSetMonitorThreadTest.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,49 @@ | ||
| /* | ||
| * Copyright OpenSearch Contributors | ||
| * SPDX-License-Identifier: Apache-2.0 | ||
| */ | ||
|
|
||
| package org.opensearch.dataprepper.acknowledgements; | ||
|
|
||
| import org.junit.jupiter.api.BeforeEach; | ||
| import org.junit.jupiter.api.Test; | ||
| import org.junit.jupiter.api.extension.ExtendWith; | ||
| import org.mockito.Mock; | ||
| import org.mockito.junit.jupiter.MockitoExtension; | ||
|
|
||
| import java.time.Duration; | ||
|
|
||
| import static org.awaitility.Awaitility.await; | ||
| import static org.mockito.Mockito.atLeastOnce; | ||
| import static org.mockito.Mockito.verify; | ||
|
|
||
| @ExtendWith(MockitoExtension.class) | ||
| class AcknowledgementSetMonitorThreadTest { | ||
| @Mock | ||
| private AcknowledgementSetMonitor acknowledgementSetMonitor; | ||
| private Duration delayTime; | ||
|
|
||
| @BeforeEach | ||
| void setUp() { | ||
| delayTime = Duration.ofMillis(10); | ||
| } | ||
|
|
||
| private AcknowledgementSetMonitorThread createObjectUnderTest() { | ||
| return new AcknowledgementSetMonitorThread(acknowledgementSetMonitor, delayTime); | ||
| } | ||
|
|
||
| @Test | ||
| void run_will_call_monitor_run() { | ||
| final AcknowledgementSetMonitorThread objectUnderTest = createObjectUnderTest(); | ||
|
|
||
| objectUnderTest.start(); | ||
| await().atMost(delayTime.plusMillis(500)) | ||
| .untilAsserted(() -> { | ||
| verify(acknowledgementSetMonitor, atLeastOnce()).run(); | ||
| }); | ||
|
|
||
| verify(acknowledgementSetMonitor, atLeastOnce()).run(); | ||
|
|
||
| objectUnderTest.stop(); | ||
| } | ||
| } |
Oops, something went wrong.