Skip to content

Conversation

coderzc
Copy link
Member

@coderzc coderzc commented Dec 23, 2021

  • update java version to 11
  • optimize cpu and memory resource limit
@coderzc coderzc force-pushed the update_jdk11 branch 7 times, most recently from a2ea0f1 to 826956e Compare December 23, 2021 09:12
@codecov
Copy link

codecov bot commented Dec 23, 2021

Codecov Report

Merging #170 (98e46fe) into master (bb0bbd0) will decrease coverage by 0.05%.
The diff coverage is 100.00%.

Impacted file tree graph

@@ Coverage Diff @@ ## master #170 +/- ## ============================================ - Coverage 87.34% 87.29% -0.06%  - Complexity 3051 3136 +85  ============================================ Files 320 332 +12 Lines 11432 11741 +309 Branches 993 1038 +45 ============================================ + Hits 9985 10249 +264  - Misses 980 991 +11  - Partials 467 501 +34 
Impacted Files Coverage Δ
...ugegraph/computer/core/config/ComputerOptions.java 100.00% <ø> (ø)
...va/com/baidu/hugegraph/computer/k8s/Constants.java 90.90% <ø> (ø)
...hugegraph/computer/core/network/TransportConf.java 94.33% <100.00%> (-0.58%) ⬇️
...r/k8s/operator/controller/ComputerJobDeployer.java 87.76% <100.00%> (+0.34%) ⬆️
...er/algorithm/path/rings/filter/FilterDescribe.java 76.47% <0.00%> (-11.77%) ⬇️
...puter/core/sort/sorting/AbstractInputsSorting.java 77.77% <0.00%> (-11.12%) ⬇️
...uter/core/combiner/MergeNewPropertiesCombiner.java 93.33% <0.00%> (-6.67%) ⬇️
...uter/core/combiner/MergeOldPropertiesCombiner.java 93.33% <0.00%> (-6.67%) ⬇️
...raph/computer/core/graph/vertex/DefaultVertex.java 72.00% <0.00%> (-4.00%) ⬇️
...k8s/operator/controller/ComputerJobController.java 85.92% <0.00%> (-1.77%) ⬇️
... and 67 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bb0bbd0...98e46fe. Read the comment docs.

corgiboygsj
corgiboygsj previously approved these changes Dec 23, 2021
@coderzc coderzc marked this pull request as draft December 23, 2021 10:37
javeme
javeme previously approved these changes Dec 23, 2021
with:
java-version: '8'
distribution: 'adopt'
distribution: 'zulu'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

need to also update to java-version: '11'?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it is need to compile hugegraph-server

@coderzc coderzc dismissed stale reviews from javeme and corgiboygsj via aa8c845 December 24, 2021 11:11

if (cpu != null) {
EnvVar cpuLimit = new EnvVarBuilder()
.withName(Constants.ENV_CPU_LIMIT)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

align with new


if (memory != null) {
EnvVar memoryLimit = new EnvVarBuilder()
.withName(Constants.ENV_MEMORY_LIMIT)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto

# avoid `Runtime.getRuntime().availableProcessors()` always return 1
if [ "${DRIVE}" = "${K8S_DRIVE}" && -z "${CPU_LIMIT}"]; then
CPU_COUNT="$(cat /proc/cpuinfo| grep "processor"| wc -l)"
JAVA="${JAVA} -XX:ActiveProcessorCount=${CPU_COUNT}"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

prefer to set to JAVA_OPTS

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

@coderzc coderzc marked this pull request as ready for review December 24, 2021 11:37

if (cpu != null) {
EnvVar cpuLimit = new EnvVarBuilder()
.withName(Constants.ENV_CPU_LIMIT)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add one more space

@coderzc coderzc force-pushed the update_jdk11 branch 5 times, most recently from 2bc6d2b to b8f3ffa Compare December 28, 2021 14:11
FROM mcr.microsoft.com/java/jre:11-zulu-ubuntu
LABEL maintainer="HugeGraph Docker Maintainers <hugegraph@googlegroups.com>"
ENV JAVA_OPTS="-XX:+UnlockExperimentalVMOptions -XX:+UseCGroupMemoryLimitForHeap -XX:MaxRAMFraction=2 -XshowSettings:vm"
ENV JAVA_OPTS="-XX:+UnlockExperimentalVMOptions -XX:+UseParallelGC -XX:+UseContainerSupport -XX:MaxRAMPercentage=80"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we add some comments why to choose ParallelGC instead of ZGC?

@coderzc coderzc force-pushed the update_jdk11 branch 3 times, most recently from ded9077 to b9ad7fe Compare January 6, 2022 10:40
if [ ${CPU_COUNT} -gt ${MAX_CPU_COUNT} ]; then
CPU_COUNT="$MAX_CPU_COUNT"
PROCESSOR_COUNT="$(cat /proc/cpuinfo| grep "processor"| wc -l)"
let MAX_PROCESSOR_COUNT=8
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the cpu number can't be greater than 8?

Copy link
Member Author

@coderzc coderzc Jan 11, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cpu number is too more will lead to huge number of GC thread
better not more than 8, if not specify cpu limit

let MAX_CPU_COUNT=8
if [ ${CPU_COUNT} -gt ${MAX_CPU_COUNT} ]; then
CPU_COUNT="$MAX_CPU_COUNT"
PROCESSOR_COUNT="$(cat /proc/cpuinfo| grep "processor"| wc -l)"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

expect a space before "|"

FROM openjdk:11-jre
LABEL maintainer="HugeGraph Docker Maintainers <hugegraph@googlegroups.com>"
ENV JAVA_OPTS="-XX:+UnlockExperimentalVMOptions -XX:+UseCGroupMemoryLimitForHeap -XX:MaxRAMFraction=2 -XshowSettings:vm"
# use ParallelGC is more friendly to olap system
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use ParallelGC which is more friendly to olap system

@coderzc coderzc changed the title update jdk version to 11 update java version to 11 Jan 18, 2022
@coderzc coderzc added the java11 label Jan 18, 2022
@javeme javeme merged commit 18bedd2 into master Jan 18, 2022
@javeme javeme deleted the update_jdk11 branch January 18, 2022 08:58
corgiboygsj pushed a commit that referenced this pull request Apr 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

4 participants