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

Conversation

andrewazores
Copy link
Member

@andrewazores andrewazores commented Mar 25, 2025

Welcome to Cryostat! 👋

Before contributing, make sure you have:

  • Read the contributing guidelines
  • Linked a relevant issue which this PR resolves
  • Linked any other relevant issues, PR's, or documentation, if any
  • Resolved all conflicts, if any
  • Rebased your branch PR on top of the latest upstream main branch
  • Attached at least one of the following labels to the PR: [chore, ci, docs, feat, fix, test]
  • Signed all commits using a GPG signature

To recreate commits with GPG signature git fetch upstream && git rebase --force --gpg-sign upstream/main


See #859
Related to #41
Related to #66
Related to #71

Description of the change:

Implements soft-deletion of Target records. When deleted, rather than the actual record being physically deleted from the database, a deleted flag is simply set. This is using a Hibernate feature, so other HQL queries etc. will automatically ignore these soft-deleted entities. Using native queries we can query the table and ignore the deleted flag, or even query for targets that explicitly have been deleted. This allows the data model to retain all of the information we had about what the target was, even after goes offline or is otherwise lost from discovery. This has been a sore point in the API and data model for a long time, but it's particularly relevant here because we may be getting eligible (correctly labelled, or whatever) archived recordings pushed to us from Agent instances that are in the process of shutting down. If we hard delete Targets instead of soft deleting them, then we always have a race condition between batch processing grabbing the Target record to preserve what information we have about the origin of the recording, and the Target deregistering itself or otherwise disappearing. By preserving the Target record and only soft deleting it when the Target disappears we can resolve the race condition, so that it doesn't matter when the batch process starts or finishes - we can always retrieve information about what the source Target is/was.

Current issues:

  1. Soft deletion is a significant change to the underlying data model, which should really have been done back in 3.0. Various behaviours will need to be adjusted - for example, connectUrls are no longer strictly unique (see [Epic] Redefine Target data model to allow multiple Connection URLs #71) since a soft-deleted target may have a connectUrl that a user wants to reuse, either for the same target instance or one at the same resolved network location. Maybe this change needs to be split out and worked on separately.

Motivation for the change:

This change is helpful because users may want to...

How to manually test:

  1. Run CRYOSTAT_IMAGE=quay.io... bash smoketest.bash...
  2. ...

@andrewazores
Copy link
Member Author

See #893 and https://docs.jboss.org/hibernate/orm/7.0/whats-new/whats-new.html#soft-delete-timestamp . It makes sense to use the soft deletion timestamp feature when this becomes available in Quarkus/Panache. It may be worth progressing this WIP PR using the existing boolean deleted data flag anyway, but the timestamp feature coming down the pipe is much better and it may be worth holding off on merging this until that can also be incorporated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat New feature or request safe-to-test
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant