Skip to content
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

Jenkins memory usage tuning documentation #6981

Merged

Conversation

jim-minter
Copy link

No description provided.

@openshift-ci-robot openshift-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jan 2, 2018
@jim-minter
Copy link
Author

@bparees all this is just preparing the ground, but I'd appreciate feedback as I'm intending to build on it.

Copy link
Contributor

@bparees bparees left a comment

Choose a reason for hiding this comment

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

It's a fine start for documenting the knobs. Regarding discussing how the variables interact (e.g. container_heap_percent, java_max_heap, and jenkins_java_options), I think rather than discussing it within each variable description, it would be good to have an entire section dedicated to discussing how those variables interact/override each other/etc.


* `JENKINS_JAVA_OPTIONS`
+
Specifies additional options for the Jenkins JVM which may be overridden.
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this needs more details. If the user sets this, they are replacing all the JVM options we auto-configured and the only JVM options Jenkins will get are whatever they set in this variable (meaning they must explicitly set max heap and anything else they care about):
https://github.com/openshift/jenkins/blob/master/2/contrib/s2i/run#L320-L322

Also not clear what "which may be overridden" means. Overridden by who/what, and how are they overridden?

"false".
* `JENKINS_JAVA_OVERRIDES`
+
Specifies additional options for the Jenkins JVM which may not be overridden.
Copy link
Contributor

Choose a reason for hiding this comment

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

I think what really needs to be said here is that these JVM options will appear last in the JVM arguments, meaning they will/can override arguments provided earlier (such as via the automatically generated arguments that appear in JENKINS_JAVA_OPTIONS, or the arguments set by the user in JENKINS_JAVA_OPTIONS if they chose to set it explicitly).

Also worth distinguishing that unlike JENKINS_JAVA_OPTIONS, there is no "default/automatic" value for JENKINS_JAVA_OVERRIDES so if they choose to set this value, they are not erasing any arguments the image would have normally supplied. (They may, however, be overriding values the image is trying to supply).

* `JAVA_MAX_HEAP_PARAM`
+
If set, entirely overrides the Jenkins JVM maximum heap size (-Xmx). Example
setting: `-Xmx512m`.
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 discuss how this interacts with JENKINS_JAVA_OPTIONS and JENKINS_JAVA_OVERRIDES. And CONTAINER_HEAP_PERCENT.

* `CONTAINER_HEAP_PERCENT` (default: `0.5`, i.e. 50%)
+
Specifies the percentage of the container memory limit allocated to the Jenkins
JVM maximum heap size (-Xmx).
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 discuss how this interacts with JAVA_MAX_HEAP_PARAM, JENKINS_JAVA_OPTIONS, and JENKINS_JAVA_OVERRIDES.

* `JAVA_INITIAL_HEAP_PARAM`
+
If set, entirely overrides the Jenkins JVM initial heap size (-Xms). Example
setting: `-Xms32m`.
Copy link
Contributor

Choose a reason for hiding this comment

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

same comments as for JAVA_MAX_HEAP_PARAM

+
If set, specifies the Jenkins JVM initial heap size (-Xms) as a percentage of
the dynamically calculated Jenkins JVM maximum heap size value. Example
setting: `0.1`, i.e. 10%.
Copy link
Contributor

Choose a reason for hiding this comment

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

same comments as CONTAINER_HEAP_PERCENT

* `JAVA_GC_OPTS` (default: `-XX:+UseParallelGC -XX:MinHeapFreeRatio=5 -XX:MaxHeapFreeRatio=10 -XX:GCTimeRatio=4 -XX:AdaptiveSizePolicyWeight=90`)
+
Specifies Jenkins JVM garbage collection parameters. It is not recommended to
override this.
Copy link
Contributor

Choose a reason for hiding this comment

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

discuss how this interacts with JENKINS_JAVA_OPTIONS and JENKINS_JAVA_OVERRIDES.

They are available by default in the Jenkins image.

[[jenkins-pipeline-dsl-plug-in]]
=== {product-title} Jenkins Pipeline DSL Plug-in
Copy link
Contributor

Choose a reason for hiding this comment

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

we tend to call this the Jenkins OpenShift Client Plug-in when discussing it, i'd prefer to call it that here. (though I see the github repo doesn't reflect that). But it is what it's indexed as:
https://wiki.jenkins.io/display/JENKINS/OpenShift+Client+Plugin

So we should just fix the readme in the repo. (maybe @gabemontero can take care of that).

@jim-minter jim-minter force-pushed the trello143-jenkins-memory-usage-tuning branch 2 times, most recently from 809abd8 to 396d0b6 Compare January 5, 2018 00:15
@openshift-ci-robot openshift-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jan 5, 2018
@jim-minter jim-minter force-pushed the trello143-jenkins-memory-usage-tuning branch from 396d0b6 to 7bf0da2 Compare January 9, 2018 00:59
@openshift-ci-robot openshift-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Jan 9, 2018

- The memory request value, if specified, influences the {product-title} scheduler.
The scheduler considers the memory request when scheduling a container to a
node, then ring-fences the requested memory on the chosen node for the use of
Copy link
Contributor

Choose a reason for hiding this comment

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

"ring-fences"? probably too technical a term here.

Copy link
Author

Choose a reason for hiding this comment

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

Can you think of a better term?

Copy link
Contributor

Choose a reason for hiding this comment

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

exclusively allocates?

Copy link
Author

Choose a reason for hiding this comment

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

I was trying to avoid "allocates" as it clashes with the idea of memory being actually allocated from the operating system.

Copy link
Contributor

Choose a reason for hiding this comment

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

"ensures the availability of"


- If the memory allocated by all of the processes in a container
exceeds the memory limit, the node OOM killer will immediately select
and kill a process.
Copy link
Contributor

Choose a reason for hiding this comment

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

kill a process in the container.

+
This
option involves hard-coding a value, but has the advantage of allowing
a safety margin to be calculated
Copy link
Contributor

Choose a reason for hiding this comment

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

missing period

return heap memory to the operating system whenever allocated memory
exceeds 110% of in-use memory (`-XX:MaxHeapFreeRatio`), spending up to
20% of CPU time in the garbage collector (`-XX:GCTimeRatio`). Detailed
additional information is available
Copy link
Contributor

Choose a reason for hiding this comment

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

doesn't min heapsize also play a role in this? (if min heap==max heap, i don't think anything will be released, for example because the jvm won't try to compact the heap)

Copy link
Author

Choose a reason for hiding this comment

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

Yes, will update. I'm still not 100% convinced we've got the right GC arguments but am not going to let it hold up this doc.


Many Java tools use different environment variables (`JAVA_OPTS`,
`GRADLE_OPTS`, `MAVEN_OPTS`, etc.) to configure their JVMs and it can be
bewildering to ensure that the right settings are being passed to the
Copy link
Contributor

Choose a reason for hiding this comment

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

while i appreciate the sentiment, describing something as "bewildering" probably doesn't belong in technical documentation :)

Copy link
Author

Choose a reason for hiding this comment

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

Let's try "challenging" :)

Copy link
Contributor

Choose a reason for hiding this comment

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

works for me.

endif::[]

[[using-the-jenkins-kubernetes-plug-in]]
=== Using the Jenkins Kubernetes Plug-in
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

or some refactoring between the two at least.

Copy link
Author

Choose a reason for hiding this comment

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

I agree. Cop out perhaps, but I'm inclined to write a trello card to handle that. What I've written here is specifically kubernetes plugin syntax which IMO is sufficiently fiddly that it desperately needs written examples. https://docs.openshift.org/latest/dev_guide/dev_tutorials/openshift_pipeline.html is a openshift (DSL) client plugin walkthrough which doesn't use the kubernetes plugin. Then there's a separate pipeline plugin walkthrough at https://github.com/openshift/origin/blob/master/examples/jenkins/README.md. All three need rationalisation.

Copy link
Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

ok.

Copy link
Author

Choose a reason for hiding this comment

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

=== Memory Requirements

The default memory limit for the Jenkins container is 512MiB, regardless of
whether the Jenkins Ephemeral or Jenkins Persistent template is being used.
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd rephrase this as: "When deployed via the provided Jenkins Template (both ephemeral and persistent), the default memory limit is 512MiB"

To make it clear up front this is just talking about the template behavior, not the image.

[[tutorial]]
== Tutorial
=== Tutorial
Copy link
Contributor

Choose a reason for hiding this comment

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

this section doesn't feel like a tutorial.

Copy link
Author

Choose a reason for hiding this comment

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

Agreed - that's what was there before, though. Think I'll rename it as information about the sample job.

@jim-minter jim-minter force-pushed the trello143-jenkins-memory-usage-tuning branch from 7bf0da2 to 79f6ead Compare January 11, 2018 18:54
@jim-minter
Copy link
Author

Overview written and all requested updates made, ptal.

@jim-minter jim-minter changed the title [WIP] Jenkins memory usage tuning documentation Jenkins memory usage tuning documentation Jan 11, 2018
@openshift-ci-robot openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jan 11, 2018
@bparees
Copy link
Contributor

bparees commented Jan 11, 2018

lgtm

@bmcelvee
Copy link
Contributor

@jim-minter LGTM. There are just a few minor edits I'd like to make, but I'll open a follow-up PR for those. Would you mind squashing your commits? Thanks! :)

…emory

add application memory sizing document
@jim-minter jim-minter force-pushed the trello143-jenkins-memory-usage-tuning branch from 79f6ead to 502e487 Compare January 12, 2018 18:58
@jim-minter
Copy link
Author

Thanks @bmcelvee. Squashed!

@bmcelvee
Copy link
Contributor

[rev_history]
|xref:../dev_guide/application_memory_sizing.adoc#dev-guide-application-memory-sizing[Application memory sizing]
|Added xref:../dev_guide/application_memory_sizing.adoc#dev-guide-application-memory-sizing[Application memory sizing] chapter.
%
|xref:../dev_guide/builds/build_strategies.adoc#dev-guide-build-strategy-options[Build Strategy Options]
|Updated example in the xref:../dev_guide/builds/build_strategies.adoc#jenkinsfile[Providing the Jenkinsfile] section.
%
|xref:../using_images/other_images/jenkins.adoc#using-images-other-images-jenkins[Jenkins]
|Updated the xref:../using_images/other_images/jenkins.adoc#using-images-other-images-jenkins[Jenkins] chapter.
%
|xref:../using_images/other_images/jenkins_slaves.adoc#using-images-other-images-jenkins-slaves[Jenkins Slaves]
|Added xref:../using_images/other_images/jenkins_slaves.adoc#using-images-other-images-jenkins-slaves[Jenkins Slaves] chapter.
%

@bmcelvee bmcelvee merged commit 85eefba into openshift:master Jan 12, 2018
bmcelvee pushed a commit to bmcelvee/openshift-docs that referenced this pull request Jan 12, 2018
…d env vars and memory

add application memory sizing document

(cherry picked from commit 502e487) xref:openshift#6981
bmcelvee pushed a commit to bmcelvee/openshift-docs that referenced this pull request Jan 12, 2018
…nd env vars and memory

add application memory sizing document

(cherry picked from commit 502e487) xref:openshift#6981
bmcelvee pushed a commit to bmcelvee/openshift-docs that referenced this pull request Jan 12, 2018
…nv vars and memory

add application memory sizing document

(cherry picked from commit 502e487) xref:openshift#6981
@bmcelvee bmcelvee removed this from the Future Release milestone Jan 12, 2018
@vikram-redhat
Copy link
Contributor

@jim-minter @bmcelvee - I made some changes to the jenkins.adoc file due to some conflicts that were unresolved. I want to make sure that the file contains content correct for 3.9 and 3.7 respectively.

Could you please review them here for 3.9:
https://raw.githubusercontent.com/openshift/openshift-docs/enterprise-3.9/using_images/other_images/jenkins.adoc

And here for 3.7:
https://raw.githubusercontent.com/openshift/openshift-docs/enterprise-3.7/using_images/other_images/jenkins.adoc

@bmcelvee
Copy link
Contributor

@vikram-redhat Is this a content leak into 3.7? I didn't have conflicts on this PR or the follow-up PR when I was working on them, and I don't see anything in the blame for 3.7 or 3.9. Do you have a commit or PR that I can review for more information?

Thanks!

@vikram-redhat
Copy link
Contributor

Hey @bmcelvee not a content leak, but unresolved conflict resolution. See here: https://github.com/openshift/openshift-docs/pull/7126/files#r170095649.

I have fixed and pushed the changes, so if you could confirm that the changes that I did for both 3.7 and 3.9 branches are ok.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants