You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@druid.apache.org by GitBox <gi...@apache.org> on 2021/07/09 15:34:35 UTC
[GitHub] [druid] oliverdding opened a new pull request #11427: helm chart fix/update
oliverdding opened a new pull request #11427:
URL: https://github.com/apache/druid/pull/11427
<!-- Thanks for trying to help us make Apache Druid be the best it can be! Please fill out as much of the following information as is possible (where relevant, and remove it when irrelevant) to help make the intention and scope of this PR clear in order to ease review. -->
<!-- Please read the doc for contribution (https://github.com/apache/druid/blob/master/CONTRIBUTING.md) before making this PR. Also, once you open a PR, please _avoid using force pushes and rebasing_ since these make it difficult for reviewers to see what you've changed in response to their reviews. See [the 'If your pull request shows conflicts with master' section](https://github.com/apache/druid/blob/master/CONTRIBUTING.md#if-your-pull-request-shows-conflicts-with-master) for more details. -->
Fixes #11421.
<!-- Replace XXXX with the id of the issue fixed in this PR. Remove this section if there is no corresponding issue. Don't reference the issue in the title of this pull-request. -->
<!-- If you are a committer, follow the PR action item checklist for committers:
https://github.com/apache/druid/blob/master/dev/committer-instructions.md#pr-and-issue-action-item-checklist-for-committers. -->
<!-- Describe the goal of this PR, what problem are you fixing. If there is a corresponding issue (referenced above), it's not necessary to repeat the description here, however, you may choose to keep one summary sentence. -->
<!-- Describe your patch: what did you change in code? How did you fix the problem? -->
<!-- If there are several relatively logically separate changes in this PR, create a mini-section for each of them. For example: -->
<!--
In each section, please describe design decisions made, including:
- Choice of algorithms
- Behavioral aspects. What configuration values are acceptable? How are corner cases and error conditions handled, such as when there are insufficient resources?
- Class organization and design (how the logic is split between classes, inheritance, composition, design patterns)
- Method organization and design (how the logic is split between methods, parameters and return types)
- Naming (class, method, API, configuration, HTTP endpoint, names of emitted metrics)
-->
<!-- It's good to describe an alternative design (or mention an alternative name) for every design (or naming) decision point and compare the alternatives with the designs that you've implemented (or the names you've chosen) to highlight the advantages of the chosen designs and names. -->
<!-- If there was a discussion of the design of the feature implemented in this PR elsewhere (e. g. a "Proposal" issue, any other issue, or a thread in the development mailing list), link to that discussion from this PR description and explain what have changed in your final design compared to your original proposal or the consensus version in the end of the discussion. If something hasn't changed since the original discussion, you can omit a detailed discussion of those aspects of the design here, perhaps apart from brief mentioning for the sake of readability of this PR description. -->
<!-- Some of the aspects mentioned above may be omitted for simple and small changes. -->
<hr>
##### Key changed/added classes in this PR
* `helm chart`
<hr>
<!-- Check the items by putting "x" in the brackets for the done things. Not all of these items apply to every PR. Remove the items which are not done or not relevant to the PR. None of the items from the checklist below are strictly necessary, but it would be very helpful if you at least self-review the PR. -->
This PR has:
- [x] been self-reviewed.
- [ ] using the [concurrency checklist](https://github.com/apache/druid/blob/master/dev/code-review/concurrency.md) (Remove this item if the PR doesn't have any relation to concurrency.)
- [ ] added documentation for new or modified features or behaviors.
- [ ] added Javadocs for most classes and all non-trivial methods. Linked related entities via Javadoc links.
- [ ] added or updated version, license, or notice information in [licenses.yaml](https://github.com/apache/druid/blob/master/dev/license.md)
- [ ] added comments explaining the "why" and the intent of the code wherever would not be obvious for an unfamiliar reader.
- [ ] added unit tests or modified existing tests to cover new code paths, ensuring the threshold for [code coverage](https://github.com/apache/druid/blob/master/dev/code-review/code-coverage.md) is met.
- [ ] added integration tests.
- [x] been tested in a test Druid cluster.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] abhishekagarwal87 commented on pull request #11427: helm chart fix/update
Posted by GitBox <gi...@apache.org>.
abhishekagarwal87 commented on pull request #11427:
URL: https://github.com/apache/druid/pull/11427#issuecomment-896692250
Thank you for your contribution @oliverdding. I had some questions and suggestions
- what zk ensemble, the druid services will be initialized with after your changes? We would still need to pass `druid_zk_service_host` somehow?
- Instead of removing `mysql` and `postgres` entirely, I will suggest keeping them with `enabled` as `false` in default `values.yaml`. You can have two more `values.yaml` files inside `mysql` and `postgres` folder that a user can choose to override default `values.yaml` with.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] oliverdding commented on pull request #11427: helm chart fix/update
Posted by GitBox <gi...@apache.org>.
oliverdding commented on pull request #11427:
URL: https://github.com/apache/druid/pull/11427#issuecomment-895769997
> @oliverdding thanks for the contribution. I don't know a lot about helm charts. How can we validate that the helm chart continues to work as new code changes come in? Are there any integration tests that will prove this works with every release of Apache Druid?
As far as I known, no... Helm chart maintainer have to konw the great picture of Druid, and helm chart is like a script for deploying, only who had read the whole chart may tell what should be changeh in the next release...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] asdf2014 commented on a change in pull request #11427: helm chart fix/update
Posted by GitBox <gi...@apache.org>.
asdf2014 commented on a change in pull request #11427:
URL: https://github.com/apache/druid/pull/11427#discussion_r667424856
##########
File path: helm/druid/README.md
##########
@@ -25,20 +6,19 @@
To install the Druid Chart into your Kubernetes cluster :
-```bash
-helm install --namespace "druid" --name "druid" incubator/druid
-```
+1. `cd` to root directory of druid project
+2. execute command ```bash helm install --namespace "druid" --name "druid" helm/druid```
Review comment:
I recommend the former, you can also use your local repo, but we shouldn't force rebase. FYI, https://github.com/apache/druid/blob/master/CONTRIBUTING.md#if-your-pull-request-shows-conflicts-with-master
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] suneet-s commented on pull request #11427: helm chart fix/update
Posted by GitBox <gi...@apache.org>.
suneet-s commented on pull request #11427:
URL: https://github.com/apache/druid/pull/11427#issuecomment-895288183
@asdf2014 is there anything left to merge this change?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] oliverdding commented on a change in pull request #11427: helm chart fix/update
Posted by GitBox <gi...@apache.org>.
oliverdding commented on a change in pull request #11427:
URL: https://github.com/apache/druid/pull/11427#discussion_r686748893
##########
File path: helm/druid/values.yaml
##########
@@ -26,32 +26,37 @@ configMap:
##
enabled: true
-## Define the key value pairs in the configmap
-configVars:
+## The configurations below would be set as environment variable across all pod
+globalConfig:
## DRUID env vars. ref: https://github.com/apache/druid/blob/master/distribution/docker/druid.sh#L29
- # DRUID_LOG_LEVEL: "warn"
- # DRUID_LOG4J: <?xml version="1.0" encoding="UTF-8" ?><Configuration status="WARN"><Appenders><Console name="Console" target="SYSTEM_OUT"><PatternLayout pattern="%d{ISO8601} %p [%t] %c - %m%n"/></Console></Appenders><Loggers><Root level="info"><AppenderRef ref="Console"/></Root><Logger name="org.apache.druid.jetty.RequestLog" additivity="false" level="DEBUG"><AppenderRef ref="Console"/></Logger></Loggers></Configuration>
- DRUID_USE_CONTAINER_IP: "true"
+ DRUID_USE_CONTAINER_IP: 'true'
## Druid Common Configurations. ref: https://druid.apache.org/docs/latest/configuration/index.html#common-configurations
- druid_extensions_loadList: '["druid-histogram", "druid-datasketches", "druid-lookups-cached-global", "postgresql-metadata-storage"]'
- druid_metadata_storage_type: postgresql
- druid_metadata_storage_connector_connectURI: jdbc:postgresql://postgres:5432/druid
- druid_metadata_storage_connector_user: druid
- druid_metadata_storage_connector_password: druid
+ druid_extensions_loadList: '["druid-datasketches", "druid-lookups-cached-global", "postgresql-metadata-storage"]'
+
+ druid_metadata_storage_type: derby
+
druid_storage_type: local
+
druid_indexer_logs_type: file
- druid_indexer_logs_directory: /opt/data/indexing-logs
## Druid Emitting Metrics. ref: https://druid.apache.org/docs/latest/configuration/index.html#emitting-metrics
druid_emitter: noop
- druid_emitter_logging_logLevel: debug
- druid_emitter_http_recipientBaseUrl: http://druid_exporter_url:druid_exporter_port/druid
+ druid_emitter_logging_logLevel: info
gCloudStorage:
enabled: false
secretName: google-cloud-key
+hdfs:
+ # Enable hdfs as deep-storage reference: http://druid.apache.org/docs/latest/development/extensions-core/hdfs.html
+ # Need to do:
+ # 1. enabled: true
+ # 2. add `"druid-hdfs-storage"` to `druid_extensions_loadList`
+ # 3. supply configmap's name which contains hdfs-site.xml and core-site.xml
+ enabled: false
+ configMapName: ''
Review comment:
The configmap is looks like:
```yaml
apiVersion: v1
kind: ConfigMap
data:
core-site.xml: |
<balabala something here>
hdfs-site.xml: |
<balabala something here>
```
In fact, the configmap would be generate by every deployment of hdfs, which ought to contain the configurations that hdfs client needs when accessing hdfs. I did not supply a template file for that configmap, since when deploying druid with hdfs as deep storage, the configmap would have been in place.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] asdf2014 commented on a change in pull request #11427: helm chart fix/update
Posted by GitBox <gi...@apache.org>.
asdf2014 commented on a change in pull request #11427:
URL: https://github.com/apache/druid/pull/11427#discussion_r667347010
##########
File path: helm/druid/values.yaml
##########
@@ -26,32 +26,37 @@ configMap:
##
enabled: true
-## Define the key value pairs in the configmap
-configVars:
+## The configurations below would be set as environment variable across all pod
+globalConfig:
## DRUID env vars. ref: https://github.com/apache/druid/blob/master/distribution/docker/druid.sh#L29
- # DRUID_LOG_LEVEL: "warn"
- # DRUID_LOG4J: <?xml version="1.0" encoding="UTF-8" ?><Configuration status="WARN"><Appenders><Console name="Console" target="SYSTEM_OUT"><PatternLayout pattern="%d{ISO8601} %p [%t] %c - %m%n"/></Console></Appenders><Loggers><Root level="info"><AppenderRef ref="Console"/></Root><Logger name="org.apache.druid.jetty.RequestLog" additivity="false" level="DEBUG"><AppenderRef ref="Console"/></Logger></Loggers></Configuration>
- DRUID_USE_CONTAINER_IP: "true"
+ DRUID_USE_CONTAINER_IP: 'true'
## Druid Common Configurations. ref: https://druid.apache.org/docs/latest/configuration/index.html#common-configurations
- druid_extensions_loadList: '["druid-histogram", "druid-datasketches", "druid-lookups-cached-global", "postgresql-metadata-storage"]'
- druid_metadata_storage_type: postgresql
- druid_metadata_storage_connector_connectURI: jdbc:postgresql://postgres:5432/druid
- druid_metadata_storage_connector_user: druid
- druid_metadata_storage_connector_password: druid
+ druid_extensions_loadList: '["druid-datasketches", "druid-lookups-cached-global", "postgresql-metadata-storage"]'
+
+ druid_metadata_storage_type: derby
+
druid_storage_type: local
+
druid_indexer_logs_type: file
- druid_indexer_logs_directory: /opt/data/indexing-logs
## Druid Emitting Metrics. ref: https://druid.apache.org/docs/latest/configuration/index.html#emitting-metrics
druid_emitter: noop
- druid_emitter_logging_logLevel: debug
- druid_emitter_http_recipientBaseUrl: http://druid_exporter_url:druid_exporter_port/druid
+ druid_emitter_logging_logLevel: info
gCloudStorage:
enabled: false
secretName: google-cloud-key
+hdfs:
+ # Enable hdfs as deep-storage reference: http://druid.apache.org/docs/latest/development/extensions-core/hdfs.html
+ # Need to do:
+ # 1. enabled: true
+ # 2. add hdfs extension
Review comment:
```suggestion
# 2. add `"druid-hdfs-storage"` to `druid_extensions_loadList`
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] asdf2014 commented on a change in pull request #11427: helm chart fix/update
Posted by GitBox <gi...@apache.org>.
asdf2014 commented on a change in pull request #11427:
URL: https://github.com/apache/druid/pull/11427#discussion_r667425705
##########
File path: helm/druid/Chart.yaml
##########
@@ -1,39 +1,10 @@
-# Licensed to the Apache Software Foundation (ASF) under one or more
Review comment:
We shouldn't remove these license headers, we can remove them when we release Helm chart by [helm-chart-releaser](https://github.com/marketplace/actions/helm-chart-releaser).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] oliverdding commented on a change in pull request #11427: helm chart fix/update
Posted by GitBox <gi...@apache.org>.
oliverdding commented on a change in pull request #11427:
URL: https://github.com/apache/druid/pull/11427#discussion_r667409530
##########
File path: helm/druid/README.md
##########
@@ -25,20 +6,19 @@
To install the Druid Chart into your Kubernetes cluster :
-```bash
-helm install --namespace "druid" --name "druid" incubator/druid
-```
+1. `cd` to root directory of druid project
+2. execute command ```bash helm install --namespace "druid" --name "druid" helm/druid```
Review comment:
Thanks for review! By the way, should I just commit suggestion here, or in my local git repository and then force rebase it to one commit?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] abhishekagarwal87 commented on pull request #11427: helm chart fix/update
Posted by GitBox <gi...@apache.org>.
abhishekagarwal87 commented on pull request #11427:
URL: https://github.com/apache/druid/pull/11427#issuecomment-896771074
> > Thank you for your contribution @oliverdding. I had some questions and suggestions
> >
> > * what zk ensemble, the druid services will be initialized with after your changes? We would still need to pass `druid_zk_service_host` somehow?
> > * Instead of removing `mysql` and `postgres` entirely, I will suggest keeping them with `enabled` as `false` in default `values.yaml`. You can have two more `values.yaml` files inside `mysql` and `postgres` folder that a user can choose to override default `values.yaml` with.
>
> 1. Yes, you need to pass `druid_zk_service_host` if you want to use druid under zookeeper, but I consider that free from zookeeper is the future [run without zookeeper](http://druid.apache.org/docs/latest/development/extensions-core/kubernetes.html), so we should leave this blank in the base chart.
> 2. I did this considering that:
>
> * That's not K.I.S.S. User may want to deploy druid with mysql, may want to deploy without zookeeper... Though postgresql and zookeeper are our dependencies, we should not package them with our application.
> * User should deploy their own pod to fill in their own business. For example, my company has postgresql deployed as base componment, so I just want the druid to use that one instead of taking it's own without deployed.
1. Most of the deployments are still zk based so the helm chart should support that.
2. That can be achieved by tweaking the conditions. You can introduce new values such as `install.postgres`, `install.zookeeper`, and `install.mysql` that can be `false` by default. You can use these flags as conditions for deciding whether to download a dependency.
3. As suggested, please also add values.yaml that are specific to mysql and postgres that a user can pass in addition to the default values. The `values.yaml` inside `mysql` folder will look something like this
```
mysql:
enabled: true
mysqlRootPassword: druidroot
mysqlUser: druid
mysqlPassword: druid
mysqlDatabase: druid
configurationFiles:
mysql_collate.cnf: |-
[mysqld]
character-set-server=utf8
collation-server=utf8_unicode_ci
```
default root-directory `values.yaml` will have the following section
```
mysql:
enabled: false
```
same for postgres. The changes in configmap.yaml can be reverted.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] abhishekagarwal87 commented on pull request #11427: helm chart fix/update
Posted by GitBox <gi...@apache.org>.
abhishekagarwal87 commented on pull request #11427:
URL: https://github.com/apache/druid/pull/11427#issuecomment-898880486
@oliverdding you can try to create a new fork and checkout this PR into a branch in that fork.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] oliverdding commented on pull request #11427: helm chart fix/update
Posted by GitBox <gi...@apache.org>.
oliverdding commented on pull request #11427:
URL: https://github.com/apache/druid/pull/11427#issuecomment-896742361
> Thank you for your contribution @oliverdding. I had some questions and suggestions
>
> * what zk ensemble, the druid services will be initialized with after your changes? We would still need to pass `druid_zk_service_host` somehow?
> * Instead of removing `mysql` and `postgres` entirely, I will suggest keeping them with `enabled` as `false` in default `values.yaml`. You can have two more `values.yaml` files inside `mysql` and `postgres` folder that a user can choose to override default `values.yaml` with.
1. Yes, you need to pass `druid_zk_service_host` if you want to use druid under zookeeper, but I consider that free from zookeeper is the future [run without zookeeper](http://druid.apache.org/docs/latest/development/extensions-core/kubernetes.html), so we should leave this blank in the base chart.
2. I did this considering that:
* That's not K.I.S.S. User may want to deploy druid with mysql, may want to deploy without zookeeper... Though postgresql and zookeeper are our dependencies, we should not package them with our application.
* User should deploy their own pod to fill in their own business. For example, my company has postgresql deployed as base componment, so I just want the druid to use that one instead of taking it's own without deployed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] oliverdding commented on a change in pull request #11427: helm chart fix/update
Posted by GitBox <gi...@apache.org>.
oliverdding commented on a change in pull request #11427:
URL: https://github.com/apache/druid/pull/11427#discussion_r686735345
##########
File path: helm/druid/values.yaml
##########
@@ -26,32 +26,37 @@ configMap:
##
enabled: true
-## Define the key value pairs in the configmap
-configVars:
+## The configurations below would be set as environment variable across all pod
+globalConfig:
## DRUID env vars. ref: https://github.com/apache/druid/blob/master/distribution/docker/druid.sh#L29
- # DRUID_LOG_LEVEL: "warn"
- # DRUID_LOG4J: <?xml version="1.0" encoding="UTF-8" ?><Configuration status="WARN"><Appenders><Console name="Console" target="SYSTEM_OUT"><PatternLayout pattern="%d{ISO8601} %p [%t] %c - %m%n"/></Console></Appenders><Loggers><Root level="info"><AppenderRef ref="Console"/></Root><Logger name="org.apache.druid.jetty.RequestLog" additivity="false" level="DEBUG"><AppenderRef ref="Console"/></Logger></Loggers></Configuration>
- DRUID_USE_CONTAINER_IP: "true"
+ DRUID_USE_CONTAINER_IP: 'true'
## Druid Common Configurations. ref: https://druid.apache.org/docs/latest/configuration/index.html#common-configurations
- druid_extensions_loadList: '["druid-histogram", "druid-datasketches", "druid-lookups-cached-global", "postgresql-metadata-storage"]'
- druid_metadata_storage_type: postgresql
- druid_metadata_storage_connector_connectURI: jdbc:postgresql://postgres:5432/druid
- druid_metadata_storage_connector_user: druid
- druid_metadata_storage_connector_password: druid
+ druid_extensions_loadList: '["druid-datasketches", "druid-lookups-cached-global", "postgresql-metadata-storage"]'
Review comment:
Because the [doc](http://druid.apache.org/docs/latest/development/extensions-core/approximate-histograms.html) says that it's deprecated and should use druid-datasketches instead.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] oliverdding commented on pull request #11427: helm chart fix/update
Posted by GitBox <gi...@apache.org>.
oliverdding commented on pull request #11427:
URL: https://github.com/apache/druid/pull/11427#issuecomment-898844738
>
>
> > > Thank you for your contribution @oliverdding. I had some questions and suggestions
> > >
> > > * what zk ensemble, the druid services will be initialized with after your changes? We would still need to pass `druid_zk_service_host` somehow?
> > > * Instead of removing `mysql` and `postgres` entirely, I will suggest keeping them with `enabled` as `false` in default `values.yaml`. You can have two more `values.yaml` files inside `mysql` and `postgres` folder that a user can choose to override default `values.yaml` with.
> >
> >
> >
> > 1. Yes, you need to pass `druid_zk_service_host` if you want to use druid under zookeeper, but I consider that free from zookeeper is the future [run without zookeeper](http://druid.apache.org/docs/latest/development/extensions-core/kubernetes.html), so we should leave this blank in the base chart.
> > 2. I did this considering that:
> >
> > * That's not K.I.S.S. User may want to deploy druid with mysql, may want to deploy without zookeeper... Though postgresql and zookeeper are our dependencies, we should not package them with our application.
> > * User should deploy their own pod to fill in their own business. For example, my company has postgresql deployed as base componment, so I just want the druid to use that one instead of taking it's own without deployed.
>
> 1. Most of the deployments are still zk based so the helm chart should support that.
>
> 2. That can be achieved by tweaking the conditions. You can introduce new values such as `install.postgres`, `install.zookeeper`, and `install.mysql` that can be `false` by default. You can use these flags as conditions for deciding whether to download a dependency.
>
> 3. As suggested, please also add values.yaml that are specific to mysql and postgres that a user can pass in addition to the default values. The `values.yaml` inside `mysql` folder will look something like this
>
>
> ```
> mysql:
> enabled: true
> mysqlRootPassword: druidroot
> mysqlUser: druid
> mysqlPassword: druid
> mysqlDatabase: druid
> configurationFiles:
> mysql_collate.cnf: |-
> [mysqld]
> character-set-server=utf8
> collation-server=utf8_unicode_ci
> ```
>
> default root-directory `values.yaml` will have the following section
>
> ```
> mysql:
> enabled: false
> ```
>
> same for postgres. The changes in configmap.yaml can be reverted.
>
> if a user wants to run with mysql enabled, he/she can run the helm install with values.yaml -f mysql/values.yaml
Sorry for replaying late. I'm convinced.
But I got a problem...I have deleted the forked repository when this MR is accepted...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] suneet-s commented on pull request #11427: helm chart fix/update
Posted by GitBox <gi...@apache.org>.
suneet-s commented on pull request #11427:
URL: https://github.com/apache/druid/pull/11427#issuecomment-895354496
@oliverdding thanks for the contribution. I don't know a lot about helm charts. How can we validate that the helm chart continues to work as new code changes come in? Are there any integration tests that will prove this works with every release of Apache Druid?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] abhishekagarwal87 edited a comment on pull request #11427: helm chart fix/update
Posted by GitBox <gi...@apache.org>.
abhishekagarwal87 edited a comment on pull request #11427:
URL: https://github.com/apache/druid/pull/11427#issuecomment-896771074
> > Thank you for your contribution @oliverdding. I had some questions and suggestions
> >
> > * what zk ensemble, the druid services will be initialized with after your changes? We would still need to pass `druid_zk_service_host` somehow?
> > * Instead of removing `mysql` and `postgres` entirely, I will suggest keeping them with `enabled` as `false` in default `values.yaml`. You can have two more `values.yaml` files inside `mysql` and `postgres` folder that a user can choose to override default `values.yaml` with.
>
> 1. Yes, you need to pass `druid_zk_service_host` if you want to use druid under zookeeper, but I consider that free from zookeeper is the future [run without zookeeper](http://druid.apache.org/docs/latest/development/extensions-core/kubernetes.html), so we should leave this blank in the base chart.
> 2. I did this considering that:
>
> * That's not K.I.S.S. User may want to deploy druid with mysql, may want to deploy without zookeeper... Though postgresql and zookeeper are our dependencies, we should not package them with our application.
> * User should deploy their own pod to fill in their own business. For example, my company has postgresql deployed as base componment, so I just want the druid to use that one instead of taking it's own without deployed.
1. Most of the deployments are still zk based so the helm chart should support that.
2. That can be achieved by tweaking the conditions. You can introduce new values such as `install.postgres`, `install.zookeeper`, and `install.mysql` that can be `false` by default. You can use these flags as conditions for deciding whether to download a dependency.
3. As suggested, please also add values.yaml that are specific to mysql and postgres that a user can pass in addition to the default values. The `values.yaml` inside `mysql` folder will look something like this
```
mysql:
enabled: true
mysqlRootPassword: druidroot
mysqlUser: druid
mysqlPassword: druid
mysqlDatabase: druid
configurationFiles:
mysql_collate.cnf: |-
[mysqld]
character-set-server=utf8
collation-server=utf8_unicode_ci
```
default root-directory `values.yaml` will have the following section
```
mysql:
enabled: false
```
same for postgres. The changes in configmap.yaml can be reverted.
if a user wants to run with mysql enabled, he/she can run the helm install with values.yaml -f mysql/values.yaml
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] oliverdding commented on pull request #11427: helm chart fix/update
Posted by GitBox <gi...@apache.org>.
oliverdding commented on pull request #11427:
URL: https://github.com/apache/druid/pull/11427#issuecomment-896758246
@abhishekagarwal87 I don't think packing other based componments with druid is a good idea. What's wanted would be deployed by users, what's not would bother users.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] asdf2014 commented on pull request #11427: helm chart fix/update
Posted by GitBox <gi...@apache.org>.
asdf2014 commented on pull request #11427:
URL: https://github.com/apache/druid/pull/11427#issuecomment-895304511
@suneet-s No, all good
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] abhishekagarwal87 commented on a change in pull request #11427: helm chart fix/update
Posted by GitBox <gi...@apache.org>.
abhishekagarwal87 commented on a change in pull request #11427:
URL: https://github.com/apache/druid/pull/11427#discussion_r686694231
##########
File path: helm/druid/values.yaml
##########
@@ -26,32 +26,37 @@ configMap:
##
enabled: true
-## Define the key value pairs in the configmap
-configVars:
+## The configurations below would be set as environment variable across all pod
+globalConfig:
## DRUID env vars. ref: https://github.com/apache/druid/blob/master/distribution/docker/druid.sh#L29
- # DRUID_LOG_LEVEL: "warn"
- # DRUID_LOG4J: <?xml version="1.0" encoding="UTF-8" ?><Configuration status="WARN"><Appenders><Console name="Console" target="SYSTEM_OUT"><PatternLayout pattern="%d{ISO8601} %p [%t] %c - %m%n"/></Console></Appenders><Loggers><Root level="info"><AppenderRef ref="Console"/></Root><Logger name="org.apache.druid.jetty.RequestLog" additivity="false" level="DEBUG"><AppenderRef ref="Console"/></Logger></Loggers></Configuration>
- DRUID_USE_CONTAINER_IP: "true"
+ DRUID_USE_CONTAINER_IP: 'true'
## Druid Common Configurations. ref: https://druid.apache.org/docs/latest/configuration/index.html#common-configurations
- druid_extensions_loadList: '["druid-histogram", "druid-datasketches", "druid-lookups-cached-global", "postgresql-metadata-storage"]'
- druid_metadata_storage_type: postgresql
- druid_metadata_storage_connector_connectURI: jdbc:postgresql://postgres:5432/druid
- druid_metadata_storage_connector_user: druid
- druid_metadata_storage_connector_password: druid
+ druid_extensions_loadList: '["druid-datasketches", "druid-lookups-cached-global", "postgresql-metadata-storage"]'
+
+ druid_metadata_storage_type: derby
+
druid_storage_type: local
+
druid_indexer_logs_type: file
- druid_indexer_logs_directory: /opt/data/indexing-logs
## Druid Emitting Metrics. ref: https://druid.apache.org/docs/latest/configuration/index.html#emitting-metrics
druid_emitter: noop
- druid_emitter_logging_logLevel: debug
- druid_emitter_http_recipientBaseUrl: http://druid_exporter_url:druid_exporter_port/druid
+ druid_emitter_logging_logLevel: info
gCloudStorage:
enabled: false
secretName: google-cloud-key
+hdfs:
+ # Enable hdfs as deep-storage reference: http://druid.apache.org/docs/latest/development/extensions-core/hdfs.html
+ # Need to do:
+ # 1. enabled: true
+ # 2. add `"druid-hdfs-storage"` to `druid_extensions_loadList`
+ # 3. supply configmap's name which contains hdfs-site.xml and core-site.xml
+ enabled: false
+ configMapName: ''
Review comment:
can you add an example in `hdfs/values.yaml`? It will also be a bonus if you can add configmap template for HDFS in `templates`. That configmap will be deployed only if `.values.hdfs.enabled` is set to true.
##########
File path: helm/druid/values.yaml
##########
@@ -26,32 +26,37 @@ configMap:
##
enabled: true
-## Define the key value pairs in the configmap
-configVars:
+## The configurations below would be set as environment variable across all pod
+globalConfig:
## DRUID env vars. ref: https://github.com/apache/druid/blob/master/distribution/docker/druid.sh#L29
- # DRUID_LOG_LEVEL: "warn"
- # DRUID_LOG4J: <?xml version="1.0" encoding="UTF-8" ?><Configuration status="WARN"><Appenders><Console name="Console" target="SYSTEM_OUT"><PatternLayout pattern="%d{ISO8601} %p [%t] %c - %m%n"/></Console></Appenders><Loggers><Root level="info"><AppenderRef ref="Console"/></Root><Logger name="org.apache.druid.jetty.RequestLog" additivity="false" level="DEBUG"><AppenderRef ref="Console"/></Logger></Loggers></Configuration>
- DRUID_USE_CONTAINER_IP: "true"
+ DRUID_USE_CONTAINER_IP: 'true'
## Druid Common Configurations. ref: https://druid.apache.org/docs/latest/configuration/index.html#common-configurations
- druid_extensions_loadList: '["druid-histogram", "druid-datasketches", "druid-lookups-cached-global", "postgresql-metadata-storage"]'
- druid_metadata_storage_type: postgresql
Review comment:
as suggested, this whole section can go into `postgres/values.yaml` so existing users can still use postgres without changing the chart etc.
##########
File path: helm/druid/values.yaml
##########
@@ -26,32 +26,37 @@ configMap:
##
enabled: true
-## Define the key value pairs in the configmap
-configVars:
+## The configurations below would be set as environment variable across all pod
+globalConfig:
## DRUID env vars. ref: https://github.com/apache/druid/blob/master/distribution/docker/druid.sh#L29
- # DRUID_LOG_LEVEL: "warn"
- # DRUID_LOG4J: <?xml version="1.0" encoding="UTF-8" ?><Configuration status="WARN"><Appenders><Console name="Console" target="SYSTEM_OUT"><PatternLayout pattern="%d{ISO8601} %p [%t] %c - %m%n"/></Console></Appenders><Loggers><Root level="info"><AppenderRef ref="Console"/></Root><Logger name="org.apache.druid.jetty.RequestLog" additivity="false" level="DEBUG"><AppenderRef ref="Console"/></Logger></Loggers></Configuration>
- DRUID_USE_CONTAINER_IP: "true"
+ DRUID_USE_CONTAINER_IP: 'true'
## Druid Common Configurations. ref: https://druid.apache.org/docs/latest/configuration/index.html#common-configurations
- druid_extensions_loadList: '["druid-histogram", "druid-datasketches", "druid-lookups-cached-global", "postgresql-metadata-storage"]'
- druid_metadata_storage_type: postgresql
- druid_metadata_storage_connector_connectURI: jdbc:postgresql://postgres:5432/druid
- druid_metadata_storage_connector_user: druid
- druid_metadata_storage_connector_password: druid
+ druid_extensions_loadList: '["druid-datasketches", "druid-lookups-cached-global", "postgresql-metadata-storage"]'
Review comment:
what is the reason for removing `druid-histogram` from here?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] asdf2014 commented on a change in pull request #11427: helm chart fix/update
Posted by GitBox <gi...@apache.org>.
asdf2014 commented on a change in pull request #11427:
URL: https://github.com/apache/druid/pull/11427#discussion_r667346546
##########
File path: helm/druid/README.md
##########
@@ -25,20 +6,19 @@
To install the Druid Chart into your Kubernetes cluster :
-```bash
-helm install --namespace "druid" --name "druid" incubator/druid
-```
+1. `cd` to root directory of druid project
+2. execute command ```bash helm install --namespace "druid" --name "druid" helm/druid```
Review comment:
```suggestion
2. execute command `helm install --namespace "druid" --name "druid" helm/druid`
```
##########
File path: helm/druid/values.yaml
##########
@@ -26,32 +26,37 @@ configMap:
##
enabled: true
-## Define the key value pairs in the configmap
-configVars:
+## The configurations below would be set as environment variable across all pod
+globalConfig:
## DRUID env vars. ref: https://github.com/apache/druid/blob/master/distribution/docker/druid.sh#L29
- # DRUID_LOG_LEVEL: "warn"
- # DRUID_LOG4J: <?xml version="1.0" encoding="UTF-8" ?><Configuration status="WARN"><Appenders><Console name="Console" target="SYSTEM_OUT"><PatternLayout pattern="%d{ISO8601} %p [%t] %c - %m%n"/></Console></Appenders><Loggers><Root level="info"><AppenderRef ref="Console"/></Root><Logger name="org.apache.druid.jetty.RequestLog" additivity="false" level="DEBUG"><AppenderRef ref="Console"/></Logger></Loggers></Configuration>
- DRUID_USE_CONTAINER_IP: "true"
+ DRUID_USE_CONTAINER_IP: 'true'
## Druid Common Configurations. ref: https://druid.apache.org/docs/latest/configuration/index.html#common-configurations
- druid_extensions_loadList: '["druid-histogram", "druid-datasketches", "druid-lookups-cached-global", "postgresql-metadata-storage"]'
- druid_metadata_storage_type: postgresql
- druid_metadata_storage_connector_connectURI: jdbc:postgresql://postgres:5432/druid
- druid_metadata_storage_connector_user: druid
- druid_metadata_storage_connector_password: druid
+ druid_extensions_loadList: '["druid-datasketches", "druid-lookups-cached-global", "postgresql-metadata-storage"]'
+
+ druid_metadata_storage_type: derby
+
druid_storage_type: local
+
druid_indexer_logs_type: file
- druid_indexer_logs_directory: /opt/data/indexing-logs
## Druid Emitting Metrics. ref: https://druid.apache.org/docs/latest/configuration/index.html#emitting-metrics
druid_emitter: noop
- druid_emitter_logging_logLevel: debug
- druid_emitter_http_recipientBaseUrl: http://druid_exporter_url:druid_exporter_port/druid
+ druid_emitter_logging_logLevel: info
gCloudStorage:
enabled: false
secretName: google-cloud-key
+hdfs:
+ # Enable hdfs as deep-storage reference: http://druid.apache.org/docs/latest/development/extensions-core/hdfs.html
+ # Need to do:
+ # 1. enabled: true
+ # 2. add hdfs extension
Review comment:
```suggestion
# 2. add `"druid-hdfs-storage"` to `druid_extensions_loadList `
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org