-
Notifications
You must be signed in to change notification settings - Fork 165
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Segfetcher: Make NextQuery dependent on segments #3208
Conversation
72c4063
to
28eacf7
Compare
go/lib/infra/modules/segfetcher/fetcher.go, line 198 at r1 (raw file):
Maybe simplify this a bit:
Better names probably exist. |
With this PR, the next query is dependent on the verified segments. The next query interval is at least minQueryInterval and at most the configured query interval. However, if only segments are found where the time until segment expiration minus the `expirationLeadTime` (2 minutes) are found that are less than the configured query interval, then that is chosen. I.e. the query interval is: ```` maxExpirationDiff = max(seg.Exp().Add(-expirationLeadTime).Sub(now)) min(max(minQueryInterval, maxExpirationDiff), QueryInterval) ```` fixes scionproto#2979
36e78a0
to
767c82e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 0 of 5 files reviewed, 1 unresolved discussion (waiting on @scrye)
go/lib/infra/modules/segfetcher/fetcher.go, line 198 at r1 (raw file):
Previously, scrye (Sergiu Costea) wrote…
Maybe simplify this a bit:
lastExpiration := getLastExpiration(segs) lastExpiration -= expirationLeadTime relativeNextQuery := findNearestInInterval(lastExpiration, minQueryInterval, f.QueryInterval) return time.Now() + relativeNextQuery
Better names probably exist.
Done.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 5 of 5 files at r1, 1 of 1 files at r2.
Reviewable status:complete! all files reviewed, all discussions resolved
With this PR, the next query is dependent on the verified segments.
The next query interval is at least minQueryInterval and at most
the configured query interval.
However, if only segments are found where the time until segment
expiration minus the
expirationLeadTime
(2 minutes) are found that areless than the configured query interval, then that is chosen.
I.e. the query interval is:
fixes #2979
Also, revert band-aid fix introduced by #3089
This change is