Skip to content

Commit

Permalink
scylla_prepare + scylla_cpuset_setup: make scylla_cpuset_setup idempo…
Browse files Browse the repository at this point in the history
…tent without introducing regressions

This patch fixes the regression introduced by 3a51e78 which broke
a very important contract: perftune.yaml should not be "touched"
by Scylla scriptology unless explicitly requested.

And a call for scylla_cpuset_setup is such an explicit request.

The issue that the offending patch was intending to fix was that
cpuset.conf was always generated anew for every call of
scylla_cpuset_setup - even if a resulting cpuset.conf would come
out exactly the same as the one present on the disk before tha call.

And since the original code was following the contract mentioned above
it was also deleting perftune.yaml every time too.
However, this was just an unavoidable side-effect of that cpuset.conf
re-generation.

The above also means that if scylla_cpuset_setup doesn't write to cpuset.conf
we should not "touch" perftune.yaml and vise versa.

This patch implements exactly that together with reverting the dangerous
logic introduced by 3a51e78.

Fixes scylladb#11385
Fixes scylladb#10121
  • Loading branch information
vladzcloudius authored and syuu1228 committed Oct 8, 2022
1 parent e383997 commit 72de094
Show file tree
Hide file tree
Showing 2 changed files with 4 additions and 11 deletions.
3 changes: 3 additions & 0 deletions dist/common/scripts/scylla_cpuset_setup
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,9 @@ if __name__ == '__main__':
except:
pass
if cpuset != args.cpuset or smp != args.smp:
if os.path.exists('/etc/scylla.d/perftune.yaml'):
os.remove('/etc/scylla.d/perftune.yaml')

cfg.set('CPUSET', '{cpuset}{smp}'.format( \
cpuset='--cpuset {} '.format(args.cpuset) if args.cpuset else '', \
smp='--smp {} '.format(args.smp) if args.smp else '' \
Expand Down
12 changes: 1 addition & 11 deletions dist/common/scripts/scylla_prepare
Original file line number Diff line number Diff line change
Expand Up @@ -62,16 +62,6 @@ def get_irq_cpu_mask():

return out(hwloc_cmd).strip()

def config_updated():
perftune_mtime = os.path.getmtime('/etc/scylla.d/perftune.yaml')
cpuset_mtime = os.path.getmtime('/etc/scylla.d/cpuset.conf')
sysconfig_mtime = os.path.getmtime(sysconfdir_p() / 'scylla-server')
print("perftune_mtime < cpuset_mtime:{}".format(perftune_mtime < cpuset_mtime))
print("perftune_mtime < sysconfig_mtime:{}".format(perftune_mtime < sysconfig_mtime))
if perftune_mtime < cpuset_mtime or perftune_mtime < sysconfig_mtime:
return True
return False

def create_perftune_conf(cfg):
"""
This function checks if a perftune configuration file should be created and
Expand All @@ -97,7 +87,7 @@ def create_perftune_conf(cfg):
params += ' --write-back-cache=false'

if len(params) > 0:
if os.path.exists('/etc/scylla.d/perftune.yaml') and not config_updated():
if os.path.exists('/etc/scylla.d/perftune.yaml'):
return True

params += ' --dump-options-file'
Expand Down

0 comments on commit 72de094

Please sign in to comment.