From a6a71a0f907b348fedf6951b13b66ba88fabbabe Mon Sep 17 00:00:00 2001 From: jmatsuok Date: Thu, 30 Jan 2025 19:36:06 -0500 Subject: [PATCH 1/5] Increase default timeout length, add custom timeout for upload actions --- src/main/java/io/cryostat/ConfigProperties.java | 2 ++ .../recordings/ActiveRecordingsDownload.java | 7 ++++++- .../recordings/LongRunningRequestGenerator.java | 10 ++++++++-- .../java/io/cryostat/recordings/RecordingHelper.java | 12 ++++++++---- .../RemoteRecordingInputStreamFactory.java | 8 +++++--- .../java/io/cryostat/reports/ReportsServiceImpl.java | 6 +++++- .../io/cryostat/targets/TargetConnectionManager.java | 5 +++++ src/main/resources/application.properties | 3 ++- 8 files changed, 41 insertions(+), 12 deletions(-) diff --git a/src/main/java/io/cryostat/ConfigProperties.java b/src/main/java/io/cryostat/ConfigProperties.java index 81342e5d0..c315240b7 100644 --- a/src/main/java/io/cryostat/ConfigProperties.java +++ b/src/main/java/io/cryostat/ConfigProperties.java @@ -32,6 +32,8 @@ public class ConfigProperties { public static final String CONNECTIONS_TTL = "cryostat.connections.ttl"; public static final String CONNECTIONS_FAILED_BACKOFF = "cryostat.connections.failed-backoff"; public static final String CONNECTIONS_FAILED_TIMEOUT = "cryostat.connections.failed-timeout"; + public static final String CONNECTIONS_UPLOAD_FAILED_TIMEOUT = + "cryostat.connections.upload-failed-timeout"; public static final String REPORTS_SIDECAR_URL = "quarkus.rest-client.reports.url"; public static final String REPORTS_MEMORY_CACHE_ENABLED = diff --git a/src/main/java/io/cryostat/recordings/ActiveRecordingsDownload.java b/src/main/java/io/cryostat/recordings/ActiveRecordingsDownload.java index ed60352c2..5fd470a53 100644 --- a/src/main/java/io/cryostat/recordings/ActiveRecordingsDownload.java +++ b/src/main/java/io/cryostat/recordings/ActiveRecordingsDownload.java @@ -49,6 +49,9 @@ public class ActiveRecordingsDownload { @Named(Producers.BASE64_URL) Base64 base64Url; + @ConfigProperty(name = ConfigProperties.CONNECTIONS_FAILED_TIMEOUT) + Duration connectionFailedTimeout; + @ConfigProperty(name = ConfigProperties.STORAGE_TRANSIENT_ARCHIVES_ENABLED) boolean transientArchivesEnabled; @@ -66,7 +69,9 @@ public RestResponse handleActiveDownload(@RestPath long id) throws HttpHeaders.CONTENT_DISPOSITION, String.format("attachment; filename=\"%s.jfr\"", recording.name)) .header(HttpHeaders.CONTENT_TYPE, HttpMimeType.OCTET_STREAM.mime()) - .entity(recordingHelper.getActiveInputStream(recording)) + .entity( + recordingHelper.getActiveInputStream( + recording, connectionFailedTimeout)) .build(); } diff --git a/src/main/java/io/cryostat/recordings/LongRunningRequestGenerator.java b/src/main/java/io/cryostat/recordings/LongRunningRequestGenerator.java index fae8525a8..7ef1e402c 100644 --- a/src/main/java/io/cryostat/recordings/LongRunningRequestGenerator.java +++ b/src/main/java/io/cryostat/recordings/LongRunningRequestGenerator.java @@ -62,6 +62,9 @@ public class LongRunningRequestGenerator { @ConfigProperty(name = ConfigProperties.CONNECTIONS_FAILED_TIMEOUT) Duration timeout; + @ConfigProperty(name = ConfigProperties.CONNECTIONS_UPLOAD_FAILED_TIMEOUT) + Duration uploadFailedTimeout; + public LongRunningRequestGenerator() {} @ConsumeEvent(value = ARCHIVE_ADDRESS, blocking = true) @@ -88,7 +91,10 @@ public void onMessage(ArchiveRequest request) { public void onMessage(GrafanaArchiveUploadRequest request) { try { logger.trace("Job ID: " + request.getId() + " submitted."); - recordingHelper.uploadToJFRDatasource(request.getPair()).await().atMost(timeout); + recordingHelper + .uploadToJFRDatasource(request.getPair()) + .await() + .atMost(uploadFailedTimeout); logger.trace("Grafana upload complete, firing notification"); bus.publish( MessagingServer.class.getName(), @@ -108,7 +114,7 @@ public void onMessage(GrafanaActiveUploadRequest request) { recordingHelper .uploadToJFRDatasource(request.getTargetId(), request.getRemoteId()) .await() - .atMost(timeout); + .atMost(uploadFailedTimeout); logger.trace("Grafana upload complete, firing notification"); bus.publish( MessagingServer.class.getName(), diff --git a/src/main/java/io/cryostat/recordings/RecordingHelper.java b/src/main/java/io/cryostat/recordings/RecordingHelper.java index bd734245c..aefbcbb1b 100644 --- a/src/main/java/io/cryostat/recordings/RecordingHelper.java +++ b/src/main/java/io/cryostat/recordings/RecordingHelper.java @@ -175,6 +175,9 @@ public class RecordingHelper { @ConfigProperty(name = ConfigProperties.CONNECTIONS_FAILED_TIMEOUT) Duration connectionFailedTimeout; + @ConfigProperty(name = ConfigProperties.CONNECTIONS_UPLOAD_FAILED_TIMEOUT) + Duration uploadFailedTimeout; + @ConfigProperty(name = ConfigProperties.GRAFANA_DATASOURCE_URL) Optional grafanaDatasourceURLProperty; @@ -821,7 +824,7 @@ public ArchivedRecording archiveRecording( String multipartId = null; List> parts = new ArrayList<>(); long accum = 0; - try (var stream = getActiveInputStream(recording); + try (var stream = getActiveInputStream(recording, uploadFailedTimeout); var ch = Channels.newChannel(stream)) { ByteBuffer buf = ByteBuffer.allocate(20 * MIB); CreateMultipartUploadRequest.Builder builder = @@ -973,14 +976,15 @@ public Pair decodedKey(String encodedKey) { return Pair.of(parts[0], parts[1]); } - public InputStream getActiveInputStream(ActiveRecording recording) throws Exception { - return remoteRecordingStreamFactory.open(recording); + public InputStream getActiveInputStream(ActiveRecording recording, Duration timeout) + throws Exception { + return remoteRecordingStreamFactory.open(recording, timeout); } public InputStream getActiveInputStream(long targetId, long remoteId) throws Exception { var target = Target.getTargetById(targetId); var recording = target.getRecordingById(remoteId); - var stream = remoteRecordingStreamFactory.open(recording); + var stream = remoteRecordingStreamFactory.open(recording, connectionFailedTimeout); return stream; } diff --git a/src/main/java/io/cryostat/recordings/RemoteRecordingInputStreamFactory.java b/src/main/java/io/cryostat/recordings/RemoteRecordingInputStreamFactory.java index 055552552..35e86b1b5 100644 --- a/src/main/java/io/cryostat/recordings/RemoteRecordingInputStreamFactory.java +++ b/src/main/java/io/cryostat/recordings/RemoteRecordingInputStreamFactory.java @@ -16,6 +16,7 @@ package io.cryostat.recordings; import java.io.InputStream; +import java.time.Duration; import org.openjdk.jmc.flightrecorder.configuration.IRecordingDescriptor; @@ -33,14 +34,15 @@ public class RemoteRecordingInputStreamFactory { @Inject TargetConnectionManager connectionManager; @Inject RecordingHelper recordingHelper; - public ProgressInputStream open(ActiveRecording recording) throws Exception { - return connectionManager.executeConnectedTask( + public ProgressInputStream open(ActiveRecording recording, Duration timeout) throws Exception { + return connectionManager.executeConnectedTaskTimeout( recording.target, conn -> { IRecordingDescriptor desc = recordingHelper.getDescriptor(conn, recording).orElseThrow(); return open(conn, recording.target, desc); - }); + }, + timeout); } public ProgressInputStream open(JFRConnection conn, Target target, IRecordingDescriptor desc) diff --git a/src/main/java/io/cryostat/reports/ReportsServiceImpl.java b/src/main/java/io/cryostat/reports/ReportsServiceImpl.java index fe449c2ec..7de5659ec 100644 --- a/src/main/java/io/cryostat/reports/ReportsServiceImpl.java +++ b/src/main/java/io/cryostat/reports/ReportsServiceImpl.java @@ -17,6 +17,7 @@ import java.io.BufferedInputStream; import java.io.InputStream; +import java.time.Duration; import java.util.Map; import java.util.function.Predicate; @@ -41,6 +42,9 @@ class ReportsServiceImpl implements ReportsService { private static final String NO_SIDECAR_URL = "http://localhost/"; + @ConfigProperty(name = ConfigProperties.CONNECTIONS_FAILED_TIMEOUT) + Duration connectionFailedTimeout; + @ConfigProperty(name = ConfigProperties.REPORTS_SIDECAR_URL) String sidecarUri; @@ -55,7 +59,7 @@ public Uni> reportFor( ActiveRecording recording, Predicate predicate) { InputStream stream; try { - stream = helper.getActiveInputStream(recording); + stream = helper.getActiveInputStream(recording, connectionFailedTimeout); } catch (Exception e) { throw new ReportGenerationException(e); } diff --git a/src/main/java/io/cryostat/targets/TargetConnectionManager.java b/src/main/java/io/cryostat/targets/TargetConnectionManager.java index 51c3ca36d..d6fe0c009 100644 --- a/src/main/java/io/cryostat/targets/TargetConnectionManager.java +++ b/src/main/java/io/cryostat/targets/TargetConnectionManager.java @@ -207,6 +207,11 @@ public T executeConnectedTask(Target target, ConnectedTask task) { return executeConnectedTaskUni(target, task).await().atMost(failedTimeout); } + public T executeConnectedTaskTimeout( + Target target, ConnectedTask task, Duration timeout) { + return executeConnectedTaskUni(target, task).await().atMost(timeout); + } + public Uni executeDirect( Target target, Optional credentials, ConnectedTask task) { return executeInternal( diff --git a/src/main/resources/application.properties b/src/main/resources/application.properties index 5a2910330..9a157a286 100644 --- a/src/main/resources/application.properties +++ b/src/main/resources/application.properties @@ -28,7 +28,8 @@ quarkus.test.integration-test-profile=test cryostat.connections.max-open=0 cryostat.connections.ttl=10s cryostat.connections.failed-backoff=2s -cryostat.connections.failed-timeout=10s +cryostat.connections.failed-timeout=30s +cryostat.connections.upload-failed-timeout=120s quarkus.rest-client.reports.url=http://localhost/ quarkus.cache.enabled=true cryostat.services.reports.memory-cache.enabled=true From cde28e0494f1c3c704fbdeffe6ad8bb48300d04b Mon Sep 17 00:00:00 2001 From: jmatsuok Date: Thu, 30 Jan 2025 20:08:06 -0500 Subject: [PATCH 2/5] Increase test timeout, increase upload timeout --- src/main/resources/application-test.properties | 1 + src/main/resources/application.properties | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/main/resources/application-test.properties b/src/main/resources/application-test.properties index 261585f9b..1e5e21dcf 100644 --- a/src/main/resources/application-test.properties +++ b/src/main/resources/application-test.properties @@ -6,6 +6,7 @@ cryostat.discovery.docker.enabled=true cryostat.discovery.kubernetes.enabled=true quarkus.test.env.JAVA_OPTS_APPEND=-Dquarkus.http.host=0.0.0.0 -Djava.util.logging.manager=org.jboss.logmanager.LogManager -Dcom.sun.management.jmxremote -Dcom.sun.management.jmxremote.port=9091 -Dcom.sun.management.jmxremote.rmi.port=9091 -Djava.rmi.server.hostname=127.0.0.1 -Dcom.sun.management.jmxremote.authenticate=false -Dcom.sun.management.jmxremote.ssl=false -Dcom.sun.management.jmxremote.local.only=false +quarkus.http.test-timeout=60s quarkus.datasource.devservices.enabled=true quarkus.datasource.devservices.image-name=quay.io/cryostat/cryostat-db diff --git a/src/main/resources/application.properties b/src/main/resources/application.properties index 5c11f4d3e..150da83ee 100644 --- a/src/main/resources/application.properties +++ b/src/main/resources/application.properties @@ -29,7 +29,7 @@ cryostat.connections.max-open=0 cryostat.connections.ttl=10s cryostat.connections.failed-backoff=2s cryostat.connections.failed-timeout=30s -cryostat.connections.upload-failed-timeout=120s +cryostat.connections.upload-failed-timeout=30m quarkus.rest-client.reports.url=http://localhost/ quarkus.rest-client.reports.verify-host=true quarkus.cache.enabled=true From eb3ebcda8975ccfe591c8c809aed7c98342f9679 Mon Sep 17 00:00:00 2001 From: Andrew Azores Date: Thu, 30 Jan 2025 21:04:50 -0500 Subject: [PATCH 3/5] rename --- src/main/java/io/cryostat/ConfigProperties.java | 4 ++-- .../io/cryostat/recordings/LongRunningRequestGenerator.java | 2 +- src/main/java/io/cryostat/recordings/RecordingHelper.java | 2 +- src/main/resources/application.properties | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/main/java/io/cryostat/ConfigProperties.java b/src/main/java/io/cryostat/ConfigProperties.java index c315240b7..5969024cc 100644 --- a/src/main/java/io/cryostat/ConfigProperties.java +++ b/src/main/java/io/cryostat/ConfigProperties.java @@ -32,8 +32,8 @@ public class ConfigProperties { public static final String CONNECTIONS_TTL = "cryostat.connections.ttl"; public static final String CONNECTIONS_FAILED_BACKOFF = "cryostat.connections.failed-backoff"; public static final String CONNECTIONS_FAILED_TIMEOUT = "cryostat.connections.failed-timeout"; - public static final String CONNECTIONS_UPLOAD_FAILED_TIMEOUT = - "cryostat.connections.upload-failed-timeout"; + public static final String CONNECTIONS_UPLOAD_TIMEOUT = + "cryostat.connections.upload-timeout"; public static final String REPORTS_SIDECAR_URL = "quarkus.rest-client.reports.url"; public static final String REPORTS_MEMORY_CACHE_ENABLED = diff --git a/src/main/java/io/cryostat/recordings/LongRunningRequestGenerator.java b/src/main/java/io/cryostat/recordings/LongRunningRequestGenerator.java index 8c326bc43..d65070a1c 100644 --- a/src/main/java/io/cryostat/recordings/LongRunningRequestGenerator.java +++ b/src/main/java/io/cryostat/recordings/LongRunningRequestGenerator.java @@ -62,7 +62,7 @@ public class LongRunningRequestGenerator { @ConfigProperty(name = ConfigProperties.CONNECTIONS_FAILED_TIMEOUT) Duration timeout; - @ConfigProperty(name = ConfigProperties.CONNECTIONS_UPLOAD_FAILED_TIMEOUT) + @ConfigProperty(name = ConfigProperties.CONNECTIONS_UPLOAD_TIMEOUT) Duration uploadFailedTimeout; public LongRunningRequestGenerator() {} diff --git a/src/main/java/io/cryostat/recordings/RecordingHelper.java b/src/main/java/io/cryostat/recordings/RecordingHelper.java index dd67edf6c..36a2eb6a4 100644 --- a/src/main/java/io/cryostat/recordings/RecordingHelper.java +++ b/src/main/java/io/cryostat/recordings/RecordingHelper.java @@ -176,7 +176,7 @@ public class RecordingHelper { @ConfigProperty(name = ConfigProperties.CONNECTIONS_FAILED_TIMEOUT) Duration connectionFailedTimeout; - @ConfigProperty(name = ConfigProperties.CONNECTIONS_UPLOAD_FAILED_TIMEOUT) + @ConfigProperty(name = ConfigProperties.CONNECTIONS_UPLOAD_TIMEOUT) Duration uploadFailedTimeout; @ConfigProperty(name = ConfigProperties.GRAFANA_DATASOURCE_URL) diff --git a/src/main/resources/application.properties b/src/main/resources/application.properties index 150da83ee..c9c684f94 100644 --- a/src/main/resources/application.properties +++ b/src/main/resources/application.properties @@ -29,7 +29,7 @@ cryostat.connections.max-open=0 cryostat.connections.ttl=10s cryostat.connections.failed-backoff=2s cryostat.connections.failed-timeout=30s -cryostat.connections.upload-failed-timeout=30m +cryostat.connections.upload-timeout=30m quarkus.rest-client.reports.url=http://localhost/ quarkus.rest-client.reports.verify-host=true quarkus.cache.enabled=true From 2e8aa4f30194100d2b9f9434736d87d8d0639b50 Mon Sep 17 00:00:00 2001 From: Andrew Azores Date: Thu, 30 Jan 2025 21:08:57 -0500 Subject: [PATCH 4/5] refactor --- src/main/java/io/cryostat/ConfigProperties.java | 3 +-- .../java/io/cryostat/recordings/RecordingHelper.java | 11 ++++++----- .../recordings/RemoteRecordingInputStreamFactory.java | 8 ++++---- .../io/cryostat/targets/TargetConnectionManager.java | 5 ++--- 4 files changed, 13 insertions(+), 14 deletions(-) diff --git a/src/main/java/io/cryostat/ConfigProperties.java b/src/main/java/io/cryostat/ConfigProperties.java index 5969024cc..b117d3965 100644 --- a/src/main/java/io/cryostat/ConfigProperties.java +++ b/src/main/java/io/cryostat/ConfigProperties.java @@ -32,8 +32,7 @@ public class ConfigProperties { public static final String CONNECTIONS_TTL = "cryostat.connections.ttl"; public static final String CONNECTIONS_FAILED_BACKOFF = "cryostat.connections.failed-backoff"; public static final String CONNECTIONS_FAILED_TIMEOUT = "cryostat.connections.failed-timeout"; - public static final String CONNECTIONS_UPLOAD_TIMEOUT = - "cryostat.connections.upload-timeout"; + public static final String CONNECTIONS_UPLOAD_TIMEOUT = "cryostat.connections.upload-timeout"; public static final String REPORTS_SIDECAR_URL = "quarkus.rest-client.reports.url"; public static final String REPORTS_MEMORY_CACHE_ENABLED = diff --git a/src/main/java/io/cryostat/recordings/RecordingHelper.java b/src/main/java/io/cryostat/recordings/RecordingHelper.java index 36a2eb6a4..b1c5c1e3c 100644 --- a/src/main/java/io/cryostat/recordings/RecordingHelper.java +++ b/src/main/java/io/cryostat/recordings/RecordingHelper.java @@ -478,7 +478,7 @@ public Uni createSnapshot(Target target) { desc = updatedDescriptor.get(); try (InputStream snapshot = - remoteRecordingStreamFactory.open(connection, target, desc)) { + remoteRecordingStreamFactory.openDirect(connection, target, desc)) { if (!snapshotIsReadable(target, snapshot)) { safeCloseRecording(connection, desc); throw new SnapshotCreationException( @@ -1001,13 +1001,14 @@ public Pair decodedKey(String encodedKey) { public InputStream getActiveInputStream(ActiveRecording recording, Duration timeout) throws Exception { - return remoteRecordingStreamFactory.open(recording, timeout); + return getActiveInputStream(recording.target.id, recording.remoteId, timeout); } - public InputStream getActiveInputStream(long targetId, long remoteId) throws Exception { + public InputStream getActiveInputStream(long targetId, long remoteId, Duration timeout) + throws Exception { var target = Target.getTargetById(targetId); var recording = target.getRecordingById(remoteId); - var stream = remoteRecordingStreamFactory.open(recording, connectionFailedTimeout); + var stream = remoteRecordingStreamFactory.open(recording, timeout); return stream; } @@ -1327,7 +1328,7 @@ Optional getRecordingCopyPath( try { Path tempFile = fs.createTempFile(null, null); try (var stream = - remoteRecordingStreamFactory.open( + remoteRecordingStreamFactory.openDirect( connection, target, descriptor)) { fs.copy(stream, tempFile, StandardCopyOption.REPLACE_EXISTING); } diff --git a/src/main/java/io/cryostat/recordings/RemoteRecordingInputStreamFactory.java b/src/main/java/io/cryostat/recordings/RemoteRecordingInputStreamFactory.java index 35e86b1b5..f094dc582 100644 --- a/src/main/java/io/cryostat/recordings/RemoteRecordingInputStreamFactory.java +++ b/src/main/java/io/cryostat/recordings/RemoteRecordingInputStreamFactory.java @@ -35,18 +35,18 @@ public class RemoteRecordingInputStreamFactory { @Inject RecordingHelper recordingHelper; public ProgressInputStream open(ActiveRecording recording, Duration timeout) throws Exception { - return connectionManager.executeConnectedTaskTimeout( + return connectionManager.executeConnectedTask( recording.target, conn -> { IRecordingDescriptor desc = recordingHelper.getDescriptor(conn, recording).orElseThrow(); - return open(conn, recording.target, desc); + return openDirect(conn, recording.target, desc); }, timeout); } - public ProgressInputStream open(JFRConnection conn, Target target, IRecordingDescriptor desc) - throws Exception { + public ProgressInputStream openDirect( + JFRConnection conn, Target target, IRecordingDescriptor desc) throws Exception { InputStream bareStream = conn.getService().openStream(desc, false); return new ProgressInputStream( bareStream, n -> connectionManager.markConnectionInUse(target)); diff --git a/src/main/java/io/cryostat/targets/TargetConnectionManager.java b/src/main/java/io/cryostat/targets/TargetConnectionManager.java index d6fe0c009..1faf1ca39 100644 --- a/src/main/java/io/cryostat/targets/TargetConnectionManager.java +++ b/src/main/java/io/cryostat/targets/TargetConnectionManager.java @@ -204,11 +204,10 @@ public Uni executeConnectedTaskUni(Target target, ConnectedTask task) } public T executeConnectedTask(Target target, ConnectedTask task) { - return executeConnectedTaskUni(target, task).await().atMost(failedTimeout); + return executeConnectedTask(target, task, failedTimeout); } - public T executeConnectedTaskTimeout( - Target target, ConnectedTask task, Duration timeout) { + public T executeConnectedTask(Target target, ConnectedTask task, Duration timeout) { return executeConnectedTaskUni(target, task).await().atMost(timeout); } From 0e202639f798ca4166e9b8904e3289d7256395e1 Mon Sep 17 00:00:00 2001 From: Andrew Azores Date: Thu, 30 Jan 2025 21:12:09 -0500 Subject: [PATCH 5/5] use upload timeout for sidecar report generation --- src/main/java/io/cryostat/reports/ReportsServiceImpl.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/main/java/io/cryostat/reports/ReportsServiceImpl.java b/src/main/java/io/cryostat/reports/ReportsServiceImpl.java index 7de5659ec..6a0136f56 100644 --- a/src/main/java/io/cryostat/reports/ReportsServiceImpl.java +++ b/src/main/java/io/cryostat/reports/ReportsServiceImpl.java @@ -42,8 +42,8 @@ class ReportsServiceImpl implements ReportsService { private static final String NO_SIDECAR_URL = "http://localhost/"; - @ConfigProperty(name = ConfigProperties.CONNECTIONS_FAILED_TIMEOUT) - Duration connectionFailedTimeout; + @ConfigProperty(name = ConfigProperties.CONNECTIONS_UPLOAD_TIMEOUT) + Duration uploadFailedTimeout; @ConfigProperty(name = ConfigProperties.REPORTS_SIDECAR_URL) String sidecarUri; @@ -59,7 +59,7 @@ public Uni> reportFor( ActiveRecording recording, Predicate predicate) { InputStream stream; try { - stream = helper.getActiveInputStream(recording, connectionFailedTimeout); + stream = helper.getActiveInputStream(recording, uploadFailedTimeout); } catch (Exception e) { throw new ReportGenerationException(e); }