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

ensure that persisted scheduler state gets cleared when an environment is cleared #8502

Closed
sanderr opened this issue Dec 19, 2024 · 3 comments
Assignees

Comments

@sanderr
Copy link
Contributor

sanderr commented Dec 19, 2024

The assumption is that this is already the case, but it needs to be verified.

  1. scheduler table should be cleaned. Probably already achieved through cascading delete
  2. resource persisted state table should be cleaned. Probably already implemented when we first introduced it.
  3. Environment.clear and Environment.delete have some explicit ordering to prevent locking (not fully relying on cascading. This should be checked over.
@jptrindade
Copy link
Contributor

I took a brief look at this:

  1. Scheduler table was not being cleaned on the clear. But was on the delete because of the cascade
  2. ResourcePersistentState was being explicitly cleaned on the clear/delete
  3. Not sure what to check here, it appears that the order has not changed since October 2023 do we have reason to assume that there is something wrong with it?

I would say that we only need to clean the Scheduler table on the Environment.clear

@sanderr
Copy link
Contributor Author

sanderr commented Jan 6, 2025

I mostly agree. 3 is relevant both for points 1 and 2. For 2 I agree that we don't need to do anything because we have no "reason to assume that there is something wrong with it". For 1 we should probably make sure to order it so that all transactions lock the tables in the same order. So have a look at where we write to the table in inmanta.deploy.scheduler and inmanta.deploy.state, and check if we write to any other tables in the same transaction. I can do that part if you prefer.

@sanderr
Copy link
Contributor Author

sanderr commented Jan 6, 2025

A bit more context in the internal docs, mostly first paragraph is relevant here.

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

Successfully merging a pull request may close this issue.

2 participants