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

Revert "pre-allocate swapfile while building image" #493

Merged
merged 1 commit into from
Dec 20, 2023

Conversation

syuu1228
Copy link
Contributor

This reverts commit 069318b.

Related with #491, we need to reduce snapshot size of the rootfs, we should not pre-allocate swapfile while building image.

@syuu1228
Copy link
Contributor Author

I'm still not sure what is the best approach for the swapfile (there are discussions on #491 and #492), but it's clear we do not want to pre-allocate swap since we want to reduce snapshot size of the rootfs, so let's just revert the commit first.

@syuu1228 syuu1228 force-pushed the revert_preallocate_swap branch 2 times, most recently from 95a88fc to 7571f62 Compare December 11, 2023 07:20
@yaronkaikov
Copy link
Collaborator

I'm still not sure what is the best approach for the swapfile (there are discussions on #491 and #492), but it's clear we do not want to pre-allocate swap since we want to reduce snapshot size of the rootfs, so let's just revert the commit first.

@syuu1228 What will happen to our image if we revert this? i assume we will not have swap . right?

@yaronkaikov
Copy link
Collaborator

@syuu1228 If we merge this, we will have no swap at all. We should at least create the swap during scylla_image_setup

@syuu1228
Copy link
Contributor Author

@syuu1228 If we merge this, we will have no swap at all. We should at least create the swap during scylla_image_setup

@yaronkaikov No, it will allocate swapfile during scylla_image_setup.
We already have Azure specific code to allocate swapfile on /mnt/swapfile, this patch allows to allocate swapfile on /swapfile for AWS/GCE.

See: https://github.com/scylladb/scylla-machine-image/pull/493/files#diff-e8d220a78f08409142c99b415ca08f58748b3c948af3c3dbfa827dbf20ab020bR14

@yaronkaikov
Copy link
Collaborator

@syuu1228 If we merge this, we will have no swap at all. We should at least create the swap during scylla_image_setup

@yaronkaikov No, it will allocate swapfile during scylla_image_setup. We already have Azure specific code to allocate swapfile on /mnt/swapfile, this patch allows to allocate swapfile on /swapfile for AWS/GCE.

See: https://github.com/scylladb/scylla-machine-image/pull/493/files#diff-e8d220a78f08409142c99b415ca08f58748b3c948af3c3dbfa827dbf20ab020bR14

@syuu1228 Please verify it's working. I am using us-east-1-x86_64 = ami-0ecd0f56a75ebd7f7 image which was build based on your branch. I have set an instance based on this image, and i don't see swap, can you please verify it?

@syuu1228
Copy link
Contributor Author

@syuu1228 Please verify it's working. I am using us-east-1-x86_64 = ami-0ecd0f56a75ebd7f7 image which was build based on your branch. I have set an instance based on this image, and i don't see swap, can you please verify it?

I just checked us-east-1-x86_64 = ami-0ecd0f56a75ebd7f7, scylla_image_setup on this image is not applied this patch for some reason.

If the patch applied it should be like this:

if __name__ == '__main__':
    if is_azure():
        swap_directory = Path('/mnt')
        swap_unit = Path('/etc/systemd/system/mnt-swapfile.swap')
    else:
        swap_directory = Path('/')
        swap_unit = Path('/etc/systemd/system/swapfile.swap')
    swapfile = swap_directory / 'swapfile'
    if not swapfile.exists():
        swap_unit.unlink(missing_ok=True)
        run(f'/opt/scylladb/scripts/scylla_swap_setup --swap-directory {swap_directory}', shell=True, check=True)

But scylla_image_setup on this image seems like not applied the patch:

$ sudo head -n20 /opt/scylladb/scylla-machine-image/libexec/scylla_image_setup 
#!/usr/bin/env python3
#
# Copyright 2020 ScyllaDB
#
# SPDX-License-Identifier: Apache-2.0

import os
import sys
from pathlib import Path
from lib.scylla_cloud import get_cloud_instance, is_gce, is_azure, is_redhat_variant
from subprocess import run

if __name__ == '__main__':
    if is_azure() and not Path('/mnt/swapfile').exists():
        Path('/etc/systemd/system/mnt-swapfile.swap').unlink(missing_ok=True)
        run('/opt/scylladb/scripts/scylla_swap_setup --swap-directory /mnt', shell=True, check=True)
    machine_image_configured = Path('/etc/scylla/machine_image_configured')
    if not machine_image_configured.exists():
        # On Ubuntu, we configure CPU scaling while AMI building time
        if is_redhat_variant():

@yaronkaikov
Copy link
Collaborator

@syuu1228 Please verify it's working. I am using us-east-1-x86_64 = ami-0ecd0f56a75ebd7f7 image which was build based on your branch. I have set an instance based on this image, and i don't see swap, can you please verify it?

I just checked us-east-1-x86_64 = ami-0ecd0f56a75ebd7f7, scylla_image_setup on this image is not applied this patch for some reason.

If the patch applied it should be like this:

if __name__ == '__main__':
    if is_azure():
        swap_directory = Path('/mnt')
        swap_unit = Path('/etc/systemd/system/mnt-swapfile.swap')
    else:
        swap_directory = Path('/')
        swap_unit = Path('/etc/systemd/system/swapfile.swap')
    swapfile = swap_directory / 'swapfile'
    if not swapfile.exists():
        swap_unit.unlink(missing_ok=True)
        run(f'/opt/scylladb/scripts/scylla_swap_setup --swap-directory {swap_directory}', shell=True, check=True)

But scylla_image_setup on this image seems like not applied the patch:

$ sudo head -n20 /opt/scylladb/scylla-machine-image/libexec/scylla_image_setup 
#!/usr/bin/env python3
#
# Copyright 2020 ScyllaDB
#
# SPDX-License-Identifier: Apache-2.0

import os
import sys
from pathlib import Path
from lib.scylla_cloud import get_cloud_instance, is_gce, is_azure, is_redhat_variant
from subprocess import run

if __name__ == '__main__':
    if is_azure() and not Path('/mnt/swapfile').exists():
        Path('/etc/systemd/system/mnt-swapfile.swap').unlink(missing_ok=True)
        run('/opt/scylladb/scripts/scylla_swap_setup --swap-directory /mnt', shell=True, check=True)
    machine_image_configured = Path('/etc/scylla/machine_image_configured')
    if not machine_image_configured.exists():
        # On Ubuntu, we configure CPU scaling while AMI building time
        if is_redhat_variant():

That's wired. @syuu1228 Please use https://jenkins.scylladb.com/job/scylla-master/job/releng-testing/job/ami/288/ to re-build AMI and verify it's working

@yaronkaikov
Copy link
Collaborator

Also you should rebase :-)

This reverts commit 069318b.

Related with scylladb#491, we need to reduce snapshot size of the rootfs, we
should not pre-allocate swapfile while building image.
@syuu1228
Copy link
Contributor Author

That's wired. @syuu1228 Please use https://jenkins.scylladb.com/job/scylla-master/job/releng-testing/job/ami/288/ to re-build AMI and verify it's working

Seems like https://jenkins.scylladb.com/job/scylla-master/job/releng-testing/job/ami/288/ does not build scylla-machine-image package in my branch, probably it's latest master.

When I tested on locally built AMI, it just worked:
us-east-1: ami-057ebb446e55189d3

I'm also trying to build AMI on releng-testing/next-machine-image, but keep failing without understandable error message, not sure why.
https://jenkins.scylladb.com/job/scylla-master/job/releng-testing/job/next-machine-image/251/console

@yaronkaikov
Copy link
Collaborator

That's wired. @syuu1228 Please use https://jenkins.scylladb.com/job/scylla-master/job/releng-testing/job/ami/288/ to re-build AMI and verify it's working

Seems like https://jenkins.scylladb.com/job/scylla-master/job/releng-testing/job/ami/288/ does not build scylla-machine-image package in my branch, probably it's latest master.

When I tested on locally built AMI, it just worked: us-east-1: ami-057ebb446e55189d3

I'm also trying to build AMI on releng-testing/next-machine-image, but keep failing without understandable error message, not sure why. https://jenkins.scylladb.com/job/scylla-master/job/releng-testing/job/next-machine-image/251/console

Probably because of the last successful next under releng-testing. I fixed it now. next run should work

@yaronkaikov
Copy link
Collaborator

That's wired. @syuu1228 Please use https://jenkins.scylladb.com/job/scylla-master/job/releng-testing/job/ami/288/ to re-build AMI and verify it's working

Seems like https://jenkins.scylladb.com/job/scylla-master/job/releng-testing/job/ami/288/ does not build scylla-machine-image package in my branch, probably it's latest master.
When I tested on locally built AMI, it just worked: us-east-1: ami-057ebb446e55189d3
I'm also trying to build AMI on releng-testing/next-machine-image, but keep failing without understandable error message, not sure why. https://jenkins.scylladb.com/job/scylla-master/job/releng-testing/job/next-machine-image/251/console

Probably because of the last successful next under releng-testing. I fixed it now. next run should work

Just passed @syuu1228

@syuu1228
Copy link
Contributor Author

next-machine-image passed: https://jenkins.scylladb.com/job/scylla-master/job/releng-testing/job/next-machine-image/255/
Also verified swapfile is created successfully on all clouds:

  • AWS
$ swapon
NAME      TYPE SIZE USED PRIO
/swapfile file  10G   0B   -2
  • GCE
$ swapon
NAME      TYPE SIZE USED PRIO
/swapfile file 2.6G   0B   -2
  • Azure
$ swapon
NAME          TYPE SIZE USED PRIO
/mnt/swapfile file  16G   0B   -2

(The difference of swap size is comming from memory size of the instance)

@yaronkaikov yaronkaikov merged commit 26b93d5 into scylladb:next Dec 20, 2023
1 check passed
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.

2 participants