voxpupuli / puppet-elasticsearch

Elasticsearch Puppet module
Apache License 2.0
403 stars 478 forks source link

Whitespace in init_defaults(for -Xms -Xmx) #736

Closed MoisesRT closed 7 years ago

MoisesRT commented 7 years ago

I need to configure -Xms and -Xmx in the init_defaults but I can't. I think it is because the module is using augeas which handles whitespaces and I am trying to add:

'ES_JAVA_OPTS' => "-Xmx${es_heap_size}m -Xms${es_heap_size}m"

in the init_defaults

elisiano commented 7 years ago

I'm having exactly the same issue (I commented yesterday in https://github.com/elastic/puppet-elasticsearch/issues/105 ).

I also narrowed down to the space in the value (because I tried setting only one option without spaces and it works).

I do have multiple augeas providers (camptocamp/augeas and herculesteam/agueasproviders_shellvar) but I was unable to select the augeasprovider_shellvar. I tried with Augeas { provider => 'X' } (where X was one of the values found in /var/lib/puppet/lib/puppet/provider/ ).

tylerjl commented 7 years ago

I think the correct answer here is to just manage the jvm.options file in the Elasticsearch 5.x configuration directory - that's the recommended way to control the JVM options in 5.x.

@MoisesRT @elisiano, I propose changing this issue to managing jvm.options and exposing parameters to control the file contents via a parameter, thoughts?

elisiano commented 7 years ago

Personally I'm fine with that option but there are few things to keep in mind:

Thoughts?

MoisesRT commented 7 years ago

+1

Even if I manage the jvm file using an ERB it will be one more thing to manage. I have 2 suggestions: 1.The startup script is already an ERB, therefore it won't be hard to add it to the command there.

  1. Generate the defaults file using ERB and not using augeas.
tylerjl commented 7 years ago

I did some quick testing and confirmed a workaround for now for anyone that runs into this. Because of the way augeas interacts with key/value pairs, a setting like the one below will work as expected:

'ES_JAVA_OPTS' => "\"-Xmx${es_heap_size}m -Xms${es_heap_size}m\""

Essentially you just need to ensure the value for the hash is a string wrapped in quotes, and augeas will write the value correctly.

With regards to jvm.options, AFAIK, if the configuration path for an Elasticsearch instance points to an individual directory (i.e., /etc/elasticsearch/es-01) and there's a jvm.options file there, it will control each instance individually, so we don't necessarily need to worry about it applying globally. As for how to control the content, I also think we just need a parameter that can accept an array of values that gets written to jvm.options defaulting to what Elasticsearch 5.x currently ships, with the ability to override those options.

elisiano commented 7 years ago

This works, thanks @tylerjl ! Although in the long term we might definitely want to manage jvm.options

erikanderson commented 7 years ago

It looks like the /etc/elasticsearch/jvm.options values (in my case 2G) are overriding what is being set in /etc/default/elasticsearch-${instance} (set to 4G) when the service is started

elisiano commented 7 years ago

not for me. Just to be clear, I'm defining my values in hiera with the following key: elasticsearch::init_defaults (which indeed are not instance-specific)

agarstang commented 7 years ago

I appear to be getting the same as @erikanderson

olafz commented 7 years ago

I got the same issue. When defining ES_JAVA_OPTS="-Xms4g -Xmx4g" in init_defaults, augeas fails ("Failed to match").

agarstang commented 7 years ago

I am doing the following (using augeas to remove the defaults) because the jvm.options file is not yet managed.

$init_hash = {
    'ES_JAVA_OPTS' => "\"-Xms${heap_size}m -Xmx${heap_size}m\"",
 }

class { 'elasticsearch':
    config                => { 'cluster.name' => $clustername },
    version               => $version,
    manage_repo           => true,
    repo_version          => "${major_version}.x",
    init_defaults         => $init_hash,
    restart_config_change => true,
    java_install          => true,
    datadir               => $data_dir,
}

augeas { 'jvm.options':
    incl    => '/etc/elasticsearch/jvm.options',
    lens    => 'Properties.lns',
    changes => "rm -Xms2g\nrm -Xmx2g",
    notify  => Service["elasticsearch-instance-${es_instance_name}"],
    require => Class['elasticsearch'],
} # Remove HEAP settings from jvm.options so that ES_JAVA_OPTS above takes effect
tylerjl commented 7 years ago

I've merged a change to support managing the jvm.options file in #766 . This should resolve setting memory settings for ES 5.x instances so setting the -Xmx/-Xms flags shouldn't be needed in the init defaults file. This will go out in the next release.

Thanks!

cherweg commented 7 years ago

@tylerjl: Any ideas when this will be released?

Waiting for release ... or building an workaround myself?!

regards Christian

tylerjl commented 7 years ago

@cherweg it's looking like with the changes to support x-pack, I may be cutting a major release since some of these changes are backwards-incompatible. If that's the case, I'll be syncing up the module version with the rest of the Elastic projects (i.e., version 5.0) and releasing under the elastic/ namespace on the Puppet forget, so it's a work in progress. Thanks for your patience!