From 46f1b03145fe17ea7cb9913a6f2b9d9b22f3754d Mon Sep 17 00:00:00 2001 From: Nathan Ward Date: Mon, 2 May 2016 16:41:53 +1200 Subject: [PATCH 1/4] Manage the maximum-pool-size configuration parameter in database.ini --- manifests/init.pp | 4 +++ manifests/params.pp | 18 +++++----- manifests/server.pp | 34 +++++++++++-------- manifests/server/database.pp | 6 ++++ manifests/server/read_database.pp | 33 +++++++++--------- spec/unit/classes/server/database_ini_spec.rb | 16 +++++++++ 6 files changed, 72 insertions(+), 39 deletions(-) diff --git a/manifests/init.pp b/manifests/init.pp index 96eba23..6ce5ca3 100644 --- a/manifests/init.pp +++ b/manifests/init.pp @@ -69,6 +69,8 @@ class puppetdb ( $temp_usage = $puppetdb::params::temp_usage, $certificate_whitelist_file = $puppetdb::params::certificate_whitelist_file, $certificate_whitelist = $puppetdb::params::certificate_whitelist, + $database_max_pool_size = $puppetdb::params::database_max_pool_size, + $read_database_max_pool_size = $puppetdb::params::read_database_max_pool_size, ) inherits puppetdb::params { class { '::puppetdb::server': @@ -136,6 +138,8 @@ class puppetdb ( temp_usage => $temp_usage, certificate_whitelist_file => $certificate_whitelist_file, certificate_whitelist => $certificate_whitelist, + database_max_pool_size => $database_max_pool_size, + read_database_max_pool_size => $read_database_max_pool_size, } if ($database == 'postgres') { diff --git a/manifests/params.pp b/manifests/params.pp index 9079705..23f6f9c 100644 --- a/manifests/params.pp +++ b/manifests/params.pp @@ -20,14 +20,15 @@ class puppetdb::params inherits puppetdb::globals { $postgres_version = '9.4' # The remaining database settings are not used for an embedded database - $database_host = 'localhost' - $database_port = '5432' - $database_name = 'puppetdb' - $database_username = 'puppetdb' - $database_password = 'puppetdb' - $database_ssl = undef - $jdbc_ssl_properties = '' - $database_validate = true + $database_host = 'localhost' + $database_port = '5432' + $database_name = 'puppetdb' + $database_username = 'puppetdb' + $database_password = 'puppetdb' + $database_ssl = undef + $jdbc_ssl_properties = '' + $database_validate = true + $database_max_pool_size = '25' # These settings manage the various auto-deactivation and auto-purge settings $node_ttl = '0s' @@ -57,6 +58,7 @@ class puppetdb::params inherits puppetdb::globals { $read_conn_max_age = '60' $read_conn_keep_alive = '45' $read_conn_lifetime = '0' + $read_database_max_pool_size = '25' $manage_firewall = true $java_args = {} diff --git a/manifests/server.pp b/manifests/server.pp index d668323..26e48d4 100644 --- a/manifests/server.pp +++ b/manifests/server.pp @@ -64,6 +64,8 @@ class puppetdb::server ( $temp_usage = $puppetdb::params::temp_usage, $certificate_whitelist_file = $puppetdb::params::certificate_whitelist_file, $certificate_whitelist = $puppetdb::params::certificate_whitelist, + $database_max_pool_size = $puppetdb::params::database_max_pool_size, + $read_database_max_pool_size = $puppetdb::params::read_database_max_pool_size, ) inherits puppetdb::params { # deprecation warnings if $database_ssl != undef { @@ -151,6 +153,7 @@ class puppetdb::server ( database_password => $database_password, database_name => $database_name, database_ssl => $database_ssl, + database_max_pool_size => $database_max_pool_size, jdbc_ssl_properties => $jdbc_ssl_properties, database_validate => $database_validate, database_embedded_path => $database_embedded_path, @@ -167,21 +170,22 @@ class puppetdb::server ( } class { 'puppetdb::server::read_database': - database => $read_database, - database_host => $read_database_host, - database_port => $read_database_port, - database_username => $read_database_username, - database_password => $read_database_password, - database_name => $read_database_name, - database_ssl => $read_database_ssl, - jdbc_ssl_properties => $read_database_jdbc_ssl_properties, - database_validate => $read_database_validate, - log_slow_statements => $read_log_slow_statements, - conn_max_age => $read_conn_max_age, - conn_keep_alive => $read_conn_keep_alive, - conn_lifetime => $read_conn_lifetime, - confdir => $confdir, - notify => Service[$puppetdb_service], + database => $read_database, + database_host => $read_database_host, + database_port => $read_database_port, + database_username => $read_database_username, + database_password => $read_database_password, + database_name => $read_database_name, + database_ssl => $read_database_ssl, + database_max_pool_size => $read_database_max_pool_size, + jdbc_ssl_properties => $read_database_jdbc_ssl_properties, + database_validate => $read_database_validate, + log_slow_statements => $read_log_slow_statements, + conn_max_age => $read_conn_max_age, + conn_keep_alive => $read_conn_keep_alive, + conn_lifetime => $read_conn_lifetime, + confdir => $confdir, + notify => Service[$puppetdb_service], } if str2bool($ssl_set_cert_paths) == true diff --git a/manifests/server/database.pp b/manifests/server/database.pp index 83a7dc0..ec1c3fd 100644 --- a/manifests/server/database.pp +++ b/manifests/server/database.pp @@ -19,6 +19,7 @@ class puppetdb::server::database ( $conn_keep_alive = $puppetdb::params::conn_keep_alive, $conn_lifetime = $puppetdb::params::conn_lifetime, $confdir = $puppetdb::params::confdir, + $database_max_pool_size = $puppetdb::params::database_max_pool_size, ) inherits puppetdb::params { if str2bool($database_validate) { @@ -147,4 +148,9 @@ class puppetdb::server::database ( setting => 'conn-lifetime', value => $conn_lifetime, } + + ini_setting { 'puppetdb_database_max_pool_size': + setting => 'maximum-pool-size', + value => $database_max_pool_size, + } } diff --git a/manifests/server/read_database.pp b/manifests/server/read_database.pp index 6d78b47..532889d 100644 --- a/manifests/server/read_database.pp +++ b/manifests/server/read_database.pp @@ -1,21 +1,22 @@ # PRIVATE CLASS - do not use directly class puppetdb::server::read_database ( - $database = $puppetdb::params::read_database, - $database_host = $puppetdb::params::read_database_host, - $database_port = $puppetdb::params::read_database_port, - $database_username = $puppetdb::params::read_database_username, - $database_password = $puppetdb::params::read_database_password, - $database_name = $puppetdb::params::read_database_name, - $database_ssl = $puppetdb::params::read_database_ssl, - $jdbc_ssl_properties = $puppetdb::params::read_database_jdbc_ssl_properties, - $database_validate = $puppetdb::params::read_database_validate, - $log_slow_statements = $puppetdb::params::read_log_slow_statements, - $conn_max_age = $puppetdb::params::read_conn_max_age, - $conn_keep_alive = $puppetdb::params::read_conn_keep_alive, - $conn_lifetime = $puppetdb::params::read_conn_lifetime, - $confdir = $puppetdb::params::confdir, - $puppetdb_user = $puppetdb::params::puppetdb_user, - $puppetdb_group = $puppetdb::params::puppetdb_group, + $database = $puppetdb::params::read_database, + $database_host = $puppetdb::params::read_database_host, + $database_port = $puppetdb::params::read_database_port, + $database_username = $puppetdb::params::read_database_username, + $database_password = $puppetdb::params::read_database_password, + $database_name = $puppetdb::params::read_database_name, + $database_ssl = $puppetdb::params::read_database_ssl, + $jdbc_ssl_properties = $puppetdb::params::read_database_jdbc_ssl_properties, + $database_validate = $puppetdb::params::read_database_validate, + $log_slow_statements = $puppetdb::params::read_log_slow_statements, + $conn_max_age = $puppetdb::params::read_conn_max_age, + $conn_keep_alive = $puppetdb::params::read_conn_keep_alive, + $conn_lifetime = $puppetdb::params::read_conn_lifetime, + $confdir = $puppetdb::params::confdir, + $puppetdb_user = $puppetdb::params::puppetdb_user, + $puppetdb_group = $puppetdb::params::puppetdb_group, + $database_max_pool_size = $puppetdb::params::read_database_max_pool_size, ) inherits puppetdb::params { # Only add the read database configuration if database host is defined. diff --git a/spec/unit/classes/server/database_ini_spec.rb b/spec/unit/classes/server/database_ini_spec.rb index ffc9e35..c80ef5e 100644 --- a/spec/unit/classes/server/database_ini_spec.rb +++ b/spec/unit/classes/server/database_ini_spec.rb @@ -118,6 +118,14 @@ describe 'puppetdb::server::database', :type => :class do 'setting' => 'conn-lifetime', 'value' => '0' )} + it { should contain_ini_setting('puppetdb_database_max_pool_size'). + with( + 'ensure' => 'present', + 'path' => '/etc/puppetlabs/puppetdb/conf.d/database.ini', + 'section' => 'database', + 'setting' => 'maximum-pool-size', + 'value' => '25' + )} end describe 'when using a legacy PuppetDB version' do @@ -226,6 +234,14 @@ describe 'puppetdb::server::database', :type => :class do 'setting' => 'conn-lifetime', 'value' => '0' )} + it { should contain_ini_setting('puppetdb_database_max_pool_size'). + with( + 'ensure' => 'present', + 'path' => '/etc/puppetdb/conf.d/database.ini', + 'section' => 'database', + 'setting' => 'maximum-pool-size', + 'value' => '25' + )} end describe 'when overriding database_path for embedded' do From 63ab89048d0f88facadc5636c53a5459b580d299 Mon Sep 17 00:00:00 2001 From: Kevin Paulisse Date: Sat, 28 May 2016 23:16:56 -0500 Subject: [PATCH 2/4] Connection params off by default but settable via hiera --- manifests/params.pp | 13 ++- manifests/server/database.pp | 15 ++- manifests/server/read_database.pp | 14 +++ spec/unit/classes/server/database_ini_spec.rb | 108 +++++++++++++++--- 4 files changed, 129 insertions(+), 21 deletions(-) diff --git a/manifests/params.pp b/manifests/params.pp index 23f6f9c..804cc0f 100644 --- a/manifests/params.pp +++ b/manifests/params.pp @@ -28,7 +28,7 @@ class puppetdb::params inherits puppetdb::globals { $database_ssl = undef $jdbc_ssl_properties = '' $database_validate = true - $database_max_pool_size = '25' + $database_max_pool_size = undef # These settings manage the various auto-deactivation and auto-purge settings $node_ttl = '0s' @@ -58,7 +58,7 @@ class puppetdb::params inherits puppetdb::globals { $read_conn_max_age = '60' $read_conn_keep_alive = '45' $read_conn_lifetime = '0' - $read_database_max_pool_size = '25' + $read_database_max_pool_size = undef $manage_firewall = true $java_args = {} @@ -166,4 +166,13 @@ class puppetdb::params inherits puppetdb::globals { $certificate_whitelist = [ ] # change to this to only allow access by the puppet master by default: #$certificate_whitelist = [ $::servername ] + + # Get the parameter name for the database connection pool tuning + if $puppetdb_version in ['latest','present'] or versioncmp($puppetdb_version, '4.0.0') >= 0 { + $database_max_pool_size_setting_name = 'maximum-pool-size' + } elsif versioncmp($puppetdb_version, '3.2.0') >= 0 { + $database_max_pool_size_setting_name = 'partition-conn-max' + } else { + $database_max_pool_size_setting_name = undef + } } diff --git a/manifests/server/database.pp b/manifests/server/database.pp index 02277d6..c3a19ae 100644 --- a/manifests/server/database.pp +++ b/manifests/server/database.pp @@ -161,8 +161,17 @@ class puppetdb::server::database ( value => $conn_lifetime, } - ini_setting { 'puppetdb_database_max_pool_size': - setting => 'maximum-pool-size', - value => $database_max_pool_size, + if $puppetdb::params::database_max_pool_size_setting_name != undef { + if $database_max_pool_size == 'absent' { + ini_setting { 'puppetdb_database_max_pool_size': + ensure => absent, + setting => $puppetdb::params::database_max_pool_size_setting_name, + } + } elsif $database_max_pool_size != undef { + ini_setting { 'puppetdb_database_max_pool_size': + setting => $puppetdb::params::database_max_pool_size_setting_name, + value => $database_max_pool_size, + } + } } } diff --git a/manifests/server/read_database.pp b/manifests/server/read_database.pp index 37abd64..daa1904 100644 --- a/manifests/server/read_database.pp +++ b/manifests/server/read_database.pp @@ -130,6 +130,20 @@ class puppetdb::server::read_database ( setting => 'conn-lifetime', value => $conn_lifetime, } + + if $puppetdb::params::database_max_pool_size_setting_name != undef { + if $database_max_pool_size == 'absent' { + ini_setting { 'puppetdb_read_database_max_pool_size': + ensure => absent, + setting => $puppetdb::params::database_max_pool_size_setting_name, + } + } elsif $database_max_pool_size != undef { + ini_setting { 'puppetdb_read_database_max_pool_size': + setting => $puppetdb::params::database_max_pool_size_setting_name, + value => $database_max_pool_size, + } + } + } } else { file { "${confdir}/read_database.ini": ensure => absent, diff --git a/spec/unit/classes/server/database_ini_spec.rb b/spec/unit/classes/server/database_ini_spec.rb index 5d9956d..f8886f4 100644 --- a/spec/unit/classes/server/database_ini_spec.rb +++ b/spec/unit/classes/server/database_ini_spec.rb @@ -125,14 +125,7 @@ describe 'puppetdb::server::database', :type => :class do 'setting' => 'conn-lifetime', 'value' => '0' )} - it { should contain_ini_setting('puppetdb_database_max_pool_size'). - with( - 'ensure' => 'present', - 'path' => '/etc/puppetlabs/puppetdb/conf.d/database.ini', - 'section' => 'database', - 'setting' => 'maximum-pool-size', - 'value' => '25' - )} + it { should_not contain_ini_setting('puppetdb_database_max_pool_size') } end describe 'when using a legacy PuppetDB version' do @@ -241,14 +234,7 @@ describe 'puppetdb::server::database', :type => :class do 'setting' => 'conn-lifetime', 'value' => '0' )} - it { should contain_ini_setting('puppetdb_database_max_pool_size'). - with( - 'ensure' => 'present', - 'path' => '/etc/puppetdb/conf.d/database.ini', - 'section' => 'database', - 'setting' => 'maximum-pool-size', - 'value' => '25' - )} + it { should_not contain_ini_setting('puppetdb_database_max_pool_size') } end describe 'when overriding database_path for embedded' do @@ -267,5 +253,95 @@ describe 'puppetdb::server::database', :type => :class do 'value' => 'file:/tmp/foo;hsqldb.tx=mvcc;sql.syntax_pgs=true' )} end + + describe 'when setting max pool size' do + context 'on current PuppetDB' do + describe 'to a numeric value' do + let(:params) do + { + 'database_max_pool_size': 12345 + } + end + it { should contain_ini_setting('puppetdb_database_max_pool_size'). + with( + 'ensure' => 'present', + 'path' => '/etc/puppetlabs/puppetdb/conf.d/database.ini', + 'section' => 'database', + 'setting' => 'maximum-pool-size', + 'value' => '12345' + )} + end + + describe 'to absent' do + let(:params) do + { + 'database_max_pool_size': 'absent' + } + end + it { should contain_ini_setting('puppetdb_database_max_pool_size'). + with( + 'ensure' => 'absent', + 'path' => '/etc/puppetlabs/puppetdb/conf.d/database.ini', + 'section' => 'database', + 'setting' => 'maximum-pool-size' + )} + end + end + + context 'on PuppetDB 3.2' do + let (:pre_condition) { 'class { "puppetdb::globals": version => "3.2.0", }' } + describe 'to a numeric value' do + let(:params) do + { + 'database_max_pool_size': 12345 + } + end + it { should contain_ini_setting('puppetdb_database_max_pool_size'). + with( + 'ensure' => 'present', + 'path' => '/etc/puppetlabs/puppetdb/conf.d/database.ini', + 'section' => 'database', + 'setting' => 'partition-conn-max', + 'value' => '12345' + )} + end + + describe 'to absent' do + let(:params) do + { + 'database_max_pool_size': 'absent' + } + end + it { should contain_ini_setting('puppetdb_database_max_pool_size'). + with( + 'ensure' => 'absent', + 'path' => '/etc/puppetlabs/puppetdb/conf.d/database.ini', + 'section' => 'database', + 'setting' => 'partition-conn-max' + )} + end + end + + context 'on a legacy PuppetDB version' do + let (:pre_condition) { 'class { "puppetdb::globals": version => "2.2.0", }' } + describe 'to a numeric value' do + let(:params) do + { + 'database_max_pool_size': 12345 + } + end + it { should_not contain_ini_setting('puppetdb_database_max_pool_size') } + end + + describe 'to absent' do + let(:params) do + { + 'database_max_pool_size': 'absent' + } + end + it { should_not contain_ini_setting('puppetdb_database_max_pool_size') } + end + end + end end end From feebb0088ea05f84a95d153442c46f39869c4475 Mon Sep 17 00:00:00 2001 From: Kevin Paulisse Date: Sat, 28 May 2016 23:57:48 -0500 Subject: [PATCH 3/4] Fix syntax error in tests --- spec/unit/classes/server/database_ini_spec.rb | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/spec/unit/classes/server/database_ini_spec.rb b/spec/unit/classes/server/database_ini_spec.rb index f8886f4..07b48fd 100644 --- a/spec/unit/classes/server/database_ini_spec.rb +++ b/spec/unit/classes/server/database_ini_spec.rb @@ -259,7 +259,7 @@ describe 'puppetdb::server::database', :type => :class do describe 'to a numeric value' do let(:params) do { - 'database_max_pool_size': 12345 + 'database_max_pool_size' => 12345 } end it { should contain_ini_setting('puppetdb_database_max_pool_size'). @@ -275,7 +275,7 @@ describe 'puppetdb::server::database', :type => :class do describe 'to absent' do let(:params) do { - 'database_max_pool_size': 'absent' + 'database_max_pool_size' => 'absent' } end it { should contain_ini_setting('puppetdb_database_max_pool_size'). @@ -293,7 +293,7 @@ describe 'puppetdb::server::database', :type => :class do describe 'to a numeric value' do let(:params) do { - 'database_max_pool_size': 12345 + 'database_max_pool_size' => 12345 } end it { should contain_ini_setting('puppetdb_database_max_pool_size'). @@ -309,7 +309,7 @@ describe 'puppetdb::server::database', :type => :class do describe 'to absent' do let(:params) do { - 'database_max_pool_size': 'absent' + 'database_max_pool_size' => 'absent' } end it { should contain_ini_setting('puppetdb_database_max_pool_size'). @@ -327,7 +327,7 @@ describe 'puppetdb::server::database', :type => :class do describe 'to a numeric value' do let(:params) do { - 'database_max_pool_size': 12345 + 'database_max_pool_size' => 12345 } end it { should_not contain_ini_setting('puppetdb_database_max_pool_size') } @@ -336,7 +336,7 @@ describe 'puppetdb::server::database', :type => :class do describe 'to absent' do let(:params) do { - 'database_max_pool_size': 'absent' + 'database_max_pool_size' => 'absent' } end it { should_not contain_ini_setting('puppetdb_database_max_pool_size') } From 84fefe9c0b0a8e30f99dd1f9c8081ff3065c9976 Mon Sep 17 00:00:00 2001 From: Kevin Paulisse Date: Wed, 22 Jun 2016 15:01:13 -0500 Subject: [PATCH 4/4] Support partition-conn-max back to 2.8 --- manifests/params.pp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/manifests/params.pp b/manifests/params.pp index 804cc0f..639d3fa 100644 --- a/manifests/params.pp +++ b/manifests/params.pp @@ -170,7 +170,7 @@ class puppetdb::params inherits puppetdb::globals { # Get the parameter name for the database connection pool tuning if $puppetdb_version in ['latest','present'] or versioncmp($puppetdb_version, '4.0.0') >= 0 { $database_max_pool_size_setting_name = 'maximum-pool-size' - } elsif versioncmp($puppetdb_version, '3.2.0') >= 0 { + } elsif versioncmp($puppetdb_version, '2.8.0') >= 0 { $database_max_pool_size_setting_name = 'partition-conn-max' } else { $database_max_pool_size_setting_name = undef