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

Don't delete "off" persisted_info records #39272

Merged
merged 2 commits into from Feb 29, 2024
Merged

Conversation

dpsutton
Copy link
Contributor

If they are off, they are "eligible" models for persistence that we do not want to persist. And to remember that use choice to not persist them we need to keep the record around. This was handled previously, but #39219 was a bit too blunt. We had a bunch of records in a "creating" state that we weren't removing, so i deleted them regardless of state. But we want to make sure that the "off" ones are not deleted.

On stats, a bunch of ones that were set to off are now slated to be persisted after the prune job deleted the persisted_info records.

If they are off, they are "eligible" models for persistence that we do
not want to persist. And to remember that use choice to not persist them
we need to keep the record around. This was handled previously, but
#39219 was a bit too blunt. We
had a bunch of records in a "creating" state that we weren't removing,
so i deleted them regardless of state. But we want to make sure that the
"off" ones are not deleted.

On stats, a bunch of ones that were set to off are now slated to be
persisted after the prune job deleted the persisted_info records.
Copy link

replay-io bot commented Feb 28, 2024

Status Complete ↗︎
Commit ea743e2
Results
⚠️ 5 Flaky
2329 Passed

@dpsutton dpsutton added the backport Automatically create PR on current release branch on merge label Feb 28, 2024
@dpsutton dpsutton requested a review from snoe February 29, 2024 00:32
@dpsutton dpsutton merged commit 80f90f4 into master Feb 29, 2024
109 of 121 checks passed
@dpsutton dpsutton deleted the handle-off-persisted-info branch February 29, 2024 17:44
Copy link

@dpsutton Did you forget to add a milestone to the issue for this PR? When and where should I add a milestone?

@dpsutton dpsutton added this to the 0.49 milestone Feb 29, 2024
dpsutton added a commit that referenced this pull request Feb 29, 2024
* Don't delete "off" persisted_info records

If they are off, they are "eligible" models for persistence that we do
not want to persist. And to remember that use choice to not persist them
we need to keep the record around. This was handled previously, but
#39219 was a bit too blunt. We
had a bunch of records in a "creating" state that we weren't removing,
so i deleted them regardless of state. But we want to make sure that the
"off" ones are not deleted.

On stats, a bunch of ones that were set to off are now slated to be
persisted after the prune job deleted the persisted_info records.

* unused local. whoops
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport Automatically create PR on current release branch on merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants