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 2022/10/14 00:35:49 UTC

[GitHub] [druid] vtlim commented on a diff in pull request #13224: Collocated processes instructions

vtlim commented on code in PR #13224:
URL: https://github.com/apache/druid/pull/13224#discussion_r995211483


##########
docs/development/extensions-contrib/prometheus.md:
##########
@@ -31,6 +31,8 @@ This extension exposes [Druid metrics](https://druid.apache.org/docs/latest/oper
 
 Emitter is enabled by setting `druid.emitter=prometheus` [configs](https://druid.apache.org/docs/latest/configuration/index.html#emitting-metrics) or include `prometheus` in the composing emitter list. 

Review Comment:
   ```suggestion
   Emitter is enabled by setting `druid.emitter=prometheus` [configs](https://druid.apache.org/docs/latest/configuration/index.html#enabling-metrics) or include `prometheus` in the composing emitter list. 
   ```
   While we're updating this doc, let's fix the link to https://github.com/apache/druid/blob/master/docs/configuration/index.md#enabling-metrics



##########
docs/development/extensions-contrib/prometheus.md:
##########
@@ -31,6 +31,8 @@ This extension exposes [Druid metrics](https://druid.apache.org/docs/latest/oper
 
 Emitter is enabled by setting `druid.emitter=prometheus` [configs](https://druid.apache.org/docs/latest/configuration/index.html#emitting-metrics) or include `prometheus` in the composing emitter list. 
 
+Note that if you're colocating druid processes on the same host(such as broker/router, historical/middlemanager, coordinator/overlord), then the druid.emitter.prometheus.port would need to be separately specified in each of the runtime properties configuration for each host and be different between collocated processes. Additionally peons should emit metrics via push gateway. 

Review Comment:
   A few comments:
   * **Can we break up the first sentence for better readability. Something like:** In certain instances, Druid processes may be colocated on the same host. For example, the Broker and Router may share the same server. Other colocated processes include the Historical and MiddleManager or the Coordinator and Overlord.
   * **Code format for properties:** `druid.emitter.prometheus.port`
   * **Prefer active voice:** Specify `druid.emitter.prometheus.port` separately ...
   * **Is there a better transition between the colocation and peon tasks? Should these be in separate paragraph, or do they cover the same concept related to colocated processes?**
   
   Most of the above are in the following suggestion. 
   ```suggestion
   In certain instances, Druid processes may be colocated on the same host. For example, the Broker and Router may share the same server. Other colocated processes include the Historical and MiddleManager or the Coordinator and Overlord. When you have colocated processes, specify `druid.emitter.prometheus.port` separately for each process on each host. For example, even if the Broker and Router share the same host, the Broker runtime properties and the Router runtime properties each need to list `druid.emitter.prometheus.port`, and the port value for both must be different. Peon tasks should use `pushgateway` for `druid.emitter.prometheus.strategy`.
   ```



-- 
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