dispatcher: increase heartbeat period for unknown nodes #1250
Conversation
Signed-off-by: Alexander Morozov <[email protected]>
|
LGTM, but we need to make sure this doesn't break Docker integration tests. #1238 broke the |
|
#1238 broke engine tests because it took too long (2 additional minutes) and it wasn't configurable. This change makes it proportional so I believe it can work, although it may not depending on the current timeout values. We should check that and then tweak the engine test. |
Current coverage is 54.94% (diff: 80.00%)@@ master #1250 diff @@
==========================================
Files 78 78
Lines 12422 12405 -17
Methods 0 0
Messages 0 0
Branches 0 0
==========================================
+ Hits 6809 6816 +7
+ Misses 4671 4653 -18
+ Partials 942 936 -6
|
|
LGTM |
|
@LK4D4 @aaronlehmann @dongluochen: I'm thinking of pushing this to 1.12.1 - do you agree? |
|
Agreed. |
|
But test it with the Docker integration tests before 1.12.1 :) |
| @@ -79,7 +82,7 @@ func (s *nodeStore) AddUnknown(n *api.Node, expireFunc func()) error { | |||
| Node: n, | |||
| } | |||
| s.nodes[n.ID] = rn | |||
| rn.Heartbeat = heartbeat.New(s.periodChooser.Choose()*s.gracePeriodMultiplier, expireFunc) | |||
| rn.Heartbeat = heartbeat.New(s.periodChooser.Choose()*s.gracePeriodMultiplierUnknown, expireFunc) | |||
There was a problem hiding this comment.
When the node goes to Ready, does heartbeat restore to normal?
There was a problem hiding this comment.
Yeah, and this heartbeat is cancelled.
|
Agree with 1.12.1 plan. |
|
LGTM |
|
Confirmed that this breaks @LK4D4 can you please find a fix for |
|
@aaronlehmann I'll take a look tomorrow. |
|
@aaronlehmann PR was merged in docker. |
|
Thanks, confirmed that the integration tests are working properly with this. |
ping @aaronlehmann @dongluochen