Skip to content

[19.03 backport] Use condition variable to wake stats collector.#40549

Merged
thaJeztah merged 1 commit intomoby:19.03from
cpuguy83:19.03_stats_use_cond_var
Feb 22, 2020
Merged

[19.03 backport] Use condition variable to wake stats collector.#40549
thaJeztah merged 1 commit intomoby:19.03from
cpuguy83:19.03_stats_use_cond_var

Conversation

@cpuguy83
Copy link
Copy Markdown
Member

Backport of #40481


Before the collection goroutine wakes up every 1 second (as configured).
This sleep interval is in case there are no stats to collect we don't
end up in a tight loop.

Instead use a condition variable to signal that a collection is needed.
This prevents us from waking the goroutine needlessly when there is no
one looking for stats.

For now I've kept the sleep just moved it to the end of the loop, which
gives some space between collections.

Signed-off-by: Brian Goff cpuguy83@gmail.com
(cherry picked from commit e75e6b0)
Signed-off-by: Brian Goff cpuguy83@gmail.com

- What I did

- How I did it

- How to verify it

- Description for the changelog

- A picture of a cute animal (not mandatory but encouraged)

Before the collection goroutine wakes up every 1 second (as configured).
This sleep interval is in case there are no stats to collect we don't
end up in a tight loop.

Instead use a condition variable to signal that a collection is needed.
This prevents us from waking the goroutine needlessly when there is no
one looking for stats.

For now I've kept the sleep just moved it to the end of the loop, which
gives some space between collections.

Signed-off-by: Brian Goff <cpuguy83@gmail.com>
(cherry picked from commit e75e6b0)
Signed-off-by: Brian Goff <cpuguy83@gmail.com>
@cpuguy83 cpuguy83 added this to the 19.03.7 milestone Feb 20, 2020
@thaJeztah thaJeztah changed the title Use condition variable to wake stats collector. [19.03 backport] Use condition variable to wake stats collector. Feb 20, 2020
@cpuguy83 cpuguy83 marked this pull request as ready for review February 21, 2020 05:27
@thaJeztah
Copy link
Copy Markdown
Member

Windows is failing is related to #40525 (comment)

Please see: https://support.microsoft.com/en-us/help/4542617/you-may-encounter-issues-when-using-windows-server-docker-container-im for the latest status.

I'll backport #40506 as a temporary workaround

@thaJeztah
Copy link
Copy Markdown
Member

Opened #40551

Copy link
Copy Markdown
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

LGTM

kicked CI as the Windows fix was merged, but possibly this needs a rebase to get the fix in

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants