You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by GitBox <gi...@apache.org> on 2022/07/04 08:06:07 UTC

[GitHub] [pinot] gortiz opened a new pull request, #9012: Add k8s resource request by default

gortiz opened a new pull request, #9012:
URL: https://github.com/apache/pinot/pull/9012

   This PR adds an opinionated default resource requests to helm chart.
   
   As explained in #8944, the request in CPU is necessary. Otherwise the JVM by default will optimize its parallelism to only use a single CPU, which impacts in the GC but also in the different executors Pinot uses. The memory request is set to 1.25Gi to leave some extra space from the 1G value used by default in -xmx.
   
   Anyway, it should be recommended to customize these values on each environment, either by increasing the CPU resources or using `-XX=ActiveProcessorCount`


-- 
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@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


[GitHub] [pinot] xiangfu0 merged pull request #9012: Add k8s resource request by default

Posted by GitBox <gi...@apache.org>.
xiangfu0 merged PR #9012:
URL: https://github.com/apache/pinot/pull/9012


-- 
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@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


[GitHub] [pinot] gortiz commented on pull request #9012: Add k8s resource request by default

Posted by GitBox <gi...@apache.org>.
gortiz commented on PR #9012:
URL: https://github.com/apache/pinot/pull/9012#issuecomment-1186825898

   > maybe we can reduce this to 1? some older laptops don't have 8 cores.
   
   This is not going to be enforced by k8s. Instead, the OS is the one that is going to time slice between them. In most cases this isn't going to be a problem because, for example, when a query is being executed either the threads from the server or the ones from the broker will be active, but not both at the same time. If we decide to use only 1 thread then we would need to change the GC (doesn't make sense to use G1 with 1 thread).
   
   > I am not familiar with now minikube/microk8s maps CPUs to host, just to clarify are these 8 cores virtual or the ones that are physical (include hyperthreading)?
   
   TBH I'm also not sure. But remember that by using `-XX:ActiveProcessorCount` we do not depend on k8s now.
   
   
   


-- 
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@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


[GitHub] [pinot] walterddr commented on a diff in pull request #9012: Add k8s resource request by default

Posted by GitBox <gi...@apache.org>.
walterddr commented on code in PR #9012:
URL: https://github.com/apache/pinot/pull/9012#discussion_r914168777


##########
kubernetes/helm/pinot/values.yaml:
##########
@@ -305,7 +311,10 @@ server:
     nodePort: ""
     protocol: TCP
 
-  resources: {}
+  resources:
+    requests:
+      cpu: "2000m"
+      memory: "1.25Gi"

Review Comment:
   should we set different value for server/minion from broker/controller. the usage pattern is drastically different right?



##########
kubernetes/helm/pinot/values.yaml:
##########
@@ -457,7 +469,10 @@ minionStateless:
     protocol: TCP
     name: minion
 
-  resources: {}

Review Comment:
   can't comment below but we should also set zookeeper pod 



-- 
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@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


[GitHub] [pinot] gortiz commented on a diff in pull request #9012: Add k8s resource request by default

Posted by GitBox <gi...@apache.org>.
gortiz commented on code in PR #9012:
URL: https://github.com/apache/pinot/pull/9012#discussion_r921950770


##########
kubernetes/helm/pinot/values.yaml:
##########
@@ -457,7 +469,10 @@ minionStateless:
     protocol: TCP
     name: minion
 
-  resources: {}

Review Comment:
   I'm reading our chart values and it seems we have a section:
   ```
     ## Environmental variables to set in Zookeeper
     env:
       ## The JVM heap size to allocate to Zookeeper
       ZK_HEAP_SIZE: "256M"
   ```
   
   but that env is being ignored by bitnami zookeeper chart and in fact 1024 is being used :facepalm:. So I guess I can include the same request specified to the rest of the services



-- 
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@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


[GitHub] [pinot] gortiz commented on a diff in pull request #9012: Add k8s resource request by default

Posted by GitBox <gi...@apache.org>.
gortiz commented on code in PR #9012:
URL: https://github.com/apache/pinot/pull/9012#discussion_r921925967


##########
kubernetes/helm/pinot/values.yaml:
##########
@@ -305,7 +311,10 @@ server:
     nodePort: ""
     protocol: TCP
 
-  resources: {}
+  resources:
+    requests:
+      cpu: "2000m"
+      memory: "1.25Gi"

Review Comment:
   I understand that they are quite different, but these resource requests have been decided taking into account the Xmx and GC that is being used. As all broker, controllers and servers use the same Xmx (1G) and the same GC (G1) then all of them have the same requests.
   
   I may make sense to change the default JAVA_OPTS for each service, but I think that is out of the scope of this PR



-- 
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@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


[GitHub] [pinot] walterddr commented on a diff in pull request #9012: Add k8s resource request by default

Posted by GitBox <gi...@apache.org>.
walterddr commented on code in PR #9012:
URL: https://github.com/apache/pinot/pull/9012#discussion_r922268149


##########
kubernetes/helm/pinot/values.yaml:
##########
@@ -305,7 +311,10 @@ server:
     nodePort: ""
     protocol: TCP
 
-  resources: {}
+  resources:
+    requests:
+      cpu: "2000m"
+      memory: "1.25Gi"

Review Comment:
   sounds 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@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


[GitHub] [pinot] KKcorps commented on pull request #9012: Add k8s resource request by default

Posted by GitBox <gi...@apache.org>.
KKcorps commented on PR #9012:
URL: https://github.com/apache/pinot/pull/9012#issuecomment-1173827108

   There are other places as well where appropriate defaults are not defined such as Zookeeper. For this PR, we can limit the scope to CPU and memory only. Overall, LGTM!


-- 
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@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


[GitHub] [pinot] gortiz commented on a diff in pull request #9012: Add k8s resource request by default

Posted by GitBox <gi...@apache.org>.
gortiz commented on code in PR #9012:
URL: https://github.com/apache/pinot/pull/9012#discussion_r921933573


##########
kubernetes/helm/pinot/values.yaml:
##########
@@ -457,7 +469,10 @@ minionStateless:
     protocol: TCP
     name: minion
 
-  resources: {}

Review Comment:
   I don't know Zookeeper that well. How much memory/cpu would you recommend to use?



-- 
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@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


[GitHub] [pinot] walterddr commented on pull request #9012: Add k8s resource request by default

Posted by GitBox <gi...@apache.org>.
walterddr commented on PR #9012:
URL: https://github.com/apache/pinot/pull/9012#issuecomment-1185650703

   > 
   
   
   
   > I've decided to remove the cpu requests and specify instead `-XX=ActiveProcessorCount=2` in the default jvmOpts.
   
   maybe we can reduce this to 1? some older laptops don't have 8 cores.
   
   > I think this approach is better because the request approach consumed at least 8 CPUs. This means that, by default, the chart would not start in clusters with less than 8 CPUs in total, which can be very common in local k8s installations (like minikube or microk8s) and also in small aws machines.
   
   I am not familiar with now minikube/microk8s maps CPUs to host, just to clarify are these 8 cores virtual or the ones that are physical (include hyperthreading)?
   
   > * Nodes with few cpus will have installations that use quite more threads than cpus. This could introduce performance issues due to high context switching when several queries are being executed at the same time, but the system should run well enough to do some tests, which is the common use case of this kind of installations.
   
   I am less worried about running multiple queries at the same time, given that this setting is more for quickstart setups.
   
   


-- 
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@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


[GitHub] [pinot] gortiz commented on pull request #9012: Add k8s resource request by default

Posted by GitBox <gi...@apache.org>.
gortiz commented on PR #9012:
URL: https://github.com/apache/pinot/pull/9012#issuecomment-1185340366

   I've decided to remove the cpu requests and specify instead `-XX=ActiveProcessorCount=2` in the default jvmOpts.
   
   I think this approach is better because the request approach consumed at least 8 CPUs. This means that, by default, the chart would not start in clusters with less than 8 CPUs in total, which can be very common in local k8s installations (like minikube or microk8s) and also in small aws machines.
   
   Specifically, with this change:
   - Nodes with a lot of cpus will not be affected. Anyway, in this case it is expected that a custom `-XX=ActiveProcessorCount` will be used to determine how many threads each container will use.
   - Nodes with few cpus will have installations that use quite more threads than cpus. This could introduce performance issues due to high context switching when several queries are being executed at the same time, but the system should run well enough to do some tests, which is the target of this kind of installations.


-- 
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@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org