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

Remove GODEBUG from pilot and mixer #16852

Merged
merged 1 commit into from
Sep 5, 2019

Conversation

howardjohn
Copy link
Member

A bug/feature in golang 1.13 causes GODEBUG to output thousands of
messages. Since we will release 1.4 off of 1.13, I think the best course
of action is to turn this off by default.

See #16635

Please provide a description for what this PR is for.

And to help us figure out who should review this PR, please
put an X in all the areas that this PR affects.

[ ] Configuration Infrastructure
[ ] Docs
[ ] Installation
[ ] Networking
[ ] Performance and Scalability
[ ] Policies and Telemetry
[ ] Security
[ ] Test and Release
[ ] User Experience
[ ] Developer Infrastructure

A bug/feature in golang 1.13 causes GODEBUG to output thousands of
messages. Since we will release 1.4 off of 1.13, I think the best course
of action is to turn this off by default.

See istio#16635
@howardjohn howardjohn requested a review from a team as a code owner September 5, 2019 15:56
@googlebot googlebot added the cla: yes Set by the Google CLA bot to indicate the author of a PR has signed the Google CLA. label Sep 5, 2019
@istio-testing istio-testing added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Sep 5, 2019
@ayj
Copy link
Contributor

ayj commented Sep 5, 2019

How much do we lose by disabling this? Are Prometheus metrics sufficient at this point to identify memory/gc issues?

@howardjohn
Copy link
Member Author

How much do we lose by disabling this? Are Prometheus metrics sufficient at this point to identify memory/gc issues?

Both components export all sorts of GC stats, so I don't think we lose too much here. They can always be added back - this is just changing the default.

If we don't do this the entire log will look like

scvg: inuse: 18, idle: 43, sys: 62, released: 37, consumed: 24 (MB)
scvg: 0 MB released
scvg: inuse: 18, idle: 43, sys: 62, released: 37, consumed: 24 (MB)
scvg: 0 MB released
scvg: inuse: 18, idle: 43, sys: 62, released: 37, consumed: 24 (MB)
scvg: 0 MB released
scvg: inuse: 18, idle: 43, sys: 62, released: 37, consumed: 24 (MB)
scvg: 0 MB released
scvg: inuse: 18, idle: 43, sys: 62, released: 37, consumed: 24 (MB)
scvg: 0 MB released
scvg: inuse: 18, idle: 43, sys: 62, released: 37, consumed: 24 (MB)
scvg: 0 MB released
scvg: inuse: 18, idle: 43, sys: 62, released: 37, consumed: 24 (MB)
scvg: 0 MB released
scvg: inuse: 18, idle: 43, sys: 62, released: 37, consumed: 24 (MB)
scvg: 0 MB released
scvg: inuse: 18, idle: 43, sys: 62, released: 37, consumed: 24 (MB)
scvg: 0 MB released
scvg: inuse: 18, idle: 43, sys: 62, released: 37, consumed: 24 (MB)
scvg: 0 MB released
scvg: inuse: 18, idle: 43, sys: 62, released: 37, consumed: 24 (MB)
scvg: 0 MB released
scvg: inuse: 18, idle: 43, sys: 62, released: 37, consumed: 24 (MB)
scvg: 0 MB released
scvg: inuse: 18, idle: 43, sys: 62, released: 37, consumed: 24 (MB)
scvg: 0 MB released
scvg: inuse: 18, idle: 43, sys: 62, released: 37, consumed: 24 (MB)
scvg: 0 MB released
scvg: inuse: 18, idle: 43, sys: 62, released: 37, consumed: 24 (MB)
scvg: 0 MB released
scvg: inuse: 18, idle: 43, sys: 62, released: 37, consumed: 24 (MB)
scvg: 0 MB released
scvg: inuse: 18, idle: 43, sys: 62, released: 37, consumed: 24 (MB)
scvg: 0 MB released
scvg: inuse: 18, idle: 43, sys: 62, released: 37, consumed: 24 (MB)
scvg: 0 MB released
scvg: inuse: 18, idle: 43, sys: 62, released: 37, consumed: 24 (MB)
scvg: 0 MB released
scvg: inuse: 18, idle: 43, sys: 62, released: 37, consumed: 24 (MB)
scvg: 0 MB released
scvg: inuse: 18, idle: 43, sys: 62, released: 37, consumed: 24 (MB)
scvg: 0 MB released
scvg: inuse: 18, idle: 43, sys: 62, released: 37, consumed: 24 (MB)
scvg: 0 MB released
scvg: inuse: 18, idle: 43, sys: 62, released: 37, consumed: 24 (MB)
scvg: 0 MB released
scvg: inuse: 18, idle: 43, sys: 62, released: 38, consumed: 24 (MB)
scvg: 0 MB released
scvg: inuse: 18, idle: 43, sys: 62, released: 38, consumed: 24 (MB)
scvg: 0 MB released
scvg: inuse: 18, idle: 43, sys: 62, released: 38, consumed: 24 (MB)
scvg: 0 MB released
scvg: inuse: 18, idle: 43, sys: 62, released: 38, consumed: 24 (MB)
scvg: 0 MB released
scvg: inuse: 18, idle: 43, sys: 62, released: 38, consumed: 24 (MB)
scvg: 0 MB released
scvg: inuse: 18, idle: 43, sys: 62, released: 38, consumed: 24 (MB)
scvg: 0 MB released
scvg: inuse: 18, idle: 43, sys: 62, released: 38, consumed: 24 (MB)
scvg: 0 MB released
scvg: inuse: 18, idle: 43, sys: 62, released: 38, consumed: 24 (MB)
scvg: 0 MB released
scvg: inuse: 18, idle: 43, sys: 62, released: 38, consumed: 24 (MB)
scvg: 0 MB released
scvg: inuse: 18, idle: 43, sys: 62, released: 38, consumed: 24 (MB)
scvg: 0 MB released
scvg: inuse: 18, idle: 43, sys: 62, released: 38, consumed: 24 (MB)
scvg: 0 MB released
scvg: inuse: 18, idle: 43, sys: 62, released: 38, consumed: 24 (MB)
scvg: 0 MB released
scvg: inuse: 18, idle: 43, sys: 62, released: 38, consumed: 24 (MB)
scvg: 0 MB released
scvg: inuse: 18, idle: 43, sys: 62, released: 38, consumed: 24 (MB)
scvg: 0 MB released
scvg: inuse: 18, idle: 43, sys: 62, released: 38, consumed: 24 (MB)
scvg: 0 MB released
scvg: inuse: 18, idle: 43, sys: 62, released: 38, consumed: 24 (MB)
scvg: 0 MB released
scvg: inuse: 18, idle: 43, sys: 62, released: 38, consumed: 24 (MB)
scvg: 0 MB released
scvg: inuse: 18, idle: 43, sys: 62, released: 38, consumed: 24 (MB)
scvg: 0 MB released
scvg: inuse: 18, idle: 43, sys: 62, released: 38, consumed: 24 (MB)
scvg: 0 MB released
scvg: inuse: 18, idle: 43, sys: 62, released: 38, consumed: 24 (MB)
scvg: 0 MB released
scvg: inuse: 18, idle: 43, sys: 62, released: 38, consumed: 24 (MB)
scvg: 0 MB released
scvg: inuse: 18, idle: 43, sys: 62, released: 38, consumed: 24 (MB)
scvg: 0 MB released
scvg: inuse: 18, idle: 43, sys: 62, released: 38, consumed: 24 (MB)
scvg: 0 MB released
scvg: inuse: 18, idle: 43, sys: 62, released: 38, consumed: 24 (MB)
scvg: 0 MB released
scvg: inuse: 18, idle: 43, sys: 62, released: 38, consumed: 24 (MB)
scvg: 0 MB released
scvg: inuse: 18, idle: 43, sys: 62, released: 38, consumed: 24 (MB)
scvg: 0 MB released
scvg: inuse: 18, idle: 43, sys: 62, released: 38, consumed: 24 (MB)
scvg: 0 MB released
scvg: inuse: 18, idle: 43, sys: 62, released: 38, consumed: 24 (MB)
scvg: 0 MB released
scvg: inuse: 18, idle: 43, sys: 62, released: 38, consumed: 24 (MB)
scvg: 0 MB released
scvg: inuse: 18, idle: 43, sys: 62, released: 38, consumed: 24 (MB)
scvg: 0 MB released
scvg: inuse: 18, idle: 43, sys: 62, released: 38, consumed: 24 (MB)
scvg: 0 MB released
scvg: inuse: 18, idle: 43, sys: 62, released: 38, consumed: 24 (MB)
scvg: 0 MB released
scvg: inuse: 18, idle: 43, sys: 62, released: 38, consumed: 24 (MB)
scvg: 0 MB released
scvg: inuse: 18, idle: 43, sys: 62, released: 38, consumed: 24 (MB)
scvg: 0 MB released
scvg: inuse: 18, idle: 43, sys: 62, released: 38, consumed: 24 (MB)
scvg: 0 MB released
scvg: inuse: 18, idle: 43, sys: 62, released: 38, consumed: 24 (MB)
scvg: 0 MB released
scvg: inuse: 18, idle: 43, sys: 62, released: 38, consumed: 24 (MB)
scvg: 0 MB released
scvg: inuse: 18, idle: 43, sys: 62, released: 38, consumed: 24 (MB)
scvg: 0 MB released
scvg: inuse: 18, idle: 43, sys: 62, released: 38, consumed: 24 (MB)
scvg: 0 MB released
scvg: inuse: 18, idle: 43, sys: 62, released: 38, consumed: 24 (MB)
scvg: 0 MB released
scvg: inuse: 18, idle: 43, sys: 62, released: 38, consumed: 24 (MB)
scvg: 0 MB released
scvg: inuse: 18, idle: 43, sys: 62, released: 38, consumed: 24 (MB)
scvg: 0 MB released
scvg: inuse: 18, idle: 43, sys: 62, released: 38, consumed: 24 (MB)
scvg: 0 MB released
scvg: inuse: 18, idle: 43, sys: 62, released: 38, consumed: 24 (MB)
scvg: 0 MB released
scvg: inuse: 18, idle: 43, sys: 62, released: 38, consumed: 24 (MB)
scvg: 0 MB released
scvg: inuse: 18, idle: 43, sys: 62, released: 38, consumed: 24 (MB)
scvg: 0 MB released
scvg: inuse: 18, idle: 43, sys: 62, released: 38, consumed: 24 (MB)
scvg: 0 MB released
scvg: inuse: 18, idle: 43, sys: 62, released: 38, consumed: 24 (MB)
scvg: 0 MB released
scvg: inuse: 18, idle: 43, sys: 62, released: 38, consumed: 24 (MB)
scvg: 0 MB released
scvg: inuse: 18, idle: 43, sys: 62, released: 38, consumed: 24 (MB)
scvg: 0 MB released
scvg: inuse: 18, idle: 43, sys: 62, released: 38, consumed: 24 (MB)
scvg: 0 MB released
scvg: inuse: 18, idle: 43, sys: 62, released: 38, consumed: 24 (MB)
scvg: 0 MB released
scvg: inuse: 18, idle: 43, sys: 62, released: 38, consumed: 24 (MB)
scvg: 0 MB released
scvg: inuse: 18, idle: 43, sys: 62, released: 38, consumed: 23 (MB)
scvg: 0 MB released
scvg: inuse: 18, idle: 43, sys: 62, released: 38, consumed: 23 (MB)
scvg: 0 MB released
scvg: inuse: 18, idle: 43, sys: 62, released: 38, consumed: 23 (MB)
scvg: 0 MB released
scvg: inuse: 18, idle: 43, sys: 62, released: 38, consumed: 23 (MB)
scvg: 0 MB released
scvg: inuse: 18, idle: 43, sys: 62, released: 38, consumed: 23 (MB)
scvg: 0 MB released
scvg: inuse: 18, idle: 43, sys: 62, released: 38, consumed: 23 (MB)
scvg: 0 MB released
scvg: inuse: 18, idle: 43, sys: 62, released: 38, consumed: 23 (MB)
scvg: 0 MB released
scvg: inuse: 18, idle: 43, sys: 62, released: 38, consumed: 23 (MB)
scvg: 0 MB released
scvg: inuse: 18, idle: 43, sys: 62, released: 38, consumed: 23 (MB)
scvg: 0 MB released
scvg: inuse: 18, idle: 43, sys: 62, released: 38, consumed: 23 (MB)

That is from less than a second

@howardjohn
Copy link
Member Author

/retest

1 similar comment
@howardjohn
Copy link
Member Author

/retest

@istio-testing istio-testing merged commit c269a43 into istio:master Sep 5, 2019
@hzxuzhonghu
Copy link
Member

I like this very much, it is too noisy before.

@howardjohn howardjohn mentioned this pull request Sep 9, 2019
16 tasks
howardjohn added a commit to howardjohn/istio that referenced this pull request Jan 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes Set by the Google CLA bot to indicate the author of a PR has signed the Google CLA. 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.

None yet

5 participants