From 77551778bb1f5397b65a40bf15d07aa95ff17309 Mon Sep 17 00:00:00 2001 From: Khushboo Rajput Date: Mon, 14 Aug 2023 16:50:38 -0700 Subject: [PATCH 1/5] Handle Reader thread termination gracefully Signed-off-by: Khushboo Rajput --- build.gradle | 17 ++-- docker/docker-compose.yml | 2 - .../ESLocalhostConnection.java | 57 ++++++++++++++ .../PerformanceAnalyzerApp.java | 68 ++++++++++++++-- .../performanceanalyzer/rca/Version.java | 9 ++- .../PerformanceAnalyzerAppTest.java | 77 +++++++++++++++++++ .../resources/rca/batch_metrics_enabled.conf | 1 + 7 files changed, 210 insertions(+), 21 deletions(-) create mode 100644 src/main/java/org/opensearch/performanceanalyzer/ESLocalhostConnection.java create mode 100644 src/test/resources/rca/batch_metrics_enabled.conf diff --git a/build.gradle b/build.gradle index 596403c8e..451df7f67 100644 --- a/build.gradle +++ b/build.gradle @@ -34,7 +34,6 @@ buildscript { dependencies { classpath "org.opensearch.gradle:build-tools:${opensearch_version}" - classpath group: 'com.google.guava', name: 'guava', version: '31.1-jre' } } @@ -186,7 +185,7 @@ jacocoTestCoverageVerification { } } } - } + } else { violationRules { rule { @@ -194,7 +193,7 @@ jacocoTestCoverageVerification { minimum = 0.6 } } - } + } } } @@ -373,16 +372,18 @@ dependencies { strictly "2.23.0" } } - testImplementation group: 'org.powermock', name: 'powermock-core', version: '2.0.0' - testImplementation group: 'org.powermock', name: 'powermock-api-support', version: '2.0.0' - testImplementation group: 'org.powermock', name: 'powermock-module-junit4-common', version: '2.0.0' - testImplementation group: 'org.javassist', name: 'javassist', version: '3.24.0-GA' - testImplementation group: 'org.powermock', name: 'powermock-reflect', version: '2.0.0' + testImplementation group: 'org.powermock', name: 'powermock-core', version: '2.0.2' + testImplementation group: 'org.powermock', name: 'powermock-api-support', version: '2.0.2' + testImplementation group: 'org.powermock', name: 'powermock-module-junit4-common', version: '2.0.2' + testImplementation group: 'org.javassist', name: 'javassist', version: '3.28.0-GA' + testImplementation group: 'org.powermock', name: 'powermock-reflect', version: '2.0.2' testImplementation group: 'net.bytebuddy', name: 'byte-buddy', version: '1.9.3' testImplementation group: 'org.objenesis', name: 'objenesis', version: '3.0.1' testImplementation group: 'org.hamcrest', name: 'hamcrest-library', version: '2.1' testImplementation group: 'org.hamcrest', name: 'hamcrest', version: '2.1' testImplementation group: 'junit', name: 'junit', version: "${junitVersion}" + testImplementation group: 'org.xerial', name: 'sqlite-jdbc', version: '3.41.2.2' + testImplementation group: 'com.github.stefanbirkner', name: 'system-rules', version: '1.19.0' // Required for Docker build dockerBuild group: 'org.opensearch.plugin', name:'performance-analyzer', version: "${opensearch_build}" diff --git a/docker/docker-compose.yml b/docker/docker-compose.yml index 43f47f7f1..9e2e12380 100644 --- a/docker/docker-compose.yml +++ b/docker/docker-compose.yml @@ -32,8 +32,6 @@ services: opensearch2: container_name: opensearch2 - environment: - - node.cluster_manager=false image: opensearch/pa-rca:3.0 mem_limit: 4g networks: diff --git a/src/main/java/org/opensearch/performanceanalyzer/ESLocalhostConnection.java b/src/main/java/org/opensearch/performanceanalyzer/ESLocalhostConnection.java new file mode 100644 index 000000000..098c3a0b4 --- /dev/null +++ b/src/main/java/org/opensearch/performanceanalyzer/ESLocalhostConnection.java @@ -0,0 +1,57 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +package org.opensearch.performanceanalyzer; + + +import io.netty.handler.codec.http.HttpMethod; +import java.io.DataOutputStream; +import java.net.HttpURLConnection; +import java.net.URL; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; + +public class ESLocalhostConnection { + + private static final Logger LOG = LogManager.getLogger(ESLocalhostConnection.class); + + private static final int TIMEOUT_MILLIS = 30000; + + public static int makeHttpRequest(String path, HttpMethod httpMethod, String requestBody) { + HttpURLConnection connection = null; + try { + connection = createHTTPConnection(path, httpMethod); + DataOutputStream stream = new DataOutputStream(connection.getOutputStream()); + stream.writeBytes(requestBody); + stream.flush(); + stream.close(); + return connection.getResponseCode(); + } catch (Exception e) { + throw new RuntimeException("Request failed: " + e.getMessage(), e); + } finally { + if (connection != null) { + connection.disconnect(); + } + } + } + + public static HttpURLConnection createHTTPConnection(String path, HttpMethod httpMethod) { + try { + String endPoint = "https://localhost:9200" + path; + URL endpointUrl = new URL(endPoint); + HttpURLConnection connection = (HttpURLConnection) endpointUrl.openConnection(); + connection.setRequestMethod(httpMethod.toString()); + + connection.setConnectTimeout(TIMEOUT_MILLIS); + connection.setReadTimeout(TIMEOUT_MILLIS); + connection.setUseCaches(false); + connection.setDoOutput(true); + return connection; + } catch (Exception e) { + throw new RuntimeException( + "Failed to create OpenSearch Connection: " + e.getMessage(), e); + } + } +} diff --git a/src/main/java/org/opensearch/performanceanalyzer/PerformanceAnalyzerApp.java b/src/main/java/org/opensearch/performanceanalyzer/PerformanceAnalyzerApp.java index 785b0cdaa..de1590cbe 100644 --- a/src/main/java/org/opensearch/performanceanalyzer/PerformanceAnalyzerApp.java +++ b/src/main/java/org/opensearch/performanceanalyzer/PerformanceAnalyzerApp.java @@ -9,6 +9,8 @@ import com.google.common.annotations.VisibleForTesting; import com.google.common.util.concurrent.ThreadFactoryBuilder; import com.sun.net.httpserver.HttpServer; +import io.netty.handler.codec.http.HttpMethod; +import java.net.HttpURLConnection; import java.util.*; import java.util.concurrent.*; import org.apache.logging.log4j.LogManager; @@ -51,6 +53,7 @@ public class PerformanceAnalyzerApp { private static final Logger LOG = LogManager.getLogger(PerformanceAnalyzerApp.class); + public static final int READER_RESTART_MAX_ATTEMPTS = 3; private static final int EXCEPTION_QUEUE_LENGTH = 1; private static final ScheduledMetricCollectorsExecutor METRIC_COLLECTOR_EXECUTOR = new ScheduledMetricCollectorsExecutor(1, false); @@ -234,13 +237,14 @@ public static Thread startGrpcServerThread( return grpcServerThread; } - private static void startReaderThread( + public static void startReaderThread( final AppContext appContext, final ThreadProvider threadProvider) { PluginSettings settings = PluginSettings.instance(); final Thread readerThread = threadProvider.createThreadForRunnable( () -> { - while (true) { + int retryAttemptLeft = READER_RESTART_MAX_ATTEMPTS; + while (retryAttemptLeft > 0) { try { ReaderMetricsProcessor mp = new ReaderMetricsProcessor( @@ -250,23 +254,71 @@ private static void startReaderThread( ReaderMetricsProcessor.setCurrentInstance(mp); mp.run(); } catch (Throwable e) { - if (TroubleshootingConfig.getEnableDevAssert()) { - break; - } + retryAttemptLeft--; LOG.error( - "Error in ReaderMetricsProcessor...restarting, ExceptionCode: {}", - StatExceptionCode.READER_RESTART_PROCESSING.toString()); + "Error in ReaderMetricsProcessor...restarting, retryCount left: {}." + + "Exception: {}", + retryAttemptLeft, + e.getMessage()); StatsCollector.instance() .logException( StatExceptionCode.READER_RESTART_PROCESSING); + + if (TroubleshootingConfig.getEnableDevAssert()) break; + + // All retry attempts exhausted; handle thread failure + if (retryAttemptLeft <= 0) handleReaderThreadFailed(); } } }, PerformanceAnalyzerThreads.PA_READER); - readerThread.start(); } + private static void handleReaderThreadFailed() { + // Reader subcomponent is responsible for processing, cleaning metrics written by PA. + // Since Reader thread fails to start successfully, execute following: + // + // 1. Disable PA - Stop collecting OpenSearch metrics + // 2. Terminate RCA Process - Gracefully shutdown all + // existing resources/channels, including Reader. + try { + LOG.info( + "Exhausted {} attempts - unable to start Reader Thread successfully; disable PA", + READER_RESTART_MAX_ATTEMPTS); + disablePA(); + LOG.info("Attempt to disable PA succeeded."); + StatsCollector.instance() + .logException(StatExceptionCode.READER_ERROR_PA_DISABLE_SUCCESS); + } catch (Throwable e) { + LOG.info("Attempt to disable PA failing: {}", e.getMessage()); + StatsCollector.instance() + .logException(StatExceptionCode.READER_ERROR_PA_DISABLE_FAILED); + } finally { + cleanupAndExit(); + } + } + + private static void disablePA() { + String PA_CONFIG_PATH = Util.PA_BASE_URL + "/cluster/config"; + String PA_DISABLE_PAYLOAD = "{\"enabled\": false}"; + + int resCode = + ESLocalhostConnection.makeHttpRequest( + PA_CONFIG_PATH, HttpMethod.POST, PA_DISABLE_PAYLOAD); + if (resCode != HttpURLConnection.HTTP_OK) { + throw new RuntimeException("Failed to disable PA"); + } + } + + private static void cleanupAndExit() { + LOG.info("Reader thread not coming up successfully - Shutting down RCA Runtime"); + StatsCollector.instance().logException(StatExceptionCode.READER_ERROR_RCA_AGENT_STOPPED); + + // Terminate Java Runtime, executes {@link #shutDownGracefully(ClientServers clientServers)} + System.exit(1); + } + /** * Start all the servers and clients for request processing. We start two servers: - httpServer: * To handle the curl requests sent to the endpoint. This is human readable and also used by the diff --git a/src/main/java/org/opensearch/performanceanalyzer/rca/Version.java b/src/main/java/org/opensearch/performanceanalyzer/rca/Version.java index bfc85fcd3..402013cf7 100644 --- a/src/main/java/org/opensearch/performanceanalyzer/rca/Version.java +++ b/src/main/java/org/opensearch/performanceanalyzer/rca/Version.java @@ -19,11 +19,14 @@ public final class Version { * transferred packets should be dropped. Every increment here should be accompanied with a line * describing the version bump. * - * Note: The RCA version is agnostic of OpenSearch version. + *

Note: The RCA version is agnostic of OpenSearch version. */ static final class Major { - // Bumping this post the Commons Lib(https://github.com/opensearch-project/performance-analyzer-commons/issues/2) - // and Service Metrics(https://github.com/opensearch-project/performance-analyzer-commons/issues/8) change + // Bumping this post the Commons + // Lib(https://github.com/opensearch-project/performance-analyzer-commons/issues/2) + // and Service + // Metrics(https://github.com/opensearch-project/performance-analyzer-commons/issues/8) + // change static final int RCA_MAJ_VERSION = 1; } diff --git a/src/test/java/org/opensearch/performanceanalyzer/PerformanceAnalyzerAppTest.java b/src/test/java/org/opensearch/performanceanalyzer/PerformanceAnalyzerAppTest.java index 3d838b1c7..dc347dcef 100644 --- a/src/test/java/org/opensearch/performanceanalyzer/PerformanceAnalyzerAppTest.java +++ b/src/test/java/org/opensearch/performanceanalyzer/PerformanceAnalyzerAppTest.java @@ -5,22 +5,49 @@ package org.opensearch.performanceanalyzer; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.times; +import static org.powermock.api.mockito.PowerMockito.doNothing; +import static org.powermock.api.mockito.PowerMockito.doThrow; +import static org.powermock.api.mockito.PowerMockito.mock; +import static org.powermock.api.mockito.PowerMockito.verifyPrivate; +import io.netty.handler.codec.http.HttpMethod; import java.util.concurrent.ArrayBlockingQueue; import org.junit.Assert; import org.junit.Before; +import org.junit.Rule; import org.junit.Test; +import org.junit.contrib.java.lang.system.ExpectedSystemExit; +import org.junit.runner.RunWith; import org.opensearch.performanceanalyzer.commons.config.ConfigStatus; import org.opensearch.performanceanalyzer.commons.stats.metrics.StatExceptionCode; import org.opensearch.performanceanalyzer.rca.RcaTestHelper; +import org.opensearch.performanceanalyzer.reader.ReaderMetricsProcessor; import org.opensearch.performanceanalyzer.threads.ThreadProvider; import org.opensearch.performanceanalyzer.threads.exceptions.PAThreadException; +import org.powermock.api.mockito.PowerMockito; +import org.powermock.core.classloader.annotations.PowerMockIgnore; +import org.powermock.core.classloader.annotations.PrepareForTest; +import org.powermock.modules.junit4.PowerMockRunner; +@RunWith(PowerMockRunner.class) +@PrepareForTest({ESLocalhostConnection.class, PerformanceAnalyzerApp.class}) +@PowerMockIgnore({ + "com.sun.org.apache.xerces.*", + "javax.xml.*", + "org.xml.*", + "javax.management.*", + "org.w3c.*" +}) public class PerformanceAnalyzerAppTest { + @Rule public final ExpectedSystemExit exit = ExpectedSystemExit.none(); @Before public void setup() { System.setProperty("performanceanalyzer.metrics.log.enabled", "False"); + RcaTestHelper.cleanUpLogs(); } @Test @@ -49,4 +76,54 @@ public void testStartErrorHandlingThread() throws InterruptedException { RcaTestHelper.verifyStatException( StatExceptionCode.ERROR_HANDLER_THREAD_STOPPED.toString())); } + + @Test + public void testStartReaderThreadAllAttemptFail() throws Exception { + ThreadProvider threadProvider = new ThreadProvider(); + AppContext appContext = new AppContext(); + + PowerMockito.mockStatic(ESLocalhostConnection.class); + ReaderMetricsProcessor readerMetricsProcessor = mock(ReaderMetricsProcessor.class); + doThrow(new RuntimeException("Force Crashing Reader Thread")) + .when(readerMetricsProcessor) + .run(); + PowerMockito.whenNew(ReaderMetricsProcessor.class) + .withAnyArguments() + .thenReturn(readerMetricsProcessor); + + PowerMockito.spy(PerformanceAnalyzerApp.class); + doNothing().when(PerformanceAnalyzerApp.class, "cleanupAndExit"); + + // PA Disable Success + PowerMockito.when( + ESLocalhostConnection.makeHttpRequest( + anyString(), eq(HttpMethod.POST), anyString())) + .thenReturn(200); + PerformanceAnalyzerApp.startReaderThread(appContext, threadProvider); + Assert.assertTrue( + "READER_RESTART_PROCESSING metric missing", + RcaTestHelper.verifyStatException( + StatExceptionCode.READER_RESTART_PROCESSING.toString())); + Assert.assertTrue( + "READER_ERROR_PA_DISABLE_SUCCESS metric missing", + RcaTestHelper.verifyStatException( + StatExceptionCode.READER_ERROR_PA_DISABLE_SUCCESS.toString())); + verifyPrivate(PerformanceAnalyzerApp.class, times(1)).invoke("cleanupAndExit"); + + // PA Disable Fail + PowerMockito.when( + ESLocalhostConnection.makeHttpRequest( + anyString(), eq(HttpMethod.POST), anyString())) + .thenReturn(500); + PerformanceAnalyzerApp.startReaderThread(appContext, threadProvider); + Assert.assertTrue( + "READER_RESTART_PROCESSING metric missing", + RcaTestHelper.verifyStatException( + StatExceptionCode.READER_RESTART_PROCESSING.toString())); + Assert.assertTrue( + "READER_ERROR_PA_DISABLE_FAILED metric missing", + RcaTestHelper.verifyStatException( + StatExceptionCode.READER_ERROR_PA_DISABLE_FAILED.toString())); + verifyPrivate(PerformanceAnalyzerApp.class, times(2)).invoke("cleanupAndExit"); + } } diff --git a/src/test/resources/rca/batch_metrics_enabled.conf b/src/test/resources/rca/batch_metrics_enabled.conf new file mode 100644 index 000000000..c508d5366 --- /dev/null +++ b/src/test/resources/rca/batch_metrics_enabled.conf @@ -0,0 +1 @@ +false From 7451fc46578951048c51775a7dcf7789954ec66b Mon Sep 17 00:00:00 2001 From: Khushboo Rajput Date: Tue, 5 Sep 2023 21:37:14 -0700 Subject: [PATCH 2/5] Adding retry for PA disable Signed-off-by: Khushboo Rajput --- .../ESLocalhostConnection.java | 57 --------------- .../LocalhostConnectionUtil.java | 69 +++++++++++++++++++ .../PerformanceAnalyzerApp.java | 22 ++---- .../PerformanceAnalyzerAppTest.java | 24 +++---- 4 files changed, 81 insertions(+), 91 deletions(-) delete mode 100644 src/main/java/org/opensearch/performanceanalyzer/ESLocalhostConnection.java create mode 100644 src/main/java/org/opensearch/performanceanalyzer/LocalhostConnectionUtil.java diff --git a/src/main/java/org/opensearch/performanceanalyzer/ESLocalhostConnection.java b/src/main/java/org/opensearch/performanceanalyzer/ESLocalhostConnection.java deleted file mode 100644 index 098c3a0b4..000000000 --- a/src/main/java/org/opensearch/performanceanalyzer/ESLocalhostConnection.java +++ /dev/null @@ -1,57 +0,0 @@ -/* - * Copyright OpenSearch Contributors - * SPDX-License-Identifier: Apache-2.0 - */ - -package org.opensearch.performanceanalyzer; - - -import io.netty.handler.codec.http.HttpMethod; -import java.io.DataOutputStream; -import java.net.HttpURLConnection; -import java.net.URL; -import org.apache.logging.log4j.LogManager; -import org.apache.logging.log4j.Logger; - -public class ESLocalhostConnection { - - private static final Logger LOG = LogManager.getLogger(ESLocalhostConnection.class); - - private static final int TIMEOUT_MILLIS = 30000; - - public static int makeHttpRequest(String path, HttpMethod httpMethod, String requestBody) { - HttpURLConnection connection = null; - try { - connection = createHTTPConnection(path, httpMethod); - DataOutputStream stream = new DataOutputStream(connection.getOutputStream()); - stream.writeBytes(requestBody); - stream.flush(); - stream.close(); - return connection.getResponseCode(); - } catch (Exception e) { - throw new RuntimeException("Request failed: " + e.getMessage(), e); - } finally { - if (connection != null) { - connection.disconnect(); - } - } - } - - public static HttpURLConnection createHTTPConnection(String path, HttpMethod httpMethod) { - try { - String endPoint = "https://localhost:9200" + path; - URL endpointUrl = new URL(endPoint); - HttpURLConnection connection = (HttpURLConnection) endpointUrl.openConnection(); - connection.setRequestMethod(httpMethod.toString()); - - connection.setConnectTimeout(TIMEOUT_MILLIS); - connection.setReadTimeout(TIMEOUT_MILLIS); - connection.setUseCaches(false); - connection.setDoOutput(true); - return connection; - } catch (Exception e) { - throw new RuntimeException( - "Failed to create OpenSearch Connection: " + e.getMessage(), e); - } - } -} diff --git a/src/main/java/org/opensearch/performanceanalyzer/LocalhostConnectionUtil.java b/src/main/java/org/opensearch/performanceanalyzer/LocalhostConnectionUtil.java new file mode 100644 index 000000000..a5109cf54 --- /dev/null +++ b/src/main/java/org/opensearch/performanceanalyzer/LocalhostConnectionUtil.java @@ -0,0 +1,69 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +package org.opensearch.performanceanalyzer; + + +import io.netty.handler.codec.http.HttpMethod; +import java.io.DataOutputStream; +import java.net.HttpURLConnection; +import java.net.URL; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; +import org.opensearch.performanceanalyzer.core.Util; + +public class LocalhostConnectionUtil { + + private static final int TIMEOUT_MILLIS = 30000; + + private static final Logger LOG = LogManager.getLogger(LocalhostConnectionUtil.class); + + public static void disablePA() throws InterruptedException { + String PA_CONFIG_PATH = Util.PA_BASE_URL + "/cluster/config"; + String PA_DISABLE_PAYLOAD = "{\"enabled\": false}"; + int retryCount = 3; + + while (retryCount > 0) { + HttpURLConnection connection = null; + try { + connection = createHTTPConnection(PA_CONFIG_PATH, HttpMethod.POST); + DataOutputStream stream = new DataOutputStream(connection.getOutputStream()); + stream.writeBytes(PA_DISABLE_PAYLOAD); + stream.flush(); + stream.close(); + if (connection.getResponseCode() == HttpURLConnection.HTTP_OK) { + return; + } + } catch (Exception e) { + LOG.info("PA Disable Request failed: " + e.getMessage(), e); + } finally { + if (connection != null) { + connection.disconnect(); + } + } + --retryCount; + Thread.sleep((int) (5000 * (Math.random() * 2) + 100)); + } + throw new RuntimeException("Failed to disable PA after " + retryCount + " attempts"); + } + + private static HttpURLConnection createHTTPConnection(String path, HttpMethod httpMethod) { + try { + String endPoint = "https://localhost:9200" + path; + URL endpointUrl = new URL(endPoint); + HttpURLConnection connection = (HttpURLConnection) endpointUrl.openConnection(); + connection.setRequestMethod(httpMethod.toString()); + + connection.setConnectTimeout(TIMEOUT_MILLIS); + connection.setReadTimeout(TIMEOUT_MILLIS); + connection.setUseCaches(false); + connection.setDoOutput(true); + return connection; + } catch (Exception e) { + throw new RuntimeException( + "Failed to create OpenSearch Connection: " + e.getMessage(), e); + } + } +} diff --git a/src/main/java/org/opensearch/performanceanalyzer/PerformanceAnalyzerApp.java b/src/main/java/org/opensearch/performanceanalyzer/PerformanceAnalyzerApp.java index de1590cbe..5ce62077a 100644 --- a/src/main/java/org/opensearch/performanceanalyzer/PerformanceAnalyzerApp.java +++ b/src/main/java/org/opensearch/performanceanalyzer/PerformanceAnalyzerApp.java @@ -9,8 +9,6 @@ import com.google.common.annotations.VisibleForTesting; import com.google.common.util.concurrent.ThreadFactoryBuilder; import com.sun.net.httpserver.HttpServer; -import io.netty.handler.codec.http.HttpMethod; -import java.net.HttpURLConnection; import java.util.*; import java.util.concurrent.*; import org.apache.logging.log4j.LogManager; @@ -53,7 +51,7 @@ public class PerformanceAnalyzerApp { private static final Logger LOG = LogManager.getLogger(PerformanceAnalyzerApp.class); - public static final int READER_RESTART_MAX_ATTEMPTS = 3; + public static final int READER_RESTART_MAX_ATTEMPTS = 12; private static final int EXCEPTION_QUEUE_LENGTH = 1; private static final ScheduledMetricCollectorsExecutor METRIC_COLLECTOR_EXECUTOR = new ScheduledMetricCollectorsExecutor(1, false); @@ -286,12 +284,12 @@ private static void handleReaderThreadFailed() { LOG.info( "Exhausted {} attempts - unable to start Reader Thread successfully; disable PA", READER_RESTART_MAX_ATTEMPTS); - disablePA(); - LOG.info("Attempt to disable PA succeeded."); + LocalhostConnectionUtil.disablePA(); + LOG.info("PA disable succeeded. "); StatsCollector.instance() .logException(StatExceptionCode.READER_ERROR_PA_DISABLE_SUCCESS); } catch (Throwable e) { - LOG.info("Attempt to disable PA failing: {}", e.getMessage()); + LOG.info(e.getMessage()); StatsCollector.instance() .logException(StatExceptionCode.READER_ERROR_PA_DISABLE_FAILED); } finally { @@ -299,18 +297,6 @@ private static void handleReaderThreadFailed() { } } - private static void disablePA() { - String PA_CONFIG_PATH = Util.PA_BASE_URL + "/cluster/config"; - String PA_DISABLE_PAYLOAD = "{\"enabled\": false}"; - - int resCode = - ESLocalhostConnection.makeHttpRequest( - PA_CONFIG_PATH, HttpMethod.POST, PA_DISABLE_PAYLOAD); - if (resCode != HttpURLConnection.HTTP_OK) { - throw new RuntimeException("Failed to disable PA"); - } - } - private static void cleanupAndExit() { LOG.info("Reader thread not coming up successfully - Shutting down RCA Runtime"); StatsCollector.instance().logException(StatExceptionCode.READER_ERROR_RCA_AGENT_STOPPED); diff --git a/src/test/java/org/opensearch/performanceanalyzer/PerformanceAnalyzerAppTest.java b/src/test/java/org/opensearch/performanceanalyzer/PerformanceAnalyzerAppTest.java index dc347dcef..979d51837 100644 --- a/src/test/java/org/opensearch/performanceanalyzer/PerformanceAnalyzerAppTest.java +++ b/src/test/java/org/opensearch/performanceanalyzer/PerformanceAnalyzerAppTest.java @@ -5,15 +5,12 @@ package org.opensearch.performanceanalyzer; -import static org.mockito.ArgumentMatchers.anyString; -import static org.mockito.ArgumentMatchers.eq; -import static org.mockito.Mockito.times; +import static org.mockito.Mockito.atLeastOnce; import static org.powermock.api.mockito.PowerMockito.doNothing; import static org.powermock.api.mockito.PowerMockito.doThrow; import static org.powermock.api.mockito.PowerMockito.mock; import static org.powermock.api.mockito.PowerMockito.verifyPrivate; -import io.netty.handler.codec.http.HttpMethod; import java.util.concurrent.ArrayBlockingQueue; import org.junit.Assert; import org.junit.Before; @@ -33,7 +30,7 @@ import org.powermock.modules.junit4.PowerMockRunner; @RunWith(PowerMockRunner.class) -@PrepareForTest({ESLocalhostConnection.class, PerformanceAnalyzerApp.class}) +@PrepareForTest({LocalhostConnectionUtil.class, PerformanceAnalyzerApp.class}) @PowerMockIgnore({ "com.sun.org.apache.xerces.*", "javax.xml.*", @@ -82,7 +79,7 @@ public void testStartReaderThreadAllAttemptFail() throws Exception { ThreadProvider threadProvider = new ThreadProvider(); AppContext appContext = new AppContext(); - PowerMockito.mockStatic(ESLocalhostConnection.class); + PowerMockito.mockStatic(LocalhostConnectionUtil.class); ReaderMetricsProcessor readerMetricsProcessor = mock(ReaderMetricsProcessor.class); doThrow(new RuntimeException("Force Crashing Reader Thread")) .when(readerMetricsProcessor) @@ -95,10 +92,7 @@ public void testStartReaderThreadAllAttemptFail() throws Exception { doNothing().when(PerformanceAnalyzerApp.class, "cleanupAndExit"); // PA Disable Success - PowerMockito.when( - ESLocalhostConnection.makeHttpRequest( - anyString(), eq(HttpMethod.POST), anyString())) - .thenReturn(200); + doNothing().when(LocalhostConnectionUtil.class, "disablePA"); PerformanceAnalyzerApp.startReaderThread(appContext, threadProvider); Assert.assertTrue( "READER_RESTART_PROCESSING metric missing", @@ -108,13 +102,11 @@ public void testStartReaderThreadAllAttemptFail() throws Exception { "READER_ERROR_PA_DISABLE_SUCCESS metric missing", RcaTestHelper.verifyStatException( StatExceptionCode.READER_ERROR_PA_DISABLE_SUCCESS.toString())); - verifyPrivate(PerformanceAnalyzerApp.class, times(1)).invoke("cleanupAndExit"); + verifyPrivate(PerformanceAnalyzerApp.class, atLeastOnce()).invoke("cleanupAndExit"); // PA Disable Fail - PowerMockito.when( - ESLocalhostConnection.makeHttpRequest( - anyString(), eq(HttpMethod.POST), anyString())) - .thenReturn(500); + doThrow(new RuntimeException("Failed to disable PA")) + .when(LocalhostConnectionUtil.class, "disablePA"); PerformanceAnalyzerApp.startReaderThread(appContext, threadProvider); Assert.assertTrue( "READER_RESTART_PROCESSING metric missing", @@ -124,6 +116,6 @@ public void testStartReaderThreadAllAttemptFail() throws Exception { "READER_ERROR_PA_DISABLE_FAILED metric missing", RcaTestHelper.verifyStatException( StatExceptionCode.READER_ERROR_PA_DISABLE_FAILED.toString())); - verifyPrivate(PerformanceAnalyzerApp.class, times(2)).invoke("cleanupAndExit"); + verifyPrivate(PerformanceAnalyzerApp.class, atLeastOnce()).invoke("cleanupAndExit"); } } From 8696f575e8fb495c8ca1d896785d176f9a4111bc Mon Sep 17 00:00:00 2001 From: Khushboo Rajput Date: Tue, 5 Sep 2023 22:50:09 -0700 Subject: [PATCH 3/5] Add Content-Type for PA disable --- build.gradle | 3 +- licenses/commons-lang3-LICENSE.txt | 202 ------------------ .../LocalhostConnectionUtil.java | 12 +- .../PerformanceAnalyzerApp.java | 4 +- 4 files changed, 13 insertions(+), 208 deletions(-) delete mode 100644 licenses/commons-lang3-LICENSE.txt diff --git a/build.gradle b/build.gradle index 451df7f67..873d02ce2 100644 --- a/build.gradle +++ b/build.gradle @@ -328,6 +328,7 @@ dependencies { } def versions = VersionProperties.versions + def commonslangVersion = "${versions.commonslang}" def jacksonVersion = "${versions.jackson}" def jacksonDataBindVersion = "${versions.jackson_databind}" def nettyVersion = "${versions.netty}" @@ -349,7 +350,7 @@ dependencies { implementation "org.opensearch:performance-analyzer-commons:1.0.0-SNAPSHOT" implementation group: 'org.apache.logging.log4j', name: 'log4j-api', version: "${log4jVersion}" implementation group: 'org.apache.logging.log4j', name: 'log4j-core', version: "${log4jVersion}" - implementation group: 'org.apache.commons', name: 'commons-lang3', version: '3.9' + implementation group: 'org.apache.commons', name: 'commons-lang3', version: "${commonslangVersion}" implementation group: 'commons-io', name: 'commons-io', version: '2.7' implementation group: 'com.google.errorprone', name: 'error_prone_annotations', version: '2.9.0' implementation group: 'com.google.protobuf', name: 'protobuf-java', version: "${protobufVersion}" diff --git a/licenses/commons-lang3-LICENSE.txt b/licenses/commons-lang3-LICENSE.txt deleted file mode 100644 index 7a4a3ea24..000000000 --- a/licenses/commons-lang3-LICENSE.txt +++ /dev/null @@ -1,202 +0,0 @@ - - Apache License - Version 2.0, January 2004 - http://www.apache.org/licenses/ - - TERMS AND CONDITIONS FOR USE, REPRODUCTION, AND DISTRIBUTION - - 1. Definitions. - - "License" shall mean the terms and conditions for use, reproduction, - and distribution as defined by Sections 1 through 9 of this document. - - "Licensor" shall mean the copyright owner or entity authorized by - the copyright owner that is granting the License. - - "Legal Entity" shall mean the union of the acting entity and all - other entities that control, are controlled by, or are under common - control with that entity. For the purposes of this definition, - "control" means (i) the power, direct or indirect, to cause the - direction or management of such entity, whether by contract or - otherwise, or (ii) ownership of fifty percent (50%) or more of the - outstanding shares, or (iii) beneficial ownership of such entity. - - "You" (or "Your") shall mean an individual or Legal Entity - exercising permissions granted by this License. - - "Source" form shall mean the preferred form for making modifications, - including but not limited to software source code, documentation - source, and configuration files. - - "Object" form shall mean any form resulting from mechanical - transformation or translation of a Source form, including but - not limited to compiled object code, generated documentation, - and conversions to other media types. - - "Work" shall mean the work of authorship, whether in Source or - Object form, made available under the License, as indicated by a - copyright notice that is included in or attached to the work - (an example is provided in the Appendix below). - - "Derivative Works" shall mean any work, whether in Source or Object - form, that is based on (or derived from) the Work and for which the - editorial revisions, annotations, elaborations, or other modifications - represent, as a whole, an original work of authorship. For the purposes - of this License, Derivative Works shall not include works that remain - separable from, or merely link (or bind by name) to the interfaces of, - the Work and Derivative Works thereof. - - "Contribution" shall mean any work of authorship, including - the original version of the Work and any modifications or additions - to that Work or Derivative Works thereof, that is intentionally - submitted to Licensor for inclusion in the Work by the copyright owner - or by an individual or Legal Entity authorized to submit on behalf of - the copyright owner. For the purposes of this definition, "submitted" - means any form of electronic, verbal, or written communication sent - to the Licensor or its representatives, including but not limited to - communication on electronic mailing lists, source code control systems, - and issue tracking systems that are managed by, or on behalf of, the - Licensor for the purpose of discussing and improving the Work, but - excluding communication that is conspicuously marked or otherwise - designated in writing by the copyright owner as "Not a Contribution." - - "Contributor" shall mean Licensor and any individual or Legal Entity - on behalf of whom a Contribution has been received by Licensor and - subsequently incorporated within the Work. - - 2. Grant of Copyright License. Subject to the terms and conditions of - this License, each Contributor hereby grants to You a perpetual, - worldwide, non-exclusive, no-charge, royalty-free, irrevocable - copyright license to reproduce, prepare Derivative Works of, - publicly display, publicly perform, sublicense, and distribute the - Work and such Derivative Works in Source or Object form. - - 3. Grant of Patent License. Subject to the terms and conditions of - this License, each Contributor hereby grants to You a perpetual, - worldwide, non-exclusive, no-charge, royalty-free, irrevocable - (except as stated in this section) patent license to make, have made, - use, offer to sell, sell, import, and otherwise transfer the Work, - where such license applies only to those patent claims licensable - by such Contributor that are necessarily infringed by their - Contribution(s) alone or by combination of their Contribution(s) - with the Work to which such Contribution(s) was submitted. If You - institute patent litigation against any entity (including a - cross-claim or counterclaim in a lawsuit) alleging that the Work - or a Contribution incorporated within the Work constitutes direct - or contributory patent infringement, then any patent licenses - granted to You under this License for that Work shall terminate - as of the date such litigation is filed. - - 4. Redistribution. You may reproduce and distribute copies of the - Work or Derivative Works thereof in any medium, with or without - modifications, and in Source or Object form, provided that You - meet the following conditions: - - (a) You must give any other recipients of the Work or - Derivative Works a copy of this License; and - - (b) You must cause any modified files to carry prominent notices - stating that You changed the files; and - - (c) You must retain, in the Source form of any Derivative Works - that You distribute, all copyright, patent, trademark, and - attribution notices from the Source form of the Work, - excluding those notices that do not pertain to any part of - the Derivative Works; and - - (d) If the Work includes a "NOTICE" text file as part of its - distribution, then any Derivative Works that You distribute must - include a readable copy of the attribution notices contained - within such NOTICE file, excluding those notices that do not - pertain to any part of the Derivative Works, in at least one - of the following places: within a NOTICE text file distributed - as part of the Derivative Works; within the Source form or - documentation, if provided along with the Derivative Works; or, - within a display generated by the Derivative Works, if and - wherever such third-party notices normally appear. The contents - of the NOTICE file are for informational purposes only and - do not modify the License. You may add Your own attribution - notices within Derivative Works that You distribute, alongside - or as an addendum to the NOTICE text from the Work, provided - that such additional attribution notices cannot be construed - as modifying the License. - - You may add Your own copyright statement to Your modifications and - may provide additional or different license terms and conditions - for use, reproduction, or distribution of Your modifications, or - for any such Derivative Works as a whole, provided Your use, - reproduction, and distribution of the Work otherwise complies with - the conditions stated in this License. - - 5. Submission of Contributions. Unless You explicitly state otherwise, - any Contribution intentionally submitted for inclusion in the Work - by You to the Licensor shall be under the terms and conditions of - this License, without any additional terms or conditions. - Notwithstanding the above, nothing herein shall supersede or modify - the terms of any separate license agreement you may have executed - with Licensor regarding such Contributions. - - 6. Trademarks. This License does not grant permission to use the trade - names, trademarks, service marks, or product names of the Licensor, - except as required for reasonable and customary use in describing the - origin of the Work and reproducing the content of the NOTICE file. - - 7. Disclaimer of Warranty. Unless required by applicable law or - agreed to in writing, Licensor provides the Work (and each - Contributor provides its Contributions) on an "AS IS" BASIS, - WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or - implied, including, without limitation, any warranties or conditions - of TITLE, NON-INFRINGEMENT, MERCHANTABILITY, or FITNESS FOR A - PARTICULAR PURPOSE. You are solely responsible for determining the - appropriateness of using or redistributing the Work and assume any - risks associated with Your exercise of permissions under this License. - - 8. Limitation of Liability. In no event and under no legal theory, - whether in tort (including negligence), contract, or otherwise, - unless required by applicable law (such as deliberate and grossly - negligent acts) or agreed to in writing, shall any Contributor be - liable to You for damages, including any direct, indirect, special, - incidental, or consequential damages of any character arising as a - result of this License or out of the use or inability to use the - Work (including but not limited to damages for loss of goodwill, - work stoppage, computer failure or malfunction, or any and all - other commercial damages or losses), even if such Contributor - has been advised of the possibility of such damages. - - 9. Accepting Warranty or Additional Liability. While redistributing - the Work or Derivative Works thereof, You may choose to offer, - and charge a fee for, acceptance of support, warranty, indemnity, - or other liability obligations and/or rights consistent with this - License. However, in accepting such obligations, You may act only - on Your own behalf and on Your sole responsibility, not on behalf - of any other Contributor, and only if You agree to indemnify, - defend, and hold each Contributor harmless for any liability - incurred by, or claims asserted against, such Contributor by reason - of your accepting any such warranty or additional liability. - - END OF TERMS AND CONDITIONS - - APPENDIX: How to apply the Apache License to your work. - - To apply the Apache License to your work, attach the following - boilerplate notice, with the fields enclosed by brackets "[]" - replaced with your own identifying information. (Don't include - the brackets!) The text should be enclosed in the appropriate - comment syntax for the file format. We also recommend that a - file or class name and description of purpose be included on the - same "printed page" as the copyright notice for easier - identification within third-party archives. - - Copyright [yyyy] [name of copyright owner] - - Licensed under the Apache License, Version 2.0 (the "License"); - you may not use this file except in compliance with the License. - You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - - Unless required by applicable law or agreed to in writing, software - distributed under the License is distributed on an "AS IS" BASIS, - WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - See the License for the specific language governing permissions and - limitations under the License. \ No newline at end of file diff --git a/src/main/java/org/opensearch/performanceanalyzer/LocalhostConnectionUtil.java b/src/main/java/org/opensearch/performanceanalyzer/LocalhostConnectionUtil.java index a5109cf54..b79514d43 100644 --- a/src/main/java/org/opensearch/performanceanalyzer/LocalhostConnectionUtil.java +++ b/src/main/java/org/opensearch/performanceanalyzer/LocalhostConnectionUtil.java @@ -33,11 +33,16 @@ public static void disablePA() throws InterruptedException { stream.writeBytes(PA_DISABLE_PAYLOAD); stream.flush(); stream.close(); + LOG.info( + "PA Disable Response: " + + connection.getResponseCode() + + " " + + connection.getResponseMessage()); if (connection.getResponseCode() == HttpURLConnection.HTTP_OK) { return; } } catch (Exception e) { - LOG.info("PA Disable Request failed: " + e.getMessage(), e); + LOG.error("PA Disable Request failed: " + e.getMessage(), e); } finally { if (connection != null) { connection.disconnect(); @@ -46,15 +51,16 @@ public static void disablePA() throws InterruptedException { --retryCount; Thread.sleep((int) (5000 * (Math.random() * 2) + 100)); } - throw new RuntimeException("Failed to disable PA after " + retryCount + " attempts"); + throw new RuntimeException("Failed to disable PA after 3 attempts"); } private static HttpURLConnection createHTTPConnection(String path, HttpMethod httpMethod) { try { - String endPoint = "https://localhost:9200" + path; + String endPoint = "http://localhost:9200" + path; URL endpointUrl = new URL(endPoint); HttpURLConnection connection = (HttpURLConnection) endpointUrl.openConnection(); connection.setRequestMethod(httpMethod.toString()); + connection.setRequestProperty("Content-Type", "application/json"); connection.setConnectTimeout(TIMEOUT_MILLIS); connection.setReadTimeout(TIMEOUT_MILLIS); diff --git a/src/main/java/org/opensearch/performanceanalyzer/PerformanceAnalyzerApp.java b/src/main/java/org/opensearch/performanceanalyzer/PerformanceAnalyzerApp.java index 5ce62077a..455bbbed4 100644 --- a/src/main/java/org/opensearch/performanceanalyzer/PerformanceAnalyzerApp.java +++ b/src/main/java/org/opensearch/performanceanalyzer/PerformanceAnalyzerApp.java @@ -289,7 +289,7 @@ private static void handleReaderThreadFailed() { StatsCollector.instance() .logException(StatExceptionCode.READER_ERROR_PA_DISABLE_SUCCESS); } catch (Throwable e) { - LOG.info(e.getMessage()); + LOG.error(e.getMessage()); StatsCollector.instance() .logException(StatExceptionCode.READER_ERROR_PA_DISABLE_FAILED); } finally { @@ -298,7 +298,7 @@ private static void handleReaderThreadFailed() { } private static void cleanupAndExit() { - LOG.info("Reader thread not coming up successfully - Shutting down RCA Runtime"); + LOG.error("Reader thread not coming up successfully - Shutting down RCA Runtime"); StatsCollector.instance().logException(StatExceptionCode.READER_ERROR_RCA_AGENT_STOPPED); // Terminate Java Runtime, executes {@link #shutDownGracefully(ClientServers clientServers)} From 6405acfaa5aace8119351736f54473273443ed98 Mon Sep 17 00:00:00 2001 From: Khushboo Rajput Date: Wed, 6 Sep 2023 10:41:51 -0700 Subject: [PATCH 4/5] Remove commons-lang3-NOTICE.txt Signed-off-by: Khushboo Rajput --- licenses/commons-lang3-NOTICE.txt | 5 ----- 1 file changed, 5 deletions(-) delete mode 100644 licenses/commons-lang3-NOTICE.txt diff --git a/licenses/commons-lang3-NOTICE.txt b/licenses/commons-lang3-NOTICE.txt deleted file mode 100644 index a6fffa56d..000000000 --- a/licenses/commons-lang3-NOTICE.txt +++ /dev/null @@ -1,5 +0,0 @@ -Apache Commons Lang -Copyright 2001-2019 The Apache Software Foundation - -This product includes software developed at -The Apache Software Foundation (http://www.apache.org/). \ No newline at end of file From f50e3ee544e0850046a2f435a9e0ae5ee2a1da4b Mon Sep 17 00:00:00 2001 From: Khushboo Rajput Date: Wed, 6 Sep 2023 16:02:07 -0700 Subject: [PATCH 5/5] Update supervisord configuration for process failure expected codes and PROCESS_STATE_FATAL handling Signed-off-by: Khushboo Rajput --- config/supervisord.conf | 11 +++++++++-- .../performanceanalyzer/LocalhostConnectionUtil.java | 6 +++--- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/config/supervisord.conf b/config/supervisord.conf index 47162f7b4..e4c061cd8 100644 --- a/config/supervisord.conf +++ b/config/supervisord.conf @@ -7,7 +7,7 @@ chmod=0770 [supervisord] logfile=/usr/share/supervisor/performance_analyzer/supervisord.log ; (main log file;default $CWD/supervisord.log) pidfile=/usr/share/supervisor/performance_analyzer/supervisord.pid ; (supervisord pidfile;default supervisord.pid) -childlogdir=/usr/share/supervisor/performance_analyzer ; ('AUTO' child log dir, default $TEMP) +childlogdir=/usr/share/supervisor/performance_analyzer ; ('AUTO' child log dir, default $TEMP) ; the below section must remain in the config file for RPC ; (supervisorctl/web interface) to work, additional interfaces may be @@ -23,10 +23,17 @@ serverurl=/usr/share/supervisord.sock ; newlines). It can also contain wildcards. The filenames are ; interpreted as relative to this file. Included files *cannot* ; include files themselves. - [include] files = /etc/supervisor/conf.d/*.conf [program:performance_analyzer] command=/usr/share/opensearch/performance-analyzer-rca/bin/performance-analyzer-agent /usr/share/opensearch user=1000 +autostart=true ; start at supervisord start (default: true) +autorestart=unexpected ; autorestart if exited after running (def: unexpected) +exitcodes=1 ; 'expected' exit codes used with autorestart, System.exit(1) called from PerformanceAnalyzerApp.cleanupAndExit() - if Reader Thread crashes. + +[eventlistener:stop_supervisord] +command=bash -c "printf 'READY\n' && while read line; do kill -SIGQUIT $PPID; done < /dev/stdin" +events=PROCESS_STATE_FATAL +buffer_size=100 diff --git a/src/main/java/org/opensearch/performanceanalyzer/LocalhostConnectionUtil.java b/src/main/java/org/opensearch/performanceanalyzer/LocalhostConnectionUtil.java index b79514d43..c40fcbea8 100644 --- a/src/main/java/org/opensearch/performanceanalyzer/LocalhostConnectionUtil.java +++ b/src/main/java/org/opensearch/performanceanalyzer/LocalhostConnectionUtil.java @@ -23,7 +23,7 @@ public class LocalhostConnectionUtil { public static void disablePA() throws InterruptedException { String PA_CONFIG_PATH = Util.PA_BASE_URL + "/cluster/config"; String PA_DISABLE_PAYLOAD = "{\"enabled\": false}"; - int retryCount = 3; + int retryCount = 5; while (retryCount > 0) { HttpURLConnection connection = null; @@ -49,9 +49,9 @@ public static void disablePA() throws InterruptedException { } } --retryCount; - Thread.sleep((int) (5000 * (Math.random() * 2) + 100)); + Thread.sleep((int) (60000 * (Math.random() * 2) + 100)); } - throw new RuntimeException("Failed to disable PA after 3 attempts"); + throw new RuntimeException("Failed to disable PA after 5 attempts"); } private static HttpURLConnection createHTTPConnection(String path, HttpMethod httpMethod) {