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

[Feature Anywhere] Cleanup of dangling augment-vis saved objs #4044

Closed
ohltyler opened this issue May 16, 2023 · 7 comments
Closed

[Feature Anywhere] Cleanup of dangling augment-vis saved objs #4044

ohltyler opened this issue May 16, 2023 · 7 comments

Comments

@ohltyler
Copy link
Member

ohltyler commented May 16, 2023

There is two different scenarios where an augment-vis saved object can become stale:

  1. visualization is deleted
  2. plugin resource is deleted

To determine when a visualization is deleted: when making a call to fetch the saved object, will return an error.
To determine when a plugin resource is deleted: when make a get call to the plugin resource, will return an error (separate than some permissions error).

Although these resources could be deleted in an event-driven way (e.g., deleting all saved objs when deleting the plugin resource, deleting all saved objs when deleting the visualization), this can lead to a few problems:

  1. Can lead to unexpected expensive calls when performing the deletions, if there is a lot of saved objs associated to some particular visualization and/or plugin resource
  2. Would need to inject logic in many places where these deletions happen on the frontend (both in OSD + plugin UIs)
  3. Would miss deletions made via backend-only (direct api calls)

Proposed solution is to have some async cron job running periodically in the background that handles deletion of dangling saved objs. It can iterate through all of them, make get calls to both the visualization and the associated plugin resource, and if either of them return errors such that the resource has been deleted, then we can delete the saved obj.

@ohltyler
Copy link
Member Author

A problem with the above approach is if we don't clean up fast enough, this can lead to false limits being reached. For example, users try to add an association but reach some limit due to the existing saved obj count, when that count could possibly be including dangling/useless saved objs.

@lezzago
Copy link
Member

lezzago commented May 16, 2023

Also when we do the create association call, that is where we should have the cleanup call to make sure that we have not reached the max associated objects to a viz limit

@ohltyler
Copy link
Member Author

What do you think of this?

  1. On create augment-vis saved obj, check that for the particular vis, all associated plugin resources are not deleted. If any, clean them up
  2. On delete visualization saved obj, fetch all augment-vis saved objs that have that vis ID, and delete them too

I think both of these could be done in the respective saved obj loaders. But again it doesn't solve the problem of if these deletions are done via api directly where the logic would be skipped...

@ohltyler
Copy link
Member Author

a third scenario could be cron-job-style deletion by checking for any other dangling saved objs that could have been missed due to some failure, or if deleted via api like mentioned above, etc.

@lezzago
Copy link
Member

lezzago commented May 16, 2023

What do you think of this?

1. On create `augment-vis` saved obj, check that for the particular vis, all associated plugin resources are not deleted. If any, clean them up

2. On delete `visualization` saved obj, fetch all `augment-vis` saved objs that have that vis ID, and delete them too

I think both of these could be done in the respective saved obj loaders. But again it doesn't solve the problem of if these deletions are done via api directly where the logic would be skipped...

I believe this approach is better than the cron job as its not adding extra load on the cluster. Having some dangling objects there until create/delete calls are made should be fine since these dangling objects would only affect the user if they are not cleaned up at creation time as it would indicate that the user reached the limit of associated plugin resources to the visualization.

@ohltyler
Copy link
Member Author

After some more thought, we can go ahead with the following approach:

  1. To clean up deleted plugin resources: when rendering the vis & fetching all of its associations, for any VisLayer that returns an error indicating a deleted resource, we can delete the saved obj asynchronously. This way, the eligibility will always be up-to-date such that users will only be able to create new associations on the UI if the saved obj limit has not been reached (stale objs could impact this limit otherwise)
  2. To clean up deleted visualizations: when deleting a visualization, fetch any augment-vis saved objs that have this vis id and delete it. We can embed this within the saved obj loader, which may follow a similar pattern to making saved obj api calls within another one as seen in the UI settings PR: Add new advanced settings for vis augmenter #3961

Limitations:

  • operations made unexpectedly (deleting visualizations via api calls, creating augment-vis objs manually or via api, etc.) could still cause stale augment-vis objs to remain
  • potential unexpected load if there is a lot of stale saved objs that need to be cleaned during a particular operation (when rendering or deleting a vis)

@ohltyler
Copy link
Member Author

Closed via #4059

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants