From 7577eec93cd66575a68ff4abb1d300c8a2fb351b Mon Sep 17 00:00:00 2001 From: Thomas Goirand Date: Wed, 9 Jun 2021 09:23:59 +0200 Subject: [PATCH] Make it possible to skip parted+mkfs Previously, each run puppet runs a parted and a xfs_admin command, which itself triggers udev rules. In production, and during a rebalance, this may trigger a *very* high load on the storage nodes. This patch introduces 2 a new options: "manage_partition" and "manage_filesystem" which are "true" by default (not changing the past behavior), so administrators can, once the cluster is setup and the new disks are in, set this variable to "false", or manage partitions and formating out of puppet. Change-Id: I2740da550157c94cd80805f60321d3cd91381cff --- manifests/storage/disk.pp | 46 +++++++++++++------ manifests/storage/xfs.pp | 39 ++++++++++------ ...ition_and_filesystem-64cd4086d1bb5b0a.yaml | 13 ++++++ spec/defines/swift_storage_disk_spec.rb | 1 - 4 files changed, 71 insertions(+), 28 deletions(-) create mode 100644 releasenotes/notes/manage_partition_and_filesystem-64cd4086d1bb5b0a.yaml diff --git a/manifests/storage/disk.pp b/manifests/storage/disk.pp index 74278625..b71576e7 100644 --- a/manifests/storage/disk.pp +++ b/manifests/storage/disk.pp @@ -33,6 +33,19 @@ # (optional) The external command that will be used in parted command. # Default to ''. For making partitions, it would be 'mkpart primary 0% 100%'. # +# [*manage_partition*] +# (optional) If set to false, skip calling parted which can, in some cases, +# increase the load on the server. This is to set to false only after the +# server is fully setup or if the partition was created outside of puppet. +# Defaults to true. +# +# [*manage_filesystem*] +# (optional) If set to false, skip calling xfs_admin -l to check if a +# partition needs to be formated with mkfs.xfs, which can, in some cases, +# increase the load on the server. This is to set to false only after the +# server is fully setup, or if the filesystem was created outside of puppet. +# Defaults to true. +# # =Example= # # Simply add one disk sdb: @@ -48,10 +61,12 @@ # TODO(yuxcer): maybe we can remove param $base_dir # define swift::storage::disk( - $base_dir = '/dev', - $mnt_base_dir = '/srv/node', - $byte_size = '1024', - $ext_args = '', + $base_dir = '/dev', + $mnt_base_dir = '/srv/node', + $byte_size = '1024', + $ext_args = '', + $manage_partition = true, + $manage_filesystem = true, ) { include swift::deps @@ -66,19 +81,22 @@ define swift::storage::disk( } } - exec { "create_partition_label-${name}": - command => "parted -s ${base_dir}/${name} mklabel gpt ${ext_args}", - path => ['/usr/bin/', '/sbin','/bin'], - onlyif => ["test -b ${base_dir}/${name}","parted ${base_dir}/${name} print|tail -1|grep 'Error'"], - before => Anchor['swift::config::end'], + if $manage_partition { + exec { "create_partition_label-${name}": + command => "parted -s ${base_dir}/${name} mklabel gpt ${ext_args}", + path => ['/usr/bin/', '/sbin','/bin'], + onlyif => ["test -b ${base_dir}/${name}","parted ${base_dir}/${name} print|tail -1|grep 'Error'"], + before => Anchor['swift::config::end'], + } + Exec["create_partition_label-${name}"] ~> Swift::Storage::Xfs<| title == $name |> } swift::storage::xfs { $name: - device => "${base_dir}/${name}", - mnt_base_dir => $mnt_base_dir, - byte_size => $byte_size, - loopback => false, - subscribe => Exec["create_partition_label-${name}"], + device => "${base_dir}/${name}", + mnt_base_dir => $mnt_base_dir, + byte_size => $byte_size, + loopback => false, + manage_filesystem => $manage_filesystem, } } diff --git a/manifests/storage/xfs.pp b/manifests/storage/xfs.pp index 1dee0425..fda02f36 100644 --- a/manifests/storage/xfs.pp +++ b/manifests/storage/xfs.pp @@ -21,6 +21,13 @@ # (optional) Define if the device is mounted by the device partition path or UUID. # Defaults to 'path'. # +# [*manage_filesystem*] +# (optional) If set to false, skip calling xfs_admin -l to check if a +# partition needs to be formated with mkfs.xfs, which can, in some cases, +# increase the load on the server. This is to set to false only after the +# server is fully setup, or if the filesystem was created outside of puppet. +# Defaults to true. +# # Sample usage: # # swift::storage::xfs { @@ -34,11 +41,12 @@ # it already has an XFS FS, and mounts de FS in /srv/node/sdX # define swift::storage::xfs( - $device = '', - $byte_size = '1024', - $mnt_base_dir = '/srv/node', - $loopback = false, - $mount_type = 'path', + $device = '', + $byte_size = '1024', + $mnt_base_dir = '/srv/node', + $loopback = false, + $mount_type = 'path', + $manage_filesystem = true, ) { include swift::deps @@ -77,11 +85,19 @@ define swift::storage::xfs( # If it's not a valid XFS FS, command will return 1 # so we format it. If device has a valid XFS FS, command returns 0 # So we do NOT touch it. - exec { "mkfs-${name}": - command => "mkfs.xfs -f -i size=${byte_size} ${target_device}", - path => ['/sbin/', '/usr/sbin/'], - unless => "xfs_admin -l ${target_device}", - before => Anchor['swift::config::end'], + if $manage_filesystem { + exec { "mkfs-${name}": + command => "mkfs.xfs -f -i size=${byte_size} ${target_device}", + path => ['/sbin/', '/usr/sbin/'], + unless => "xfs_admin -l ${target_device}", + before => Anchor['swift::config::end'], + } + Package<| title == 'xfsprogs' |> + ~> Exec<| title == "mkfs-${name}" |> + ~> Swift::Storage::Mount<| title == $name |> + } else { + Package<| title == 'xfsprogs' |> + ~> Swift::Storage::Mount<| title == $name |> } swift::storage::mount { $name: @@ -90,8 +106,5 @@ define swift::storage::xfs( loopback => $loopback, } - Package<| title == 'xfsprogs' |> - ~> Exec<| title == "mkfs-${name}" |> - ~> Swift::Storage::Mount<| title == $name |> } diff --git a/releasenotes/notes/manage_partition_and_filesystem-64cd4086d1bb5b0a.yaml b/releasenotes/notes/manage_partition_and_filesystem-64cd4086d1bb5b0a.yaml new file mode 100644 index 00000000..ee8e1438 --- /dev/null +++ b/releasenotes/notes/manage_partition_and_filesystem-64cd4086d1bb5b0a.yaml @@ -0,0 +1,13 @@ +--- +features: + - | + The class swift::storage::disk now has 2 new parameters called + manage_partition and manage_filesystem. The swift::storage::xfs now has + a new parameter manage_filesystem. Both have a value of true by default. + These new options may be used to skip running parted and xfs_admin once + the storage disks are setup. The goal is to avoid the kernel to flush + the HDD buffer, which can seriously increase the load and IO wait on a + (busy) production server. These parameters should be left to true when + setting-up a new server, or when changing a disk, and can be set to false + otherwise, or always set to false if the formating is done outside of + puppet. diff --git a/spec/defines/swift_storage_disk_spec.rb b/spec/defines/swift_storage_disk_spec.rb index 7d2e20bb..a17e7563 100644 --- a/spec/defines/swift_storage_disk_spec.rb +++ b/spec/defines/swift_storage_disk_spec.rb @@ -28,7 +28,6 @@ describe 'swift::storage::disk' do :device => '/dev/sdb', :mnt_base_dir => '/srv/node', :byte_size => '1024', - :subscribe => 'Exec[create_partition_label-sdb]', :loopback => false ) } end