internal/contour: remove holdoffmaxdelay forced update #2285
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
davecheney
force-pushed
the
issue/2275c
branch
from
2efa8ac to
e6a38b4
Compare
Updates projectcontour#2275 The holdoff logic is subtle, and under tested. To address both of these issues I'm going to make the fix for projectcontour#2275 in several stages so we have a chance to git bisect. This first change removes the explicit force code path when holdoffmaxdelay is exceeded. The logic remains the same, if holdoffmaxdelay is exceeded a delayed update will occur, however after this change rather than their being a specific code path for the forced update we set the timer delay to zero, which will make the <-pending channel ready shortly afterwards. Strictly speaking a stream of <-op updates can trump a <-pending which can add further delay to rebuilding the dag but in practice this is unlikely to occur indefinitely; 1. When more than one channel is selectable, the runtime chooses between them pseudo randomly. 2. If <-op updates occur continually it is in the users' interest that the DAG rebuild is delayed, this is the point of 2275. A future PR will refactor the delay calculation so that we can test it in isolation. Signed-off-by: Dave Cheney <[email protected]>
davecheney
force-pushed
the
issue/2275c
branch
from
e6a38b4 to
f2d607e
Compare
Codecov Report
@@ Coverage Diff @@
## master #2285 +/- ##
==========================================
- Coverage 77.34% 77.32% -0.02%
==========================================
Files 59 59
Lines 5155 5151 -4
==========================================
- Hits 3987 3983 -4
Misses 1081 1081
Partials 87 87 Continue to review full report at Codecov.
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Updates #2275
The holdoff logic is subtle, and under tested. To address both of these
issues I'm going to make the fix for #2275 in several stages so we have
a chance to git bisect.
This first change removes the explicit force code path when
holdoffmaxdelay is exceeded. The logic remains the same, if
holdoffmaxdelay is exceeded a delayed update will occur, however after
this change rather than their being a specific code path for the forced
update we set the timer delay to zero, which will make the <-pending
channel ready shortly afterwards.
Strictly speaking a stream of <-op updates can trump a <-pending which
can add further delay to rebuilding the dag but in practice this is
unlikely to occur indefinitely;
them pseudo randomly.
the DAG rebuild is delayed, this is the point of 2275.
A future PR will refactor the delay calculation so that we can test it
in isolation.
Signed-off-by: Dave Cheney [email protected]