Skip to content

[RELEASE-0.21] Unregister views to avoid slow oom issue during meter cleanup (#2005)#2102

Closed
skonto wants to merge 1 commit into
knative:release-0.21from
skonto:backport_slow_oom_21
Closed

[RELEASE-0.21] Unregister views to avoid slow oom issue during meter cleanup (#2005)#2102
skonto wants to merge 1 commit into
knative:release-0.21from
skonto:backport_slow_oom_21

Conversation

@skonto
Copy link
Copy Markdown
Contributor

@skonto skonto commented Apr 23, 2021 •
edited
Loading

backports #2005 also need here knative/eventing#4645

Unregister views to avoid slow oom issue during meter cleanup (knativ…
6e47a81 
…e#2005)

* unregister views

* add a test

* fix string issue

* fix

* fixes

* fixe races in tests

* fix exporter issue

* stop meter in cleanup

* fixes

* typo

* revert timeout change

* fixes

* change msg
@google-cla google-cla Bot added the cla: yes Indicates the PR's author has signed the CLA. label Apr 23, 2021
@knative-prow-robot knative-prow-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Apr 23, 2021
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 23, 2021 •
edited
Loading

Codecov Report

Merging #2102 (6e47a81) into release-0.21 (584933f) will increase coverage by 0.93%.
The diff coverage is 100.00%.

Impacted file tree graph

@@               Coverage Diff                @@
##           release-0.21    #2102      +/-   ##
================================================
+ Coverage         67.03%   67.97%   +0.93%     
================================================
  Files               213      215       +2     
  Lines              8986     9446     +460     
================================================
+ Hits               6024     6421     +397     
- Misses             2687     2743      +56     
- Partials            275      282       +7
Impacted Files Coverage Δ
metrics/resource_view.go 88.63% <100.00%> (+2.08%) ⬆️
tracing/zipkin.go 68.18% <0.00%> (-0.24%) ⬇️
network/network.go 100.00% <0.00%> (ø)
injection/config.go 0.00% <0.00%> (ø)
injection/clients.go 100.00% <0.00%> (ø)
reconciler/events.go 100.00% <0.00%> (ø)
codegen/cmd/injection-gen/generators/client.go 0.00% <0.00%> (ø)
...odegen/cmd/injection-gen/generators/fake_client.go 0.00% <0.00%> (ø)
.../injection-gen/generators/reconciler_reconciler.go 0.00% <0.00%> (ø)
configmap/informer/synced_callback.go 100.00% <0.00%> (ø)
... and 11 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 584933f...6e47a81. Read the comment docs.

@skonto
Copy link
Copy Markdown
Contributor Author

skonto commented Apr 26, 2021

/assign @matzew

matzew
matzew approved these changes May 11, 2021
View reviewed changes
Copy link
Copy Markdown
Member

@matzew matzew left a comment

Choose a reason for hiding this comment

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

/lgtm
/approve

@knative-prow-robot knative-prow-robot added the lgtm Indicates that a PR is ready to be merged. label May 11, 2021
@knative-prow-robot
Copy link
Copy Markdown
Contributor

knative-prow-robot commented May 11, 2021

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: matzew
To complete the pull request process, please assign yanweiguo after the PR has been reviewed.
You can assign the PR to them by writing /assign @yanweiguo in a comment when ready.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@skonto skonto mentioned this pull request May 11, 2021
Merged
@skonto
Copy link
Copy Markdown
Contributor Author

skonto commented May 12, 2021

@matzew I should close this?

@skonto
Copy link
Copy Markdown
Contributor Author

skonto commented May 17, 2021

Closing as it didnt make it.

@skonto skonto closed this May 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@matzew matzew matzew approved these changes

Assignees

@matzew matzew

Labels

cla: yes Indicates the PR's author has signed the CLA. lgtm Indicates that a PR is ready to be merged. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@skonto @knative-prow-robot @matzew