Skip to content

feat(target): implement soft deletion #860

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

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft
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
3 changes: 1 addition & 2 deletions src/main/java/io/cryostat/discovery/ContainerDiscovery.java
Original file line number Diff line number Diff line change
Expand Up @@ -259,9 +259,8 @@ private Target toTarget(ContainerSpec desc) {
return null;
}

Target target = new Target();
Target target = Target.createOrUndelete(connectUrl);
target.activeRecordings = new ArrayList<>();
target.connectUrl = connectUrl;
target.alias = Optional.ofNullable(desc.Names.get(0)).orElse(desc.Id);
target.labels = desc.Labels;
target.annotations =
Expand Down
3 changes: 1 addition & 2 deletions src/main/java/io/cryostat/discovery/CustomDiscovery.java
Original file line number Diff line number Diff line change
Expand Up @@ -115,8 +115,7 @@ public RestResponse<Target> createForm(
@RestForm String password,
@RestQuery boolean dryrun,
@RestQuery boolean storeCredentials) {
var target = new Target();
target.connectUrl = connectUrl;
var target = Target.createOrUndelete(connectUrl);
target.alias = alias;

Credential credential = null;
Expand Down
3 changes: 1 addition & 2 deletions src/main/java/io/cryostat/discovery/JDPDiscovery.java
Original file line number Diff line number Diff line change
Expand Up @@ -113,9 +113,8 @@ void handleJdpEvent(JvmDiscoveryEvent evt) {

switch (evt.getEventKind()) {
case FOUND:
Target target = new Target();
Target target = Target.createOrUndelete(connectUrl);
target.activeRecordings = new ArrayList<>();
target.connectUrl = connectUrl;
target.alias = evt.getJvmDescriptor().getMainClass();
target.annotations =
new Annotations(
Expand Down
3 changes: 1 addition & 2 deletions src/main/java/io/cryostat/discovery/KubeApiDiscovery.java
Original file line number Diff line number Diff line change
Expand Up @@ -616,9 +616,8 @@ public Target toTarget() {
"/jndi/rmi://" + host + ':' + port.getPort() + "/jmxrmi");
URI connectUrl = URI.create(jmxUrl.toString());

Target target = new Target();
Target target = Target.createOrUndelete(connectUrl);
target.activeRecordings = new ArrayList<>();
target.connectUrl = connectUrl;
target.alias = objRef.getName();
target.labels = (obj != null ? obj.getMetadata().getLabels() : new HashMap<>());
target.annotations =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -302,7 +302,7 @@ public List<ProgramOption> getProgramOptions() {
}

private String[] getJfrEventTypeIds(SimplifiedTarget st) {
Target target = Target.find("id", st.id()).singleResult();
var target = Target.getTargetById(st.id());
try {
return connectionManager.executeConnectedTask(
target,
Expand Down
92 changes: 77 additions & 15 deletions src/main/java/io/cryostat/targets/Target.java
Original file line number Diff line number Diff line change
Expand Up @@ -33,15 +33,18 @@

import io.cryostat.discovery.DiscoveryNode;
import io.cryostat.recordings.ActiveRecording;
import io.cryostat.recordings.RecordingHelper;
import io.cryostat.util.URIUtil;
import io.cryostat.ws.MessagingServer;
import io.cryostat.ws.Notification;

import com.fasterxml.jackson.annotation.JsonIgnore;
import com.fasterxml.jackson.annotation.JsonProperty;
import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
import io.quarkus.hibernate.orm.panache.Panache;
import io.quarkus.hibernate.orm.panache.PanacheEntity;
import io.vertx.mutiny.core.eventbus.EventBus;
import jakarta.annotation.Nullable;
import jakarta.enterprise.context.ApplicationScoped;
import jakarta.inject.Inject;
import jakarta.persistence.CascadeType;
Expand All @@ -63,17 +66,21 @@
import jakarta.validation.constraints.NotNull;
import org.apache.commons.lang3.StringUtils;
import org.hibernate.annotations.JdbcTypeCode;
import org.hibernate.annotations.SoftDelete;
import org.hibernate.type.SqlTypes;
import org.jboss.logging.Logger;

@Entity
@EntityListeners(Target.Listener.class)
@NamedQueries({@NamedQuery(name = "Target.unconnected", query = "from Target where jvmId is null")})
@Table(
indexes = {
@Index(columnList = "jvmId"),
@Index(columnList = "connectUrl"),
})
@NamedQueries({
@NamedQuery(name = "Target.unconnected", query = "from Target where jvmId is null"),
})
@SoftDelete
public class Target extends PanacheEntity {

public static final String TARGET_JVM_DISCOVERY = "TargetJvmDiscovery";
Expand All @@ -84,7 +91,7 @@ public class Target extends PanacheEntity {

@NotBlank public String alias;

public String jvmId;
@Nullable public String jvmId;

@JdbcTypeCode(SqlTypes.JSON)
@NotNull
Expand Down Expand Up @@ -120,31 +127,84 @@ public String targetId() {
return this.connectUrl.toString();
}

public static Target createOrUndelete(URI connectUrl) {
Objects.requireNonNull(connectUrl);
var target =
Panache.getSession()
// ignore soft deletion field
.createNativeQuery(
"select * from Target where connectUrl = :connectUrl", Target.class)
.setParameter("connectUrl", connectUrl.toString().getBytes())
.uniqueResult();
if (target == null) {
target = new Target();
target.connectUrl = connectUrl;
} else {
int updates =
Panache.getSession()
.createNativeQuery(
"update target set deleted = false where id = :id",
Target.class)
.setParameter("id", target.id)
.executeUpdate();
if (updates != 1) {
Logger.getLogger(Target.class)
.warnv(
"Attempted to undelete Target {0} with connectUrl={1}, but update"
+ " affected {2} rows",
target.id, connectUrl, updates);
}
}
return target;
}

public static List<Target> getTargetsIncludingDeleted() {
return Panache.getSession()
// ignore soft deletion field
.createNativeQuery("select * from Target", Target.class)
.getResultList();
}

public static Target getTargetById(long targetId) {
return Target.find("id", targetId).singleResult();
return getTargetById(targetId, false);
}

public static Target getTargetById(long targetId, boolean includeDeleted) {
if (includeDeleted) {
return Panache.getSession()
.createNativeQuery("select * from Target where id = :id", Target.class)
.setParameter("id", targetId)
.getSingleResult();
} else {
return Target.find("id", targetId).singleResult();
}
}

public static Target getTargetByConnectUrl(URI connectUrl) {
return find("connectUrl", connectUrl).singleResult();
return Panache.getSession()
// ignore soft deletion field
.createNativeQuery(
"select * from Target where connectUrl = :connectUrl", Target.class)
.setParameter("connectUrl", connectUrl.toString().getBytes())
.getSingleResult();
}

public static Optional<Target> getTargetByJvmId(String jvmId) {
return find("jvmId", jvmId).firstResultOptional();
return Panache.getSession()
// ignore soft deletion field
.createNativeQuery("select * from Target where jvmId = :jvmId", Target.class)
.setParameter("jvmId", jvmId.getBytes())
.uniqueResultOptional();
}

public static Optional<Target> getTarget(Predicate<Target> predicate) {
List<Target> targets = listAll();
return targets.stream().filter(predicate).findFirst();
}

public static boolean deleteByConnectUrl(URI connectUrl) {
return delete("connectUrl", connectUrl) > 0;
return Target.<Target>findAll().stream().filter(predicate).findFirst();
}

public static List<Target> findByRealm(String realm) {
List<Target> targets = findAll().list();

return targets.stream()
// TODO reimplement this to work by grabbing the relevant Realm discovery node, then
// traversing the tree to find Target leaves
return Target.<Target>listAll().stream()
.filter((t) -> realm.equals(t.annotations.cryostat().get("REALM")))
.collect(Collectors.toList());
}
Expand Down Expand Up @@ -288,8 +348,9 @@ public record TargetDiscovery(EventKind kind, Target serviceRef, String jvmId) {
static class Listener {

@Inject URIUtil uriUtil;
@Inject Logger logger;
@Inject RecordingHelper recordingHelper;
@Inject EventBus bus;
@Inject Logger logger;

@PrePersist
void prePersist(Target target) {
Expand Down Expand Up @@ -335,6 +396,7 @@ void postUpdate(Target target) {
@PostRemove
void postRemove(Target target) {
notify(EventKind.LOST, target);
target.activeRecordings.forEach(recordingHelper::deleteRecording);
}

private void notify(EventKind eventKind, Target target) {
Expand Down
6 changes: 5 additions & 1 deletion src/main/java/io/cryostat/targets/TargetUpdateJob.java
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,11 @@ public void execute(JobExecutionContext context) throws JobExecutionException {
List<Target> targets;
Long targetId = (Long) context.getJobDetail().getJobDataMap().get("targetId");
if (targetId != null) {
targets = List.of(Target.getTargetById(targetId));
Target target = Target.findById(targetId);
if (target == null) {
return;
}
targets = List.of();
} else {
targets = Target.<Target>find("#Target.unconnected").list();
}
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/io/cryostat/targets/Targets.java
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,6 @@ public List<Target> list() {
@Path("/api/v4/targets/{id}")
@RolesAllowed("read")
public Target getById(@RestPath Long id) {
return Target.find("id", id).singleResult();
return Target.getTargetById(id);
}
}
8 changes: 8 additions & 0 deletions src/main/resources/db/migration/V4.1.0__cryostat.sql
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
alter sequence DiscoveryNode_SEQ increment by 50;

alter table Target
add column deleted boolean default false;
create index on Target (jvmId);
create index on Target (connectUrl);

Expand All @@ -8,3 +10,9 @@ create index on DiscoveryNode (nodeType, name);

create index on Rule (name);
alter table Rule add column metadata jsonb default '{"labels":{}}';

delete from DiscoveryNode where nodeType not in ('Universe', 'Realm');
delete from Target where true;

alter table Target drop constraint FKl0dhd7qeayg54dcoblpww6x34;
alter table Target drop constraint target_connecturl_key;
4 changes: 2 additions & 2 deletions src/test/java/itest/TargetRecordingPatchTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
*/
package itest;

import java.util.List;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.ExecutorService;
Expand All @@ -34,7 +35,6 @@
import itest.util.ITestCleanupFailedException;
import org.hamcrest.MatcherAssert;
import org.hamcrest.Matchers;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;

@QuarkusTest
Expand Down Expand Up @@ -121,7 +121,7 @@ void testSaveEmptyRecordingDoesNotArchiveRecordingFile() throws Exception {
}
});
JsonArray listResp = listRespFuture1.get(REQUEST_TIMEOUT_SECONDS, TimeUnit.SECONDS);
Assertions.assertTrue(listResp.isEmpty());
MatcherAssert.assertThat(listResp.getList(), Matchers.equalTo(List.of()));

} finally {
// Clean up recording
Expand Down
Loading