hortonworks / streamline

StreamLine - Streaming Analytics
Apache License 2.0
164 stars 96 forks source link

Default worker.childopts in topology settings should be -Xmx 1024mb _JAAS_PLACEHOLDER #653

Closed harshach closed 7 years ago

HeartSaVioR commented 7 years ago

@harshach Could we just change worker.childopts to topology.worker.childopts? Do we have use case for overriding cluster configuration?

Another approach is that we can even get the value of worker.childopts from Storm cluster configuration when opening topology config like component bundle hint.

@shahsank3t Is it possible to get hints for topology configuration for UI perspective?

HeartSaVioR commented 7 years ago

In addition, hard-coding _JAAS_PLACEHOLDER would not work with non-Ambari Storm cluster.

harshach commented 7 years ago

@HeartSaVioR agree on changing worker.childopts to topology.worker.childopts

shahsank3t commented 7 years ago

@harshach what hints are we talking about?

HeartSaVioR commented 7 years ago

@harshach OK. Maybe need to check topology.worker.childopts can override -Xmx setting. Will check and comment.

@shahsank3t We have config field worker.childopts for topology config, and we can get default value from Storm cluster configuration. Hence would like to see possibility of applying component bundle hint for topology config.

shahsank3t commented 7 years ago

@HeartSaVioR @harshach Yes, the backend can pass the data inside "api/v1/catalog/clusters/:clusterId/services/storm/topologies" into hints:{} block and I'll make the relevant changes on the UI to read and set them.

shahsank3t commented 7 years ago

@HeartSaVioR Just make sure you pass the exact fieldName into hints as mentioned into storm-topology-component.json. Like: https://github.com/hortonworks/streamline/blob/master/bootstrap/components/topology/storm-topology-component.json?short_path=329ca57#L36

HeartSaVioR commented 7 years ago

@harshach Putting -Xmx2g in topology.worker.childopts takes precedence over -Xmx768m in worker.childopts since it's placed later. The order of priority is in fact not guaranteed for all of JVMs, but seems like recent JVMs follow this approach.

I'll just create a patch around changing worker.childopts to topology.worker.childopts but I'm also open to make change with keep it as worker.childopts and provide hint value.

@shahsank3t Let's wait for decision before working on this. Btw, providing principal to topology config is already done in secured cluster hence we already provide hint for topology config, right?

HeartSaVioR commented 7 years ago

656 for changing worker.childopts option to topology.worker.childopts

harshach commented 7 years ago

@HeartSaVioR whichever is easier and faster. I am ok with both.

HeartSaVioR commented 7 years ago

@harshach OK then let's apply #656 for now.

HeartSaVioR commented 7 years ago

@harshach Since #656 is merged, let's close this.