search

scylladb / scylla-machine-image

Apache License 2.0
18 stars 25 forks source link

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

Closed syuu1228 closed 6 months ago

syuu1228 commented 6 months ago

This reverts commit 069318bb48963aa5bede707579e499a8bd1288c4.

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

syuu1228 commented 6 months ago

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.

yaronkaikov commented 6 months ago

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 commented 6 months ago

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

syuu1228 commented 6 months ago

@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 commented 6 months ago

@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 commented 6 months ago

@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 commented 6 months ago

@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 commented 6 months ago

Also you should rebase :-)

syuu1228 commented 6 months ago

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 commented 6 months ago

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 commented 6 months ago

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 commented 6 months ago

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: