-
Notifications
You must be signed in to change notification settings - Fork 3.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[deployement] make kubernetes yamls for aws operational #6192
Conversation
PULSAR_PREFIX_zkServers: zk-0.zookeeper,zk-1.zookeeper,zk-2.zookeeper | ||
PULSAR_PREFIX_statsProviderClass: org.apache.bookkeeper.stats.prometheus.PrometheusMetricsProvider | ||
PULSAR_PREFIX_useHostNameAsBookieID: "true" | ||
BOOKIE_MEM: "\"-Dio.netty.leakDetectionLevel=disabled -Dio.netty.recycler.linkCapacity=1024 -XX:+UseG1GC -XX:MaxGCPauseMillis=10 -XX:+ParallelRefProcEnabled -XX:+UnlockExperimentalVMOptions -XX:+AggressiveOpts -XX:+DoEscapeAnalysis -XX:ParallelGCThreads=32 -XX:ConcGCThreads=32 -XX:G1NewSizePercent=50 -XX:+DisableExplicitGC -XX:-ResizePLAB -XX:+ExitOnOutOfMemoryError -XX:+PerfDisableSharedMem -XX:+PrintGCDetails -XX:+PrintGCTimeStamps -XX:+PrintGCApplicationStoppedTime -XX:+PrintHeapAtGC -verbosegc -XX:G1LogLevel=finest -Xms28g -Xmx28g -XX:MaxDirectMemorySize=28g\"" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should also take out the garbage collector settings and put them into BOOKIE_GC
. You would also need to add bin/apply-config-from-env.py conf/bkenv.sh &&
in the command section if you want these changes to take effect.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@roman-popenov What should the BOKKIE_GC be set to? I just copied BOOKIE_MEM from PULSAR_MEM in this file.
And, are you sure that bkenv.sh is needed, as just setting BOOKIE_MEM worked fine for me (bookie didn't start before I set it)? If it is needed should it be the first one called?
In addition, do you think that for the bookie-autorecovery in the same yaml the PULSAR_MEM and PULSAR_GC should be changed to BOOKIE_* as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You simply would be taking out -XX:+UseG1GC -XX:MaxGCPauseMillis=10 of the BOOKIE_MEM and putting it into BOOKIE_GC like so:
BOOKIE_MEM: "\"-Xms15g -Xmx15g -XX:MaxDirectMemorySize=15g -Dio.netty.leakDetectionLevel=disabled -Dio.netty.recycler.linkCapacity=1024 -XX:+ParallelRefProcEnabled -XX:+UnlockExperimentalVMOptions -XX:+AggressiveOpts -XX:+DoEscapeAnalysis -XX:ParallelGCThreads=32 -XX:ConcGCThreads=32 -XX:G1NewSizePercent=50 -XX:+DisableExplicitGC -XX:-ResizePLAB -XX:+ExitOnOutOfMemoryError -XX:+PerfDisableSharedMem -XX:+PrintGCDetails -XX:+PrintGCTimeStamps -XX:+PrintGCApplicationStoppedTime -XX:+PrintHeapAtGC -verbosegc -XX:G1LogLevel=finest\""
BOOKIE_GC: "\"-XX:+UseG1GC -XX:MaxGCPauseMillis=10\""
bookie-autorecovery
does run bin/apply-config-from-env.py conf/bookkeeper.conf
so you would need to check which settings it takes and which config file it populates, but I suspect you might be right about this one as well. @sijie can you confirm?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will update soon after I test it.
In any case, "bin/apply-config-from-env.py conf/bkenv.sh" is definitely not needed (more so even bin/apply-config-from-env.py conf/pulsar_env.sh is not needed) as apply-config-from-env.py always updates the two sh files.
# Always apply env config to env scripts as well
conf_files = ['conf/pulsar_env.sh', 'conf/bkenv.sh'] + sys.argv[1:]
PF_ENV_PREFIX = 'PULSAR_PREFIX_'
for conf_filename in conf_files:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@roman-popenov updated the file to set BOOKIE_MEM and BOOKIE_GC correctly both for bookie and for the autorecovery (validated that they are applied with jinfo)
@trexinc thank you for your contribution! |
@sijie can we cherry-pick this fix on older versions? 2.5.x, 2.4.x as well ? |
@psilos labeled for 2.5.1 and 2.4.3. The release manager will do the cherry-pick when creating the bug fix releases. |
### Motivation Supplied Kubernetes yaml's for AWS are outdated and just don't work. ### Modifications Update yaml files and so that appying them on AWS EKS will actually set up a working Pulsar environment. ### Verifying this change This change is a trivial rework / code cleanup without any test coverage. (cherry picked from commit d631156)
### Motivation Supplied Kubernetes yaml's for AWS are outdated and just don't work. ### Modifications Update yaml files and so that appying them on AWS EKS will actually set up a working Pulsar environment. ### Verifying this change This change is a trivial rework / code cleanup without any test coverage. (cherry picked from commit d631156)
### Motivation Supplied Kubernetes yaml's for AWS are outdated and just don't work. ### Modifications Update yaml files and so that appying them on AWS EKS will actually set up a working Pulsar environment. ### Verifying this change This change is a trivial rework / code cleanup without any test coverage. (cherry picked from commit d631156)
### Motivation Supplied Kubernetes yaml's for AWS are outdated and just don't work. ### Modifications Update yaml files and so that appying them on AWS EKS will actually set up a working Pulsar environment. ### Verifying this change This change is a trivial rework / code cleanup without any test coverage.
Motivation
Supplied Kubernetes yaml's for AWS are outdated and just don't work.
Modifications
Update yaml files and so that appying them on AWS EKS will actually set up a working Pulsar environment.
Verifying this change
This change is a trivial rework / code cleanup without any test coverage.
Does this pull request potentially affect one of the following parts:
Documentation
How to apply the yaml's is already documented, it just didn't work.