Ema has submitted this change and it was merged. ( https://gerrit.wikimedia.org/r/385378 )
Change subject: cache: set varnish runtime parameters via hiera ...................................................................... cache: set varnish runtime parameters via hiera Introduce two new hiera settings under profile::cache::base, fe_runtime_params and be_runtime_params, allowing to specify varnish runtime parameters (varnishd -p) for varnish frontends and backends respectively. Bug: T159429 Change-Id: I0d4b3fd482dc45326d47e88185383684320fb844 --- M hieradata/labs.yaml M hieradata/role/common/cache/canary.yaml M hieradata/role/common/cache/misc.yaml M hieradata/role/common/cache/text.yaml M hieradata/role/common/cache/upload.yaml M modules/cacheproxy/manifests/instance_pair.pp M modules/profile/manifests/cache/base.pp M modules/profile/manifests/cache/misc.pp M modules/profile/manifests/cache/text.pp M modules/profile/manifests/cache/upload.pp M modules/varnish/manifests/common.pp M modules/varnish/manifests/instance.pp 12 files changed, 94 insertions(+), 89 deletions(-) Approvals: Ema: Looks good to me, approved jenkins-bot: Verified diff --git a/hieradata/labs.yaml b/hieradata/labs.yaml index 0e229d6..caa3fdd 100644 --- a/hieradata/labs.yaml +++ b/hieradata/labs.yaml @@ -63,6 +63,11 @@ profile::cache::base::zero_site: 'https://zero.wikimedia.beta.wmflabs.org' profile::cache::base::purge_host_only_upload_re: '^(upload|maps)\.beta\.wmflabs\.org$' profile::cache::base::purge_host_not_upload_re: '^(?!(upload|maps)\.beta\.wmflabs\.org)' +profile::cache::base::fe_runtime_params: + - default_ttl=3600 +profile::cache::base::be_runtime_params: + - default_ttl=3600 + - timeout_idle=120 # Profile::cache::ssl::unified profile::cache::ssl::unified::monitoring: false profile::cache::ssl::unified::letsencrypt: true diff --git a/hieradata/role/common/cache/canary.yaml b/hieradata/role/common/cache/canary.yaml index a7f8984..f614bfc 100644 --- a/hieradata/role/common/cache/canary.yaml +++ b/hieradata/role/common/cache/canary.yaml @@ -79,6 +79,12 @@ profile::cache::base::purge_host_only_upload_re: '^(upload|maps)\.wikimedia\.org$' profile::cache::base::purge_host_not_upload_re: '^(?!(upload|maps)\.wikimedia\.org)' profile::cache::base::storage_parts: ['sda3', 'sdb3'] +profile::cache::base::fe_runtime_params: + - default_ttl=86400 +profile::cache::base::be_runtime_params: + - default_ttl=86400 + - nuke_limit=1000 + - lru_interval=31 profile::cache::base::varnish_version: 5 # Profile::cache::ssl::unified profile::cache::ssl::unified::monitoring: true diff --git a/hieradata/role/common/cache/misc.yaml b/hieradata/role/common/cache/misc.yaml index f819499..e1b8c71 100644 --- a/hieradata/role/common/cache/misc.yaml +++ b/hieradata/role/common/cache/misc.yaml @@ -282,6 +282,13 @@ profile::cache::base::purge_host_regex: '' profile::cache::base::purge_multicasts: ['239.128.0.115'] profile::cache::base::purge_varnishes: ['127.0.0.1:3128', '127.0.0.1:3127'] +profile::cache::base::fe_runtime_params: + - default_ttl=3600 +profile::cache::base::be_runtime_params: + - default_ttl=3600 + - timeout_idle=120 + - nuke_limit=1000 + - lru_interval=31 # Profile::cache::ssl::unified profile::cache::ssl::unified::monitoring: true profile::cache::ssl::unified::letsencrypt: false diff --git a/hieradata/role/common/cache/text.yaml b/hieradata/role/common/cache/text.yaml index 6c3c798..87fb290 100644 --- a/hieradata/role/common/cache/text.yaml +++ b/hieradata/role/common/cache/text.yaml @@ -86,6 +86,12 @@ profile::cache::base::purge_host_regex: '' profile::cache::base::purge_multicasts: ['239.128.0.112'] profile::cache::base::purge_varnishes: ['127.0.0.1:3128', '127.0.0.1:3127'] +profile::cache::base::fe_runtime_params: + - default_ttl=86400 +profile::cache::base::be_runtime_params: + - default_ttl=86400 + - nuke_limit=1000 + - lru_interval=31 # Profile::cache::ssl::unified profile::cache::ssl::unified::monitoring: true profile::cache::ssl::unified::letsencrypt: false diff --git a/hieradata/role/common/cache/upload.yaml b/hieradata/role/common/cache/upload.yaml index 1e25a91..97c92c1 100644 --- a/hieradata/role/common/cache/upload.yaml +++ b/hieradata/role/common/cache/upload.yaml @@ -58,6 +58,12 @@ profile::cache::base::purge_host_regex: '[um][pa][lp][os]' profile::cache::base::purge_multicasts: ['239.128.0.112', '239.128.0.113', '239.128.0.114'] profile::cache::base::purge_varnishes: ['127.0.0.1:3128', '127.0.0.1:3127'] +profile::cache::base::fe_runtime_params: + - default_ttl=86400 +profile::cache::base::be_runtime_params: + - default_ttl=86400 + - nuke_limit=1000 + - lru_interval=31 # Profile::cache::ssl::unified profile::cache::ssl::unified::monitoring: true profile::cache::ssl::unified::letsencrypt: false diff --git a/modules/cacheproxy/manifests/instance_pair.pp b/modules/cacheproxy/manifests/instance_pair.pp index f1c5c59..e60f88f 100644 --- a/modules/cacheproxy/manifests/instance_pair.pp +++ b/modules/cacheproxy/manifests/instance_pair.pp @@ -2,8 +2,6 @@ class cacheproxy::instance_pair ( $cache_type, $fe_jemalloc_conf, - $fe_runtime_params, - $be_runtime_params, $app_directors, $app_def_be_opts, $fe_vcl_config, @@ -18,7 +16,6 @@ $fe_transient_gb=0, $be_transient_gb=0, $backend_warming=false, - $exp_thread_rt=false, ) { # ideally this could be built with "map"... # also, in theory all caches sites should be listed here for flexibility, @@ -76,16 +73,6 @@ $be_transient_storage = "-s Transient=malloc,${be_transient_gb}G" } - # Experimental backend settings to handle T145661 - if $exp_thread_rt { - $exp_thread_params = ['exp_thread_rt=true','exp_lck_inherit=true'] - } else { - $exp_thread_params = [] - } - - # backends: Should increase nuke success chances, and reduce LRU lock/modify rate - $nuke_lru_params = ['nuke_limit=1000','lru_interval=31'] - # VCL files common to all instances. It's actually ok to declare it here as this module depends # on the varnish one # lint:ignore:wmf_styleguide @@ -100,18 +87,17 @@ }) varnish::instance { "${cache_type}-backend": - instance_name => '', - layer => 'backend', - vcl => "${cache_type}-backend", - extra_vcl => $be_extra_vcl, - ports => [ '3128' ], - admin_port => 6083, - runtime_parameters => concat($be_runtime_params, $nuke_lru_params, $exp_thread_params), - storage => "${be_storage} ${be_transient_storage}", - vcl_config => $be_warming_vcl_config, - app_directors => $app_directors, - app_def_be_opts => $app_def_be_opts, - backend_caches => $our_backend_caches, + instance_name => '', + layer => 'backend', + vcl => "${cache_type}-backend", + extra_vcl => $be_extra_vcl, + ports => [ '3128' ], + admin_port => 6083, + storage => "${be_storage} ${be_transient_storage}", + vcl_config => $be_warming_vcl_config, + app_directors => $app_directors, + app_def_be_opts => $app_def_be_opts, + backend_caches => $our_backend_caches, } # Set a reduced keep value for frontends @@ -132,7 +118,6 @@ extra_vcl => $fe_extra_vcl, ports => [ '80', '3120', '3121', '3122', '3123', '3124', '3125', '3126', '3127' ], admin_port => 6082, - runtime_parameters => $fe_runtime_params, storage => "-s malloc,${fe_mem_gb}G ${fe_transient_storage}", jemalloc_conf => $fe_jemalloc_conf, backend_caches => { diff --git a/modules/profile/manifests/cache/base.pp b/modules/profile/manifests/cache/base.pp index 77bd19d..ac93afd 100644 --- a/modules/profile/manifests/cache/base.pp +++ b/modules/profile/manifests/cache/base.pp @@ -18,6 +18,8 @@ $purge_host_regex = hiera('profile::cache::base::purge_host_regex', ''), $purge_multicasts = hiera('profile::cache::base::purge_multicasts', ['239.128.0.112']), $purge_varnishes = hiera('profile::cache::base::purge_varnishes', ['127.0.0.1:3128', '127.0.0.1:3127']), + $fe_runtime_params = hiera('profile::cache::base::fe_runtime_params', []), + $be_runtime_params = hiera('profile::cache::base::be_runtime_params', []), ) { # There is no better way to do this, so it can't be a class parameter. In fact, # I consider our requirement to make hiera calls parameters @@ -61,7 +63,9 @@ } class { '::varnish::common': - varnish_version => $varnish_version, + varnish_version => $varnish_version, + fe_runtime_params => $fe_runtime_params, + be_runtime_params => $be_runtime_params, } class { [ diff --git a/modules/profile/manifests/cache/misc.pp b/modules/profile/manifests/cache/misc.pp index 000be44..c214ead 100644 --- a/modules/profile/manifests/cache/misc.pp +++ b/modules/profile/manifests/cache/misc.pp @@ -42,27 +42,19 @@ 'req_handling' => $req_handling, } - $common_runtime_params = ['default_ttl=3600'] - - # The default timeout_idle setting, 5s, seems to be causing sc_rx_timeout - # issues in our setup. See T159429 - $be_runtime_params = ['timeout_idle=120'] - class { 'cacheproxy::instance_pair': - cache_type => 'misc', - fe_jemalloc_conf => 'lg_dirty_mult:8,lg_chunk:16', - fe_runtime_params => $common_runtime_params, - be_runtime_params => concat($common_runtime_params, $be_runtime_params), - app_directors => $app_directors, - app_def_be_opts => $app_def_be_opts, - fe_vcl_config => $common_vcl_config, - be_vcl_config => $common_vcl_config, - fe_extra_vcl => ['misc-common', 'zero'], - be_extra_vcl => ['misc-common'], - be_storage => $::profile::cache::base::file_storage_args, - fe_cache_be_opts => $fe_cache_be_opts, - be_cache_be_opts => $be_cache_be_opts, - cluster_nodes => $nodes, - cache_route => $cache_route, + cache_type => 'misc', + fe_jemalloc_conf => 'lg_dirty_mult:8,lg_chunk:16', + app_directors => $app_directors, + app_def_be_opts => $app_def_be_opts, + fe_vcl_config => $common_vcl_config, + be_vcl_config => $common_vcl_config, + fe_extra_vcl => ['misc-common', 'zero'], + be_extra_vcl => ['misc-common'], + be_storage => $::profile::cache::base::file_storage_args, + fe_cache_be_opts => $fe_cache_be_opts, + be_cache_be_opts => $be_cache_be_opts, + cluster_nodes => $nodes, + cache_route => $cache_route, } } diff --git a/modules/profile/manifests/cache/text.pp b/modules/profile/manifests/cache/text.pp index a052c01..4d2469c 100644 --- a/modules/profile/manifests/cache/text.pp +++ b/modules/profile/manifests/cache/text.pp @@ -60,29 +60,25 @@ 'enable_geoiplookup' => true, }) - $common_runtime_params = ['default_ttl=86400'] - $text_storage_args = $::profile::cache::base::file_storage_args class { 'cacheproxy::instance_pair': - cache_type => 'text', - fe_jemalloc_conf => 'lg_dirty_mult:8,lg_chunk:16', - fe_runtime_params => $common_runtime_params, - be_runtime_params => $common_runtime_params, - app_directors => $app_directors, - app_def_be_opts => $app_def_be_opts, - fe_vcl_config => $fe_vcl_config, - be_vcl_config => $be_vcl_config, - fe_extra_vcl => ['text-common', 'zero', 'normalize_path', 'geoip'], - be_extra_vcl => ['text-common', 'normalize_path'], - be_storage => $text_storage_args, - fe_cache_be_opts => $fe_cache_be_opts, - be_cache_be_opts => $be_cache_be_opts, - cluster_nodes => $nodes, - cache_route => $cache_route, - fe_transient_gb => $fe_transient_gb, - be_transient_gb => $be_transient_gb, - backend_warming => $backend_warming, + cache_type => 'text', + fe_jemalloc_conf => 'lg_dirty_mult:8,lg_chunk:16', + app_directors => $app_directors, + app_def_be_opts => $app_def_be_opts, + fe_vcl_config => $fe_vcl_config, + be_vcl_config => $be_vcl_config, + fe_extra_vcl => ['text-common', 'zero', 'normalize_path', 'geoip'], + be_extra_vcl => ['text-common', 'normalize_path'], + be_storage => $text_storage_args, + fe_cache_be_opts => $fe_cache_be_opts, + be_cache_be_opts => $be_cache_be_opts, + cluster_nodes => $nodes, + cache_route => $cache_route, + fe_transient_gb => $fe_transient_gb, + be_transient_gb => $be_transient_gb, + backend_warming => $backend_warming, } # varnishkafka statsv listens for special stats related requests diff --git a/modules/profile/manifests/cache/upload.pp b/modules/profile/manifests/cache/upload.pp index ccf7df9..758a314 100644 --- a/modules/profile/manifests/cache/upload.pp +++ b/modules/profile/manifests/cache/upload.pp @@ -75,25 +75,21 @@ "-s bin4=file,/srv/${sda}/varnish.bin4,${bin4_size}M", ], ' ') - $common_runtime_params = ['default_ttl=86400'] - class { 'cacheproxy::instance_pair': - cache_type => 'upload', - fe_jemalloc_conf => 'lg_dirty_mult:8,lg_chunk:17', - fe_runtime_params => $common_runtime_params, - be_runtime_params => $common_runtime_params, - app_directors => $app_directors, - app_def_be_opts => $app_def_be_opts, - fe_vcl_config => $fe_vcl_config, - be_vcl_config => $be_vcl_config, - fe_extra_vcl => ['upload-common'], - be_extra_vcl => ['upload-common'], - be_storage => $upload_storage_args, - fe_cache_be_opts => $fe_cache_be_opts, - be_cache_be_opts => $be_cache_be_opts, - cluster_nodes => $cluster_nodes, - cache_route => $cache_route, - backend_warming => $backend_warming, + cache_type => 'upload', + fe_jemalloc_conf => 'lg_dirty_mult:8,lg_chunk:17', + app_directors => $app_directors, + app_def_be_opts => $app_def_be_opts, + fe_vcl_config => $fe_vcl_config, + be_vcl_config => $be_vcl_config, + fe_extra_vcl => ['upload-common'], + be_extra_vcl => ['upload-common'], + be_storage => $upload_storage_args, + fe_cache_be_opts => $fe_cache_be_opts, + be_cache_be_opts => $be_cache_be_opts, + cluster_nodes => $cluster_nodes, + cache_route => $cache_route, + backend_warming => $backend_warming, } # Media browser cache hit rate and request volume stats. diff --git a/modules/varnish/manifests/common.pp b/modules/varnish/manifests/common.pp index d8ec92c..3603e09 100644 --- a/modules/varnish/manifests/common.pp +++ b/modules/varnish/manifests/common.pp @@ -1,4 +1,4 @@ -class varnish::common($varnish_version=4) { +class varnish::common($varnish_version=4, $fe_runtime_params=[], $be_runtime_params=[]) { require ::varnish::packages # Mount /var/lib/varnish as tmpfs to avoid Linux flushing mlocked diff --git a/modules/varnish/manifests/instance.pp b/modules/varnish/manifests/instance.pp index 3f0462e..7d1086f 100644 --- a/modules/varnish/manifests/instance.pp +++ b/modules/varnish/manifests/instance.pp @@ -7,7 +7,6 @@ $vcl = '', $storage='-s malloc,1G', $jemalloc_conf=undef, - $runtime_parameters=[], $app_directors={}, $app_def_be_opts={}, $backend_caches={}, @@ -17,7 +16,6 @@ include ::varnish::common - $runtime_params = join(prefix($runtime_parameters, '-p '), ' ') if $instance_name == '' { $instancesuffix = '' $extraopts = '' @@ -40,10 +38,14 @@ # Write the dynamic backend caches configuration, if we need it if $instance_name == '' { $inst = 'backend' + $runtime_parameters = $::varnish::common::be_runtime_params } else { $inst = $instance_name + $runtime_parameters = $::varnish::common::fe_runtime_params } + $runtime_params = join(prefix($runtime_parameters, '-p '), ' ') + varnish::common::directors { $vcl: instance => $inst, directors => $backend_caches, -- To view, visit https://gerrit.wikimedia.org/r/385378 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: merged Gerrit-Change-Id: I0d4b3fd482dc45326d47e88185383684320fb844 Gerrit-PatchSet: 6 Gerrit-Project: operations/puppet Gerrit-Branch: production Gerrit-Owner: Ema <[email protected]> Gerrit-Reviewer: BBlack <[email protected]> Gerrit-Reviewer: Ema <[email protected]> Gerrit-Reviewer: jenkins-bot <> _______________________________________________ MediaWiki-commits mailing list [email protected] https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
