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.

Sign up for GitHub

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(graphql): avoid ConcurrentModificationException #433

Merged
merged 2 commits into from
May 4, 2024
Merged

fix(graphql): avoid ConcurrentModificationException #433

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
45a5e28
fix(recording): fix NPE when starting a recording without specified t…
andrewazores May 3, 2024
897c3c5
fix(graphql): avoid ConcurrentModificationException
andrewazores May 3, 2024
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
229 changes: 112 additions & 117 deletions src/main/java/io/cryostat/graphql/ActiveRecordings.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@
import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
import io.smallrye.common.annotation.Blocking;
import io.smallrye.graphql.api.Nullable;
import io.smallrye.graphql.execution.ExecutionException;
import jakarta.inject.Inject;
import jakarta.transaction.Transactional;
import jdk.jfr.RecordingState;
Expand Down Expand Up @@ -72,39 +71,39 @@ public class ActiveRecordings {
"Start a new Flight Recording on all Targets under the subtrees of the discovery nodes"
+ " matching the given filter")
public List<ActiveRecording> createRecording(
@NonNull DiscoveryNodeFilter nodes, @NonNull RecordingSettings recording) {
return DiscoveryNode.<DiscoveryNode>listAll().stream()
.filter(nodes)
.flatMap(
node ->
RootNode.recurseChildren(node, n -> n.target != null).stream()
.map(n -> n.target))
.map(
target -> {
var template =
recordingHelper.getPreferredTemplate(
target,
recording.template,
TemplateType.valueOf(recording.templateType));
try {
return recordingHelper
.startRecording(
target,
Optional.ofNullable(recording.replace)
.map(RecordingReplace::valueOf)
.orElse(RecordingReplace.STOPPED),
template,
recording.asOptions(),
Optional.ofNullable(recording.metadata)
.map(s -> s.labels)
.orElse(Map.of()))
.await()
.atMost(Duration.ofSeconds(10));
} catch (QuantityConversionException qce) {
throw new ExecutionException(qce);
}
})
.toList();
@NonNull DiscoveryNodeFilter nodes, @NonNull RecordingSettings recording)
throws QuantityConversionException {
var list =
DiscoveryNode.<DiscoveryNode>listAll().stream()
.filter(nodes)
.flatMap(
node ->
RootNode.recurseChildren(node, n -> n.target != null)
.stream()
.map(n -> n.target))
.toList();
var recordings = new ArrayList<ActiveRecording>();
for (var t : list) {
var template =
recordingHelper.getPreferredTemplate(
t, recording.template, TemplateType.valueOf(recording.templateType));
var r =
recordingHelper
.startRecording(
t,
Optional.ofNullable(recording.replace)
.map(RecordingReplace::valueOf)
.orElse(RecordingReplace.STOPPED),
template,
recording.asOptions(),
Optional.ofNullable(recording.metadata)
.map(s -> s.labels)
.orElse(Map.of()))
.await()
.atMost(Duration.ofSeconds(10));
recordings.add(r);
}
return recordings;
}

@Blocking
Expand All @@ -114,26 +113,29 @@ public List<ActiveRecording> createRecording(
"Archive an existing Flight Recording matching the given filter, on all Targets under"
+ " the subtrees of the discovery nodes matching the given filter")
public List<ArchivedRecording> archiveRecording(
@NonNull DiscoveryNodeFilter nodes, @Nullable ActiveRecordingsFilter recordings) {
return DiscoveryNode.<DiscoveryNode>listAll().stream()
.filter(nodes)
.flatMap(
node ->
RootNode.recurseChildren(node, n -> n.target != null).stream()
.map(n -> n.target))
.flatMap(
t ->
recordingHelper.listActiveRecordings(t).stream()
.filter(r -> recordings == null || recordings.test(r)))
.map(
recording -> {
try {
return recordingHelper.archiveRecording(recording, null, null);
} catch (Exception e) {
throw new ExecutionException(e);
}
})
.toList();
@NonNull DiscoveryNodeFilter nodes, @Nullable ActiveRecordingsFilter recordings)
throws Exception {
var list =
DiscoveryNode.<DiscoveryNode>listAll().stream()
.filter(nodes)
.flatMap(
node ->
RootNode.recurseChildren(node, n -> n.target != null)
.stream()
.map(n -> n.target))
.flatMap(
t ->
recordingHelper.listActiveRecordings(t).stream()
.filter(
r ->
recordings == null
|| recordings.test(r)))
.toList();
var archives = new ArrayList<ArchivedRecording>();
for (var r : list) {
archives.add(recordingHelper.archiveRecording(r, null, null));
}
return archives;
}

@Blocking
Expand All @@ -143,29 +145,28 @@ public List<ArchivedRecording> archiveRecording(
"Stop an existing Flight Recording matching the given filter, on all Targets under"
+ " the subtrees of the discovery nodes matching the given filter")
public List<ActiveRecording> stopRecording(
@NonNull DiscoveryNodeFilter nodes, @Nullable ActiveRecordingsFilter recordings) {
return DiscoveryNode.<DiscoveryNode>listAll().stream()
.filter(nodes)
.flatMap(
node ->
RootNode.recurseChildren(node, n -> n.target != null).stream()
.map(n -> n.target))
.flatMap(
t ->
recordingHelper.listActiveRecordings(t).stream()
.filter(r -> recordings == null || recordings.test(r)))
.map(
recording -> {
try {
return recordingHelper
.stopRecording(recording)
.await()
.atMost(Duration.ofSeconds(10));
} catch (Exception e) {
throw new ExecutionException(e);
}
})
.toList();
@NonNull DiscoveryNodeFilter nodes, @Nullable ActiveRecordingsFilter recordings)
throws Exception {
var list =
DiscoveryNode.<DiscoveryNode>listAll().stream()
.filter(nodes)
.flatMap(
node ->
RootNode.recurseChildren(node, n -> n.target != null)
.stream()
.map(n -> n.target))
.flatMap(
t ->
recordingHelper.listActiveRecordings(t).stream()
.filter(
r ->
recordings == null
|| recordings.test(r)))
.toList();
for (var r : list) {
recordingHelper.stopRecording(r).await().atMost(Duration.ofSeconds(10));
}
return list;
}

@Blocking
Expand All @@ -176,28 +177,26 @@ public List<ActiveRecording> stopRecording(
+ " the subtrees of the discovery nodes matching the given filter")
public List<ActiveRecording> deleteRecording(
@NonNull DiscoveryNodeFilter nodes, @Nullable ActiveRecordingsFilter recordings) {
return DiscoveryNode.<DiscoveryNode>listAll().stream()
.filter(nodes)
.flatMap(
node ->
RootNode.recurseChildren(node, n -> n.target != null).stream()
.map(n -> n.target))
.flatMap(
t ->
recordingHelper.listActiveRecordings(t).stream()
.filter(r -> recordings == null || recordings.test(r)))
.map(
recording -> {
try {
return recordingHelper
.deleteRecording(recording)
.await()
.atMost(Duration.ofSeconds(10));
} catch (Exception e) {
throw new ExecutionException(e);
}
})
.toList();
var list =
DiscoveryNode.<DiscoveryNode>listAll().stream()
.filter(nodes)
.flatMap(
node ->
RootNode.recurseChildren(node, n -> n.target != null)
.stream()
.map(n -> n.target))
.flatMap(
t ->
recordingHelper.listActiveRecordings(t).stream()
.filter(
r ->
recordings == null
|| recordings.test(r)))
.toList();
for (var r : list) {
recordingHelper.deleteRecording(r).await().atMost(Duration.ofSeconds(10));
}
return list;
}

@Blocking
Expand All @@ -207,24 +206,20 @@ public List<ActiveRecording> deleteRecording(
"Create a Flight Recorder Snapshot on all Targets under"
+ " the subtrees of the discovery nodes matching the given filter")
public List<ActiveRecording> createSnapshot(@NonNull DiscoveryNodeFilter nodes) {
return DiscoveryNode.<DiscoveryNode>listAll().stream()
.filter(nodes)
.flatMap(
node ->
RootNode.recurseChildren(node, n -> n.target != null).stream()
.map(n -> n.target))
.map(
target -> {
try {
return recordingHelper
.createSnapshot(target)
.await()
.atMost(Duration.ofSeconds(10));
} catch (Exception e) {
throw new ExecutionException(e);
}
})
.toList();
var targets =
DiscoveryNode.<DiscoveryNode>listAll().stream()
.filter(nodes)
.flatMap(
node ->
RootNode.recurseChildren(node, n -> n.target != null)
.stream()
.map(n -> n.target))
.toList();
var snapshots = new ArrayList<ActiveRecording>();
for (var t : targets) {
snapshots.add(recordingHelper.createSnapshot(t).await().atMost(Duration.ofSeconds(10)));
}
return snapshots;
}

@Blocking
Expand Down
10 changes: 10 additions & 0 deletions src/main/java/io/cryostat/recordings/RecordingHelper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -597,6 +597,16 @@ public Template getPreferredTemplate(
return Optional.empty();
}
};
if (templateType == null) {
return custom.get()
.or(() -> remote.get())
.orElseThrow(
() ->
new BadRequestException(
String.format(
"Invalid/unknown event template %s",
templateName)));
}
switch (templateType) {
case TARGET:
return remote.get().orElseThrow();
Expand Down
Loading