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

NAS-128893 / 24.10 / Remove md/swap usage #13863

Merged
merged 13 commits into from
Jun 24, 2024
Merged

NAS-128893 / 24.10 / Remove md/swap usage #13863

merged 13 commits into from
Jun 24, 2024

Conversation

aiden3c
Copy link
Contributor

@aiden3c aiden3c commented Jun 10, 2024

We're removing swap entirely from Middleware, refer to this parent Jira ticket for more information.

This PR has removed all instances of swap minus the four following areas:
10-truenas.conf sets kernel swappiness. Unsure the correct value to set here, or if the line should entirely be removed. I'm leaning remove the line.

htoprc sets swap as a statistic to include. Modifying this doesn't seem to make any change on a VM.

replace_disk.py within replace. I was going to just remove the entire section, but we use from_disk at a later point. Wanted to make sure that someone reviews if both that section and the other usage of from_disk (in the try/except/else) can be safely deleted.

boot.py within legacy_schema. I'm not entirely sure what is being done here (compared to get_boot_type alone).

@aiden3c aiden3c requested a review from a team June 10, 2024 19:00
@bugclerk bugclerk changed the title Remove md/swap usage NAS-128893 / 24.10 / Remove md/swap usage Jun 10, 2024
@bugclerk
Copy link
Contributor

Copy link
Member

@sonicaj sonicaj left a comment

Choose a reason for hiding this comment

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

Do we recall if we used to create persistent md mirrors because in that case they would still be around and we might probably still need to remove them and then format disks ?

@@ -66,8 +66,6 @@ async def replace(self, job, oid, options):
# If the disk we are replacing is still available, remove it from swap as well
swap_disks.append(from_disk)
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove this line (and swap_disks variable altogether), but keep from_disk as it is being wiped later

@themylogin
Copy link
Contributor

I'm not entirely sure what is being done here (compared to get_boot_type alone).

We've used to format boot device to use with only BIOS/EFI boot. We don't do this anymore, but we have to keep supporting formatting boot devices like this in order to be able to create mirrors for existing boot pools. Your changes are correct.

@aiden3c
Copy link
Contributor Author

aiden3c commented Jun 11, 2024

Do we recall if we used to create persistent md mirrors because in that case they would still be around and we might probably still need to remove them and then format disks ?

I found this PR, which seems to be making those persistent swap mirrors. I'm still going through it all to figure out how I'd check for old persistent mirrors and do said removing of them (if I need to).

@aiden3c aiden3c requested review from sonicaj and themylogin June 11, 2024 15:52
@sonicaj
Copy link
Member

sonicaj commented Jun 12, 2024

@aiden3c you can use the same command as in the PR to create persistent mirror and then try formatting disks to see if it works with your changes. If it does not, then that means we still need to break/destroy that mirror before actually trying to format the disks involved as that mirror would come up.

Copy link
Contributor

@themylogin themylogin left a comment

Choose a reason for hiding this comment

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

LGTM!

@aiden3c
Copy link
Contributor Author

aiden3c commented Jun 12, 2024

@aiden3c you can use the same command as in the PR to create persistent mirror

The command to create swap mirrors hasn't existed since this PR. Should I go to a commit where it exists, create the mirror, then go back to my changes?

@sonicaj
Copy link
Member

sonicaj commented Jun 12, 2024

@aiden3c so the path of least resistance might be to run it manually yourself without going to a scale version which actually had those changes. If you run into issues with that, then yes - the next best thing would be to go to a scale version where we were creating these mirrors.

Motivation behind this is that users who were on this scale version and had these mirrors setup, when they come to a version with your changes - they would still have those mirrors and format would likely fail in that case which we want to be sure doesn't happen.

@aiden3c aiden3c merged commit 2ddcad1 into master Jun 24, 2024
3 checks passed
@aiden3c aiden3c deleted the NAS-128893 branch June 24, 2024 18:01
@bugclerk
Copy link
Contributor

This PR has been merged and conversations have been locked.
If you would like to discuss more about this issue please use our forums or raise a Jira ticket.

@truenas truenas locked as resolved and limited conversation to collaborators Jun 24, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants