#429 Added new map_hash_bucket_size and map_hash_max_size paramater #430
Conversation
| @@ -221,6 +229,8 @@ | |||
| multi_accept => $multi_accept, | |||
| names_hash_bucket_size => $names_hash_bucket_size, | |||
| names_hash_max_size => $names_hash_max_size, | |||
| map_hash_bucket_size => $map_hash_bucket_size, | |||
There was a problem hiding this comment.
This creates an error because the new parameters haven't been added to the nginx::config class.
|
I added a couple of comments - please also add tests in |
|
Hi, you are right about your comments... I forgot the config file. Greetings Jan |
|
It's true some PRs were merged without tests in the past, but going forward tests will be strongly encouraged. New values in the templates will require tests prior to merging. |
| @@ -28,6 +28,8 @@ | |||
| $nx_types_hash_max_size = 1024 | |||
| $nx_types_hash_bucket_size = 512 | |||
| $nx_names_hash_bucket_size = 64 | |||
| $nx_map_hash_bucket_size = 64 | |||
There was a problem hiding this comment.
64 is not always an appropriate default since map_hash_bucket_size has a dynamic default based on the processor's cache line size. Please set this to undef in params.pp and add logic to the template so map_hash_bucket_size is only added to the config when set by the user.
|
For now I am not quite experienced with the testing from puppet modules. As soon I am, I will add a pull request. |
|
Closing this one, since @jg-development said he will open another PR when tests are added. |
Closes #429