Skip to content

[backport: 17.06] [manager/dispatcher] Replace call to isRunning() to isRunningLocked() in dispatcher Heartbeat()#2715

Merged
cyli merged 1 commit into
moby:bump_v17.06from
anshulpundir:race_backport
Jul 26, 2018
Merged

[backport: 17.06] [manager/dispatcher] Replace call to isRunning() to isRunningLocked() in dispatcher Heartbeat()#2715
cyli merged 1 commit into
moby:bump_v17.06from
anshulpundir:race_backport

Conversation

@anshulpundir
Copy link
Copy Markdown
Contributor

backport of #2664 for 17.06

git cherry-pick -s -S -x caee4da
cherry-pick was not clean.

We noticed repeated CI failures pointing to data races because of unlocked access to the dispatcher context in Heartbeat(). Golang also does not provide any guarantees around read-only operations on objects which are otherwise locked.

This will likely have a performance impact, which we will evaluate and some of the dispatcher code might need to be re-written accordingly. But correctness is first.

… in dispatcher Heartbeat()

Signed-off-by: Anshul Pundir <anshul.pundir@docker.com>
@codecov
Copy link
Copy Markdown

codecov Bot commented Jul 26, 2018

Codecov Report

Merging #2715 into bump_v17.06 will increase coverage by 0.41%.
The diff coverage is 0%.

@@               Coverage Diff               @@
##           bump_v17.06    #2715      +/-   ##
===============================================
+ Coverage        60.87%   61.28%   +0.41%     
===============================================
  Files               30      121      +91     
  Lines             4322    20203   +15881     
===============================================
+ Hits              2631    12382    +9751     
- Misses            1435     6462    +5027     
- Partials           256     1359    +1103

Copy link
Copy Markdown
Contributor

@cyli cyli left a comment

Choose a reason for hiding this comment

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

LGTM thank you!

@cyli cyli merged commit 7e16206 into moby:bump_v17.06 Jul 26, 2018
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.

2 participants