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.

Sign up for GitHub

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

Jump to bottom

Enable the garbage collector by default #30480

Merged
merged 1 commit into from
Aug 20, 2016
Merged

Enable the garbage collector by default #30480

merged 1 commit into from
Aug 20, 2016

Conversation

caesarxuchao
Copy link
Member

@caesarxuchao caesarxuchao commented Aug 11, 2016 •
edited
Loading

Turning GC on by default.

Memory usage of GC is back to normal after #30943. The CPU usage is a little higher than the cap in scalability test (1.11 core vs. 1 core). This PR adjusted the default GC worker to 20 to see if that helps CPU usage.

@kubernetes/sig-api-machinery @wojtek-t @lavalamp

This change is Reviewable

@caesarxuchao caesarxuchao added the sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. label Aug 11, 2016
@caesarxuchao caesarxuchao self-assigned this Aug 11, 2016
@k8s-github-robot k8s-github-robot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. release-note-label-needed labels Aug 11, 2016
@caesarxuchao
Copy link
Member Author

All tests passed! I double checked the e2e test logs and gc is running.

@lavalamp lavalamp added release-note Denotes a PR that will be considered when it comes time to generate release notes. lgtm "Looks good to me", indicates that a PR is ready to be merged. and removed release-note-label-needed labels Aug 12, 2016
@lavalamp lavalamp assigned lavalamp and unassigned caesarxuchao Aug 12, 2016
@lavalamp
Copy link
Member

lgtm!

@caesarxuchao caesarxuchao removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 13, 2016
@caesarxuchao
Copy link
Member Author

I'll add back the lgtm after the scalability tests pass.

@caesarxuchao caesarxuchao changed the title [WIP] Enable the garbage collector by default Enable the garbage collector by default Aug 15, 2016
@caesarxuchao
Copy link
Member Author

The gce-500 test passes. The gce-scale is flaky, even without running GC. I'll add back the lgtm and monitor the tests.

@caesarxuchao caesarxuchao added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 15, 2016
@ncdc
Copy link
Member

ncdc commented Aug 17, 2016

FYI @derekwaynecarr @timothysc

@timothysc
Copy link
Member

/cc @rrati

@wojtek-t
Copy link
Member

wojtek-t commented Aug 17, 2016 •
edited
Loading

In the context of #30759 I'm (temporarily) removing "lgtm" label.

Regarding kubernetes-scale - it was flaky after I increase throughput by 5x, but after enabling GC it is just broken (it didn't pass even once). [And it wasn't even close]

@wojtek-t wojtek-t removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 17, 2016
@timothysc
Copy link
Member

more fodder for @justinsb ;-)

@wojtek-t
Copy link
Member

One more significant issue: #30871 (this doesn't need to be a blocker, but this definitely needs to be a conscious decision).

@k8s-github-robot k8s-github-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 19, 2016
@lavalamp lavalamp added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 19, 2016
@lavalamp lavalamp added this to the v1.4 milestone Aug 19, 2016
@k8s-github-robot k8s-github-robot removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Aug 19, 2016
@k8s-github-robot k8s-github-robot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Aug 19, 2016
@lavalamp
Copy link
Member

lgtm again. (we think the performance problems are solved enough now.)

@wojtek-t
Copy link
Member

OK - I'm probably also fine with it now :)

@k8s-bot
Copy link

k8s-bot commented Aug 19, 2016

GCE e2e build/test passed for commit f7a1ef8.

@hongchaodeng hongchaodeng mentioned this pull request Aug 20, 2016
Merged
1 task
@k8s-github-robot
Copy link

@k8s-bot test this [submit-queue is verifying that this PR is safe to merge]

@k8s-bot
Copy link

k8s-bot commented Aug 20, 2016

GCE e2e build/test passed for commit f7a1ef8.

@k8s-github-robot
Copy link

Automatic merge from submit-queue

@k8s-github-robot k8s-github-robot merged commit c8c18b1 into kubernetes:master Aug 20, 2016
@justinsb justinsb mentioned this pull request Aug 22, 2016
Closed
1 task
k8s-github-robot pushed a commit that referenced this pull request Aug 22, 2016
Merge pull request #30843 from hongchaodeng/tls
7bd8d7d 
Automatic merge from submit-queue

etcd3 backend: support TLS

What?
Support TLS in etcd3 storage backend.
It works the same as previous etcd2 config.

- [ ] Blocked on ##30480
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@lavalamp lavalamp

Labels
lgtm "Looks good to me", indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Milestone
v1.4
Development

Successfully merging this pull request may close these issues.
8 participants
@caesarxuchao @lavalamp @ncdc @timothysc @wojtek-t @k8s-bot @k8s-github-robot @googlebot