Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 5 commits into from
Jan 10, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions compose/auth_proxy.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 3 additions & 1 deletion compose/cryostat.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
17 changes: 16 additions & 1 deletion smoketest.bash
Original file line number Diff line number Diff line change
Expand Up @@ -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}
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
38 changes: 18 additions & 20 deletions src/main/java/io/cryostat/recordings/RecordingHelper.java
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand Down Expand Up @@ -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?");
}
Expand Down Expand Up @@ -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);
});
}

Expand Down Expand Up @@ -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)
Expand All @@ -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) {
Expand Down Expand Up @@ -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");
}
}
Expand Down
Loading