fix(recordings): use safe recording close on cleanup (#763)

Author: andrewazores

compose/auth_proxy.yml

@@ -9,8 +9,6 @@ services:
       QUARKUS_HTTP_PROXY_ALLOW_X_FORWARDED: "true"
       QUARKUS_HTTP_PROXY_ENABLE_FORWARDED_HOST: "true"
       QUARKUS_HTTP_PROXY_ENABLE_FORWARDED_PREFIX: "true"
-      QUARKUS_HTTP_ACCESS_LOG_PATTERN: long
-      QUARKUS_HTTP_ACCESS_LOG_ENABLED: "true"
   auth:
     # the proxy does not actually depend on cryostat being up, but we use this
     # to ensure that when the smoketest tries to open the auth login page in a

compose/cryostat.yml

@@ -15,7 +15,9 @@ services:
     hostname: cryostat
     user: "1000"
     environment:
-      QUARKUS_LOG_LEVEL: ALL
+      QUARKUS_LOG_LEVEL: ${CRYOSTAT_LOG_LEVEL:-INFO}
+      QUARKUS_HTTP_ACCESS_LOG_ENABLED: "true"
+      QUARKUS_HTTP_ACCESS_LOG_PATTERN: long
       QUARKUS_HTTP_HOST: "cryostat"
       QUARKUS_HTTP_PORT: ${CRYOSTAT_HTTP_PORT}
       QUARKUS_HIBERNATE_ORM_LOG_SQL: "true"

smoketest.bash

@@ -19,6 +19,7 @@ OPEN_TABS=${OPEN_TABS:-false}
 
 PRECREATE_BUCKETS=${PRECREATE_BUCKETS:-archivedrecordings,archivedreports,eventtemplates,probes}
 
+LOG_LEVEL=0
 CRYOSTAT_HTTP_HOST=${CRYOSTAT_HTTP_HOST:-cryostat}
 CRYOSTAT_HTTP_PORT=${CRYOSTAT_HTTP_PORT:-8080}
 USE_PROXY=${USE_PROXY:-true}
@@ -44,11 +45,12 @@ display_usage() {
     echo -e "\t-b\t\t\t\t\t\topen a Browser tab for each running service's first mapped port (ex. auth proxy login, database viewer)"
     echo -e "\t-n\t\t\t\t\t\tdo Not apply configuration changes, instead emit the compose YAML that would have been used to stdout."
     echo -e "\t-k\t\t\t\t\t\tdisable TLS on the auth proxy."
+    echo -e "\t-v\t\t\t\t\t\tenable verbose logging. Can be passed multiple times to increase verbosity."
 }
 
 s3=seaweed
 container_engine="$(command -v podman)"
-while getopts "hs:prGtAOVXc:bnk" opt; do
+while getopts "hs:prGtAOVXc:bnkv" opt; do
     case $opt in
         h)
             display_usage
@@ -97,6 +99,9 @@ while getopts "hs:prGtAOVXc:bnk" opt; do
         O)
             PULL_IMAGES=false
             ;;
+        v)
+            LOG_LEVEL=$((LOG_LEVEL+1))
+            ;;
         V)
             KEEP_VOLUMES=true
             DATABASE_GENERATION=update
@@ -168,6 +173,16 @@ else
     fi
     GRAFANA_DASHBOARD_EXT_URL=http://grafana:3000/
 fi
+if [ $LOG_LEVEL = 0 ]; then
+    CRYOSTAT_LOG_LEVEL=INFO
+elif [ $LOG_LEVEL = 1 ]; then
+    CRYOSTAT_LOG_LEVEL=DEBUG
+elif [ $LOG_LEVEL = 2 ]; then
+    CRYOSTAT_LOG_LEVEL=TRACE
+else
+    CRYOSTAT_LOG_LEVEL=ALL
+fi
+export CRYOSTAT_LOG_LEVEL
 export CRYOSTAT_HTTP_HOST
 export CRYOSTAT_HTTP_PORT
 export GRAFANA_DASHBOARD_EXT_URL

src/main/java/io/cryostat/recordings/RecordingHelper.java

@@ -136,6 +136,9 @@
 @ApplicationScoped
 public class RecordingHelper {
 
+    private static final int S3_API_PART_LIMIT = 10_000;
+    private static final int MIB = 1024 * 1024;
+
     public static final String JFR_MIME = HttpMimeType.JFR.mime();
 
     private static final Pattern TEMPLATE_PATTERN =
@@ -473,7 +476,7 @@ public Uni<ActiveRecording> createSnapshot(Target target) {
                     try (InputStream snapshot =
                             remoteRecordingStreamFactory.open(connection, target, desc)) {
                         if (!snapshotIsReadable(target, snapshot)) {
-                            connection.getService().close(desc);
+                            safeCloseRecording(connection, desc);
                             throw new SnapshotCreationException(
                                     "Snapshot was not readable - are there any source recordings?");
                         }
@@ -570,24 +573,20 @@ public Uni<ActiveRecording> stopRecording(ActiveRecording recording) throws Exce
     }
 
     public Uni<ActiveRecording> deleteRecording(ActiveRecording recording) {
-        var closed =
-                connectionManager.executeConnectedTask(
-                        recording.target,
-                        conn -> {
-                            var desc = getDescriptorById(conn, recording.remoteId);
-                            if (desc.isEmpty()) {
-                                throw new NotFoundException();
-                            }
-                            conn.getService().close(desc.get());
-                            return recording;
-                        });
+        connectionManager.executeConnectedTask(
+                recording.target,
+                conn -> {
+                    getDescriptorById(conn, recording.remoteId)
+                            .ifPresent(d -> safeCloseRecording(conn, d));
+                    return null;
+                });
         return QuarkusTransaction.joiningExisting()
                 .call(
                         () -> {
-                            closed.target.activeRecordings.remove(recording);
-                            closed.target.persist();
-                            closed.delete();
-                            return Uni.createFrom().item(closed);
+                            recording.target.activeRecordings.remove(recording);
+                            recording.target.persist();
+                            recording.delete();
+                            return Uni.createFrom().item(recording);
                         });
     }
 
@@ -818,14 +817,13 @@ public ArchivedRecording archiveRecording(
         if (StringUtils.isBlank(savename)) {
             savename = filename;
         }
-        int mib = 1024 * 1024;
         String key = archivedRecordingKey(recording.target.jvmId, filename);
         String multipartId = null;
         List<Pair<Integer, String>> parts = new ArrayList<>();
         long accum = 0;
         try (var stream = getActiveInputStream(recording);
                 var ch = Channels.newChannel(stream)) {
-            ByteBuffer buf = ByteBuffer.allocate(20 * mib);
+            ByteBuffer buf = ByteBuffer.allocate(20 * MIB);
             CreateMultipartUploadRequest.Builder builder =
                     CreateMultipartUploadRequest.builder()
                             .bucket(archiveBucket)
@@ -840,7 +838,7 @@ public ArchivedRecording archiveRecording(
             CreateMultipartUploadRequest request = builder.build();
             multipartId = storage.createMultipartUpload(request).uploadId();
             int read = 0;
-            for (int i = 1; i <= 10_000; i++) {
+            for (int i = 1; i <= S3_API_PART_LIMIT; i++) {
                 read = ch.read(buf);
 
                 if (read == 0) {
@@ -868,7 +866,7 @@ public ArchivedRecording archiveRecording(
                 parts.add(Pair.of(i, eTag));
                 buf.clear();
                 // S3 API limit
-                if (i == 10_000) {
+                if (i == S3_API_PART_LIMIT) {
                     throw new IndexOutOfBoundsException("Exceeded S3 maximum part count");
                 }
             }