Skip to content

Remove sled_resource_vmm records without a VMM record#7898

Merged
smklein merged 2 commits intomainfrom
delete-defunct-reservations
Apr 1, 2025
Merged

Remove sled_resource_vmm records without a VMM record#7898
smklein merged 2 commits intomainfrom
delete-defunct-reservations

Conversation

@smklein
Copy link
Copy Markdown
Collaborator

@smklein smklein commented Apr 1, 2025

Fixes #7896

@smklein smklein marked this pull request as ready for review April 1, 2025 03:28
Copy link
Copy Markdown
Member

@hawkw hawkw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems right to me, after reading through the discussion between you and @gjcolombo on #7896.

I did wonder briefly if perhaps we would want to do this periodically in case any erroneous records were accidentally created somehow, but I don't really think it's worth the effort, given that the current schema code really shouldn't ever do that.

@smklein
Copy link
Copy Markdown
Collaborator Author

smklein commented Apr 1, 2025

This seems right to me, after reading through the discussion between you and @gjcolombo on #7896.

I did wonder briefly if perhaps we would want to do this periodically in case any erroneous records were accidentally created somehow, but I don't really think it's worth the effort, given that the current schema code really shouldn't ever do that.

I considered making this a background task, but the idea of doing this deletion seems like "one more interaction" with this table that I'd rather not have (e.g., could it cause issues on creation? deletion? etc). By making it a schema change, it's one-time, it can be tracked, and it happens when the system is in a quiesced state (for now).

@smklein smklein enabled auto-merge (squash) April 1, 2025 05:46
@smklein smklein merged commit cb3090b into main Apr 1, 2025
16 checks passed
@smklein smklein deleted the delete-defunct-reservations branch April 1, 2025 06:45
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 this pull request may close these issues.

Potential sled compute resource accounting issue caused by NULL instance_id entries

2 participants