You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@rocketmq.apache.org by GitBox <gi...@apache.org> on 2021/08/25 06:22:30 UTC
[GitHub] [rocketmq-operator] overstep123 opened a new pull request #81: fix cpu and mem in docker for Linux
overstep123 opened a new pull request #81:
URL: https://github.com/apache/rocketmq-operator/pull/81
fix nameservice to get correct cpu and mem resource from cgroups, and fix the KEY url in the nameservice Dockerfile.
--
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: dev-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [rocketmq-operator] overstep123 commented on a change in pull request #81: fix cpu and mem in docker for Linux
Posted by GitBox <gi...@apache.org>.
overstep123 commented on a change in pull request #81:
URL: https://github.com/apache/rocketmq-operator/pull/81#discussion_r700787915
##########
File path: images/namesrv/alpine/Dockerfile
##########
@@ -34,7 +34,7 @@ RUN set -eux; \
apk add --virtual .build-deps curl gnupg unzip; \
curl https://archive.apache.org/dist/rocketmq/${ROCKETMQ_VERSION}/rocketmq-all-${ROCKETMQ_VERSION}-bin-release.zip -o rocketmq.zip; \
curl https://archive.apache.org/dist/rocketmq/${ROCKETMQ_VERSION}/rocketmq-all-${ROCKETMQ_VERSION}-bin-release.zip.asc -o rocketmq.zip.asc; \
- curl https://www.apache.org/dist/rocketmq/KEYS -o KEYS; \
+ curl https://downloads.apache.org/rocketmq/KEYS -o KEYS; \
Review comment:
this is the result of the old url:
--
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: dev-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [rocketmq-operator] overstep123 commented on a change in pull request #81: fix cpu and mem in docker for Linux
Posted by GitBox <gi...@apache.org>.
overstep123 commented on a change in pull request #81:
URL: https://github.com/apache/rocketmq-operator/pull/81#discussion_r700967728
##########
File path: images/namesrv/alpine/runserver-customize.sh
##########
@@ -54,8 +54,8 @@ calculate_heap_sizes()
{
case "`uname`" in
Linux)
- system_memory_in_mb=`free -m| sed -n '2p' | awk '{print $2}'`
- system_cpu_cores=`egrep -c 'processor([[:space:]]+):.*' /proc/cpuinfo`
+ system_memory_in_mb=$(($(cat /sys/fs/cgroup/memory/memory.limit_in_bytes)/1024/1024))
+ system_cpu_cores=$(($(cat /sys/fs/cgroup/cpu/cpu.cfs_quota_us)/100000))
Review comment:
This is better,and I fixed it.Although i think it's not correct if anone use it in the kubernetes without memory limit.
--
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: dev-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [rocketmq-operator] overstep123 commented on a change in pull request #81: fix cpu and mem in docker for Linux
Posted by GitBox <gi...@apache.org>.
overstep123 commented on a change in pull request #81:
URL: https://github.com/apache/rocketmq-operator/pull/81#discussion_r708821847
##########
File path: images/namesrv/alpine/runserver-customize.sh
##########
@@ -55,7 +55,15 @@ calculate_heap_sizes()
case "`uname`" in
Linux)
system_memory_in_mb=`free -m| sed -n '2p' | awk '{print $2}'`
+ system_memory_in_mb_in_docker=$(($(cat /sys/fs/cgroup/memory/memory.limit_in_bytes)/1024/1024))
+ if [ $system_memory_in_mb_in_docker -lt $system_memory_in_mb ];then
+ system_memory_in_mb=$system_memory_in_mb_in_docker
+ fi
system_cpu_cores=`egrep -c 'processor([[:space:]]+):.*' /proc/cpuinfo`
+ system_cpu_cores_in_docker=$(($(cat /sys/fs/cgroup/cpu/cpu.cfs_quota_us)/100000))
Review comment:
that's right,and fixed
--
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: dev-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [rocketmq-operator] duhenglucky merged pull request #81: [Issue 84]fix cpu and mem in docker for Linux
Posted by GitBox <gi...@apache.org>.
duhenglucky merged pull request #81:
URL: https://github.com/apache/rocketmq-operator/pull/81
--
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: dev-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [rocketmq-operator] caigy commented on a change in pull request #81: fix cpu and mem in docker for Linux
Posted by GitBox <gi...@apache.org>.
caigy commented on a change in pull request #81:
URL: https://github.com/apache/rocketmq-operator/pull/81#discussion_r700169437
##########
File path: images/namesrv/alpine/Dockerfile
##########
@@ -34,7 +34,7 @@ RUN set -eux; \
apk add --virtual .build-deps curl gnupg unzip; \
curl https://archive.apache.org/dist/rocketmq/${ROCKETMQ_VERSION}/rocketmq-all-${ROCKETMQ_VERSION}-bin-release.zip -o rocketmq.zip; \
curl https://archive.apache.org/dist/rocketmq/${ROCKETMQ_VERSION}/rocketmq-all-${ROCKETMQ_VERSION}-bin-release.zip.asc -o rocketmq.zip.asc; \
- curl https://www.apache.org/dist/rocketmq/KEYS -o KEYS; \
+ curl https://downloads.apache.org/rocketmq/KEYS -o KEYS; \
Review comment:
Could you provide the reason to change it? It seems that this modification is not related to the title
##########
File path: pkg/controller/broker/broker_controller.go
##########
@@ -375,14 +375,43 @@ func getBrokerName(broker *rocketmqv1alpha1.Broker, brokerGroupIndex int) string
// getBrokerStatefulSet returns a broker StatefulSet object
func (r *ReconcileBroker) getBrokerStatefulSet(broker *rocketmqv1alpha1.Broker, brokerGroupIndex int, replicaIndex int) *appsv1.StatefulSet {
- ls := labelsForBroker(broker.Name)
+ ls := labelsForBrokerWithRole(broker.Name,replicaIndex)
var a int32 = 1
var c = &a
var statefulSetName string
+ var podAntiAffinity *corev1.PodAntiAffinity
if replicaIndex == 0 {
statefulSetName = broker.Name + "-" + strconv.Itoa(brokerGroupIndex) + "-master"
+ podAntiAffinity = &corev1.PodAntiAffinity{
+ RequiredDuringSchedulingIgnoredDuringExecution: []corev1.PodAffinityTerm{corev1.PodAffinityTerm{
+ TopologyKey: "kubernetes.io/hostname",
+ LabelSelector: &metav1.LabelSelector{
+ MatchLabels: ls,
+ },
+ }},
+ PreferredDuringSchedulingIgnoredDuringExecution: []corev1.WeightedPodAffinityTerm{corev1.WeightedPodAffinityTerm{
+ Weight: 100,
+ PodAffinityTerm: corev1.PodAffinityTerm{
+ TopologyKey: "kubernetes.io/hostname",
+ LabelSelector: &metav1.LabelSelector{
+ MatchLabels: labelsForBroker(broker.Name),
+ },
+ },
+ }},
+ }
Review comment:
Changes should be related to this PR. Also, podAntiAffinity feature is discussed in #38 with PR #75, you can join it and work together to implement this feature.
##########
File path: images/namesrv/alpine/runserver-customize.sh
##########
@@ -54,8 +54,8 @@ calculate_heap_sizes()
{
case "`uname`" in
Linux)
- system_memory_in_mb=`free -m| sed -n '2p' | awk '{print $2}'`
- system_cpu_cores=`egrep -c 'processor([[:space:]]+):.*' /proc/cpuinfo`
+ system_memory_in_mb=$(($(cat /sys/fs/cgroup/memory/memory.limit_in_bytes)/1024/1024))
+ system_cpu_cores=$(($(cat /sys/fs/cgroup/cpu/cpu.cfs_quota_us)/100000))
Review comment:
It would be better to check memory and cpu from cgroup are smaller than system memory and cpu, because resource limits may not be set.
--
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: dev-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [rocketmq-operator] caigy commented on a change in pull request #81: fix cpu and mem in docker for Linux
Posted by GitBox <gi...@apache.org>.
caigy commented on a change in pull request #81:
URL: https://github.com/apache/rocketmq-operator/pull/81#discussion_r702388943
##########
File path: images/namesrv/alpine/Dockerfile
##########
@@ -34,7 +34,7 @@ RUN set -eux; \
apk add --virtual .build-deps curl gnupg unzip; \
curl https://archive.apache.org/dist/rocketmq/${ROCKETMQ_VERSION}/rocketmq-all-${ROCKETMQ_VERSION}-bin-release.zip -o rocketmq.zip; \
curl https://archive.apache.org/dist/rocketmq/${ROCKETMQ_VERSION}/rocketmq-all-${ROCKETMQ_VERSION}-bin-release.zip.asc -o rocketmq.zip.asc; \
- curl https://www.apache.org/dist/rocketmq/KEYS -o KEYS; \
+ curl https://downloads.apache.org/rocketmq/KEYS -o KEYS; \
Review comment:
Ok, this is indeed a problem. I think you can use `curl -L` to solve the redirect problem, which is the same as that in rocketmq-docker repo.
--
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: dev-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [rocketmq-operator] overstep123 commented on a change in pull request #81: fix cpu and mem in docker for Linux
Posted by GitBox <gi...@apache.org>.
overstep123 commented on a change in pull request #81:
URL: https://github.com/apache/rocketmq-operator/pull/81#discussion_r700984944
##########
File path: pkg/controller/broker/broker_controller.go
##########
@@ -375,14 +375,43 @@ func getBrokerName(broker *rocketmqv1alpha1.Broker, brokerGroupIndex int) string
// getBrokerStatefulSet returns a broker StatefulSet object
func (r *ReconcileBroker) getBrokerStatefulSet(broker *rocketmqv1alpha1.Broker, brokerGroupIndex int, replicaIndex int) *appsv1.StatefulSet {
- ls := labelsForBroker(broker.Name)
+ ls := labelsForBrokerWithRole(broker.Name,replicaIndex)
var a int32 = 1
var c = &a
var statefulSetName string
+ var podAntiAffinity *corev1.PodAntiAffinity
if replicaIndex == 0 {
statefulSetName = broker.Name + "-" + strconv.Itoa(brokerGroupIndex) + "-master"
+ podAntiAffinity = &corev1.PodAntiAffinity{
+ RequiredDuringSchedulingIgnoredDuringExecution: []corev1.PodAffinityTerm{corev1.PodAffinityTerm{
+ TopologyKey: "kubernetes.io/hostname",
+ LabelSelector: &metav1.LabelSelector{
+ MatchLabels: ls,
+ },
+ }},
+ PreferredDuringSchedulingIgnoredDuringExecution: []corev1.WeightedPodAffinityTerm{corev1.WeightedPodAffinityTerm{
+ Weight: 100,
+ PodAffinityTerm: corev1.PodAffinityTerm{
+ TopologyKey: "kubernetes.io/hostname",
+ LabelSelector: &metav1.LabelSelector{
+ MatchLabels: labelsForBroker(broker.Name),
+ },
+ },
+ }},
+ }
Review comment:
I have no idea why the nodeAffinity is needed. But the podAntiAffinity is sure needed. Because I don't want to see that the whole cluster(maybe multi master borker) become unwritable when all the master broker are assigned to the same node and the node is down. This situation will be worse if the kubernetes can't new the master pod in time.
--
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: dev-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [rocketmq-operator] caigy commented on a change in pull request #81: fix cpu and mem in docker for Linux
Posted by GitBox <gi...@apache.org>.
caigy commented on a change in pull request #81:
URL: https://github.com/apache/rocketmq-operator/pull/81#discussion_r702390806
##########
File path: pkg/controller/broker/broker_controller.go
##########
@@ -375,14 +375,43 @@ func getBrokerName(broker *rocketmqv1alpha1.Broker, brokerGroupIndex int) string
// getBrokerStatefulSet returns a broker StatefulSet object
func (r *ReconcileBroker) getBrokerStatefulSet(broker *rocketmqv1alpha1.Broker, brokerGroupIndex int, replicaIndex int) *appsv1.StatefulSet {
- ls := labelsForBroker(broker.Name)
+ ls := labelsForBrokerWithRole(broker.Name,replicaIndex)
var a int32 = 1
var c = &a
var statefulSetName string
+ var podAntiAffinity *corev1.PodAntiAffinity
if replicaIndex == 0 {
statefulSetName = broker.Name + "-" + strconv.Itoa(brokerGroupIndex) + "-master"
+ podAntiAffinity = &corev1.PodAntiAffinity{
+ RequiredDuringSchedulingIgnoredDuringExecution: []corev1.PodAffinityTerm{corev1.PodAffinityTerm{
+ TopologyKey: "kubernetes.io/hostname",
+ LabelSelector: &metav1.LabelSelector{
+ MatchLabels: ls,
+ },
+ }},
+ PreferredDuringSchedulingIgnoredDuringExecution: []corev1.WeightedPodAffinityTerm{corev1.WeightedPodAffinityTerm{
+ Weight: 100,
+ PodAffinityTerm: corev1.PodAffinityTerm{
+ TopologyKey: "kubernetes.io/hostname",
+ LabelSelector: &metav1.LabelSelector{
+ MatchLabels: labelsForBroker(broker.Name),
+ },
+ },
+ }},
+ }
Review comment:
You'b better open another PR to do it, because this is obviously unrelated to this issue.
--
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: dev-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [rocketmq-operator] caigy commented on a change in pull request #81: fix cpu and mem in docker for Linux
Posted by GitBox <gi...@apache.org>.
caigy commented on a change in pull request #81:
URL: https://github.com/apache/rocketmq-operator/pull/81#discussion_r702441047
##########
File path: images/namesrv/alpine/runserver-customize.sh
##########
@@ -55,7 +55,15 @@ calculate_heap_sizes()
case "`uname`" in
Linux)
system_memory_in_mb=`free -m| sed -n '2p' | awk '{print $2}'`
+ system_memory_in_mb_in_docker=$(($(cat /sys/fs/cgroup/memory/memory.limit_in_bytes)/1024/1024))
+ if [ $system_memory_in_mb_in_docker -lt $system_memory_in_mb ];then
+ system_memory_in_mb=$system_memory_in_mb_in_docker
+ fi
system_cpu_cores=`egrep -c 'processor([[:space:]]+):.*' /proc/cpuinfo`
+ system_cpu_cores_in_docker=$(($(cat /sys/fs/cgroup/cpu/cpu.cfs_quota_us)/100000))
Review comment:
Is `100000` read from `/sys/fs/cgroup/cpu/cpu.cfs_period_us` ?
--
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: dev-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [rocketmq-operator] overstep123 commented on a change in pull request #81: fix cpu and mem in docker for Linux
Posted by GitBox <gi...@apache.org>.
overstep123 commented on a change in pull request #81:
URL: https://github.com/apache/rocketmq-operator/pull/81#discussion_r708821847
##########
File path: images/namesrv/alpine/runserver-customize.sh
##########
@@ -55,7 +55,15 @@ calculate_heap_sizes()
case "`uname`" in
Linux)
system_memory_in_mb=`free -m| sed -n '2p' | awk '{print $2}'`
+ system_memory_in_mb_in_docker=$(($(cat /sys/fs/cgroup/memory/memory.limit_in_bytes)/1024/1024))
+ if [ $system_memory_in_mb_in_docker -lt $system_memory_in_mb ];then
+ system_memory_in_mb=$system_memory_in_mb_in_docker
+ fi
system_cpu_cores=`egrep -c 'processor([[:space:]]+):.*' /proc/cpuinfo`
+ system_cpu_cores_in_docker=$(($(cat /sys/fs/cgroup/cpu/cpu.cfs_quota_us)/100000))
Review comment:
that's right
--
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: dev-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [rocketmq-operator] caigy commented on pull request #81: fix cpu and mem in docker for Linux
Posted by GitBox <gi...@apache.org>.
caigy commented on pull request #81:
URL: https://github.com/apache/rocketmq-operator/pull/81#issuecomment-921407899
@overstep123 Pls link the PR to related issue and rename the title starting with '[ISSUE xx]'.
--
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: dev-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org