core/tracker: add analyser and deleter #996
Merged
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
Codecov Report
@@ Coverage Diff @@
## main #996 +/- ##
==========================================
+ Coverage 53.84% 53.90% +0.05%
==========================================
Files 117 117
Lines 13022 13117 +95
==========================================
+ Hits 7012 7071 +59
- Misses 4988 5020 +32
- Partials 1022 1026 +4
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
corverroos
approved these changes
Aug 19, 2022
app/retry/retry.go
Outdated
| // ctxTimeoutFunc returns a context that is cancelled when duties for a slot have elapsed. | ||
| ctxTimeoutFunc := func(ctx context.Context, t T) (context.Context, context.CancelFunc) { | ||
| return context.WithDeadline(ctx, timeoutFunc(t)) | ||
| // Ignoring boolean value as it is of no use here. | ||
| timeout, _ := timeoutFunc(t) |
There was a problem hiding this comment.
the point of returning false is to indicate the it never times out, so then you should never cancel the context...
timeout, ok := timeoutFunc(t)
if !ok {
return ctx, func()
}
return context.WithDeadline(ctx, timeout)
core/deadline.go
Outdated
Comment on lines
124
to
125
| // Ignoring boolean value | ||
| deadline, canExpire := deadlineFunc(input.duty) |
There was a problem hiding this comment.
mmmm, the point of the boolean value is to not ignore it. You cannot use deadline response if ok is false.
deadline, ok := deadlineFunc(input.duty)
if !ok {
// Drop duties that never expire
input.success <- false
return
}
core/deadline.go
Outdated
| var currDuty Duty | ||
| currDeadline := time.Date(9999, 1, 1, 0, 0, 0, 0, time.UTC) | ||
|
|
||
| for duty := range duties { | ||
| dutyDeadline := deadlineFunc(duty) | ||
| dutyDeadline, _ := deadlineFunc(duty) |
There was a problem hiding this comment.
same here, you cannot use dutyDeadline of ok==false
core/deadline.go
Outdated
| @@ -192,7 +193,7 @@ func getCurrDuty(duties map[Duty]bool, deadlineFunc func(duty Duty) time.Time) ( | |||
| } | |||
|
|
|||
| // NewDutyDeadlineFunc returns the function that provides duty deadlines. | |||
There was a problem hiding this comment.
// NewDutyDeadlineFunc returns the function that provides duty deadlines or false if the duty never deadlines.
core/deadline.go
Outdated
| if duty.Type == DutyExit || duty.Type == DutyBuilderRegistration { | ||
| // Do not timeout exit or registration duties. | ||
| return time.Date(9999, 1, 1, 0, 0, 0, 0, time.UTC) | ||
| return time.Date(9999, 1, 1, 0, 0, 0, 0, time.UTC), false |
There was a problem hiding this comment.
return time.Time{}, false
core/tracker/tracker.go
Outdated
| events map[core.Duty][]event | ||
| // analyser returns duty from analyser.C() when it is deadlined to analyse for failures and participation. | ||
| analyser core.Deadliner | ||
| // deleter returns duty from deleter.C() after 2 * deadline time to delete from memory. |
There was a problem hiding this comment.
Tracker does not know this.
// analyser triggers duty analysis
analyser core.Deadliner
// deleter triggers duty deletion after all associated analysis are done.
deleter core.Deadliner
core/tracker/tracker.go
Outdated
| @@ -86,12 +89,13 @@ type Tracker struct { | |||
| } | |||
|
|
|||
| // New returns a new Tracker. | |||
There was a problem hiding this comment.
// New returns a new Tracker. The deleter deadliner must return well after analyser deadliner since duties of the same slot are often analysed together.
core/tracker/tracker.go
Outdated
| @@ -110,13 +114,13 @@ func (t *Tracker) Run(ctx context.Context) error { | |||
| case <-ctx.Done(): | |||
| return ctx.Err() | |||
| case e := <-t.input: | |||
| if !t.deadliner.Add(e.duty) { | |||
| if !t.analyser.Add(e.duty) && !t.deleter.Add(e.duty) { | |||
There was a problem hiding this comment.
if !t.deleter.Add(e.duty) || !t.analyser.Add(e.duty) {
continue // Ignore expired or never expiring duties
}
dB2510
added
the
merge when ready
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.
Adds 2 deadliners in tracker: analyser and deleter. When analyser deadline hits, tracker analyser failures and participation. When deleter deadline hits track trims internal state of duties.
category: bug
ticket: #993