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

#36519 causes daemon to continually spin CPU #36608

Closed
lowenna opened this issue Mar 15, 2018 · 5 comments · Fixed by #36609
Closed

#36519 causes daemon to continually spin CPU #36608

lowenna opened this issue Mar 15, 2018 · 5 comments · Fixed by #36609
Labels
area/daemon kind/bug Bugs are bugs. The cause may or may not be known at triage time so debugging may be needed.

Comments

@lowenna
Copy link
Member

lowenna commented Mar 15, 2018

#36519 changed daemon/stats/collector.go so that Run() sits in a tight loop spinning the CPU. Just starting the docker daemon is enough, no client operations needed

It spins as follows:

1	for {
		// it does not make sense in the first iteration,
		// but saves allocations in further iterations
2		pairs = pairs[:0]

3		s.m.Lock()
4		for container, publisher := range s.publishers {
			// copy pointers here to release the lock ASAP
			pairs = append(pairs, publishersPair{container, publisher})
		}
5		s.m.Unlock()
6		if len(pairs) == 0 {
7			continue --> 1
		}

@stevvooe @thaJeztah
@thaJeztah
Copy link
Member

Guess we need a time.Sleep(s.interval) here;

moby/daemon/stats/collector.go

Lines 104 to 106 in 623b1a5

if len(pairs) == 0 {
continue
}
and here

moby/daemon/stats/collector.go

Lines 109 to 112 in 623b1a5

if err != nil {
logrus.Errorf("collecting system online cpu count: %v", err)
continue
}

Alternatively, put the time.Sleep(s.interval) at the start of the loop

@stevvooe
Copy link
Contributor

Is there any error logged? The difference here is that it can fail for each pair. What is the actual error preventing the sampling of cpu usage?

@thaJeztah thaJeztah mentioned this issue Mar 15, 2018
Merged
@stevvooe
Copy link
Contributor

Ok, I see. There needs to be a sleep after the continue when no stats are being sampled.

@thaJeztah
Copy link
Member

I'll open a PR, putting the sleep at the start (discussing with @stevvooe on slack)

@thaJeztah thaJeztah mentioned this issue Mar 15, 2018
Merged
@thaJeztah
Copy link
Member

Opened #36609

@thaJeztah thaJeztah added kind/bug Bugs are bugs. The cause may or may not be known at triage time so debugging may be needed. area/daemon version/18.03 and removed version/18.03 labels Mar 15, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
area/daemon kind/bug Bugs are bugs. The cause may or may not be known at triage time so debugging may be needed.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix stats collector spinning CPU if no stats are collected thaJeztah/docker
3 participants
@stevvooe @thaJeztah @lowenna