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

Increase default node cpu size from 1 to 4. #335

Merged
merged 3 commits into from
Mar 10, 2018
Merged

Conversation

josephburnett
Copy link
Contributor

Previously we were requesting 2.5% of a CPU for Revisions which is not enough to run any meaningful load. And the scheduler will happily pack way too many Pods into each node and therefore the cluster autoscaler won't kick in when needed.

The Elafros autoscaler commit (#229) increases the CPU request per Pod to 1 full CPU. This was necessary to make autoscaling work correctly. However the default, tiny cluster of 3, 1-core nodes simply couldn't fit the Elafros control plane (Istio, Controllers, Webhooks) and two Revisions. Even 2-core machines are a little small.

So I'm increasing the default to 4 cores and autoscaling will bring the cluster down to 1 node when not used.

Each Revision needs most of a CPU.  Conformance tests need two Revisions.  And we have all the control plane stuff running.  So we need bigger nodes.
@josephburnett josephburnett requested a review from a team March 9, 2018 18:18
@google-prow-robot google-prow-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Mar 9, 2018
@josephburnett
Copy link
Contributor Author

#291 (comment) has some more context on why this change is necessary.

@bobcatfish
Copy link
Contributor

What do you think about adding some docs at https://github.com/elafros/elafros#latest-release about minimum cluster requirements as well?

@grantr
Copy link
Contributor

grantr commented Mar 9, 2018

I created a new cluster with n1-standard-4 nodes, and conformance tests now consistently pass (on this branch). 🎉

@@ -29,6 +29,7 @@ To use a k8s cluster running in GKE:
--cluster-version=1.9.2-gke.1 \
--zone=us-east1-d \
--scopes=cloud-platform \
--machine-type=n1-standard-4 \
Copy link
Contributor

Choose a reason for hiding this comment

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

Since this will be significantly more expensive for users, can we add a comment explaining why this machine type is necessary?

Copy link
Contributor

Choose a reason for hiding this comment

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

I added a comment below explaining why this is recommended.

@bobcatfish
Copy link
Contributor

/retest

@@ -29,6 +29,7 @@ To use a k8s cluster running in GKE:
--cluster-version=1.9.2-gke.1 \
--zone=us-east1-d \
--scopes=cloud-platform \
--machine-type=n1-standard-4 \
Copy link
Contributor

Choose a reason for hiding this comment

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

I added a comment below explaining why this is recommended.

@bobcatfish
Copy link
Contributor

I'd like to get this merged now so we can be in a state where folks can follow the setup instructions and have passing conformance tests, but @josephburnett could you follow up with some docs or an issue to create docs about minimum system requirements for folks that want to use Elafros?

@josephburnett
Copy link
Contributor Author

@bobcatfish. Ack. I'll add those docs.

@mattmoor mattmoor deleted the josephburnett-patch-1 branch March 20, 2018 13:12
markusthoemmes pushed a commit to markusthoemmes/knative-serving that referenced this pull request Nov 20, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants