Fix agent logging race #2578
Fix agent logging race #2578
Conversation
|
@cyli I would have just removed the |
agent/reporter.go
Outdated
| } | ||
|
|
||
| type statusReporterFunc func(ctx context.Context, taskID string, status *api.TaskStatus) error | ||
| type statusReporterFunc struct { | ||
| function func(ctx context.Context, taskID string, status *api.TaskStatus) error |
There was a problem hiding this comment.
This is pretty ugly. :| But the alternative is to always report "anonymous function". In reality, we only use this (not in tests) in one place, and that is an anonymous function for reporting on a single task.
|
@stevvooe I'm not sure if there is any value, except that maybe we could correlate the failed report with a particular task getting stuck. Probably just taking out the reporter would be better though. This is a pretty ugly way to get the info. |
…ata race. Signed-off-by: Ying Li <ying.li@docker.com>
cyli
force-pushed
the
fix-agent-data-race
branch
from
ff5fd37
to
835c449
Compare
Codecov Report
@@ Coverage Diff @@
## master #2578 +/- ##
==========================================
- Coverage 61.42% 61.34% -0.08%
==========================================
Files 134 134
Lines 21722 21722
==========================================
- Hits 13342 13325 -17
- Misses 6945 6964 +19
+ Partials 1435 1433 -2 |
|
@cyli Agreed. Let's go with the simple option here. Thanks for exploring the problem space! |
|
LGTM |
|
@stevvooe Thanks for finding the problematic line! :D |
Add an ID function to StatusReporter, so we can better log what reporter failed.
Granted, this is a lot of extra changes to fix this problematic line: https://github.com/docker/swarmkit/compare/master...cyli:fix-agent-data-race?expand=1#diff-689e5cbc4a9f04fda8f8cd9749d87db7L438.
We could just take out the reporting on which reporter failed instead, and that would be shorter. But since we've been having trouble debugging live logs, I thought more information would be more useful than less.
Fixes #2576