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

Reduce probe cpu usage #470

Merged
merged 9 commits into from
Sep 11, 2015
Merged

Reduce probe cpu usage #470

merged 9 commits into from
Sep 11, 2015

Conversation

tomwilkie
Copy link
Contributor

2 issues - report generation is taking too long, and cpu usage is too high.

Reducing CPU usage (~40% to <20%), see #284

  • WithNode -> AddNode, don't copy all the nodes every time we add a single one.
  • Cache generated endpoint IDs to relieve some more GC pressure.
  • Only gob serialise and gz compress reports once per publish cycle, not once per app.
  • In weave tagger, stop iterating through all containers to find a container by prefix. Build an index instead.

Report generation:

  • Log a warning when it takes too long
  • Generate them in parallel
  • Only fetch weave status json once per cycle (fixes Do fewer RPCs to Weave #397)

@tomwilkie tomwilkie force-pushed the 40-percent-cpu branch 3 times, most recently from ce4c336 to 947d798 Compare September 9, 2015 14:02
@peterbourgon
Copy link
Contributor

Sorry about commenting on the commits rather than the diff view. That makes it a little harder to act on.

Back over to you.

@tomwilkie
Copy link
Contributor Author

synchronisation

Is it needed? Tick and {Report, Tag} are never call concurrently. If it is, then we'll also need a pointer receiver I think?

what else does conntrack do?

It lists (or streams) connections; udp, tcp etc, and will list of stream nat mappings. I don't think it does anything else.

@tomwilkie
Copy link
Contributor Author

@peterbourgon I think I addressed all the issues.

@peterbourgon
Copy link
Contributor

Is it needed? Tick and {Report, Tag} are never call concurrently.

That's incidental and can easily and inadvertently change :)

If it is, then we'll also need a pointer receiver I think?

Yes.

@@ -22,7 +17,7 @@ const (

// Publisher is something which can send a report to a remote collector.
type Publisher interface {

This comment was marked as abuse.

overlay: mutex for Weave status
@peterbourgon
Copy link
Contributor

@tomwilkie I made the changes I proposed. Give me a 👍 and I'll merge.

@tomwilkie
Copy link
Contributor Author

LGTM

peterbourgon added a commit that referenced this pull request Sep 11, 2015
@peterbourgon peterbourgon merged commit d31abff into master Sep 11, 2015
@peterbourgon peterbourgon deleted the 40-percent-cpu branch September 11, 2015 14:43
h.Write([]byte(part3))
sum := h.Sum64()
var result string
if id, err := idCache.Get(sum); id != nil && err != nil {

This comment was marked as abuse.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Do fewer RPCs to Weave
3 participants