Skip to content

Conversation

oncilla
Copy link
Contributor

@oncilla oncilla commented Sep 4, 2019

Currently, the next query does not consider path expiration time. (see #2979)
This PR reduces the query interval for sciond in the sig_short_exp_time expiration test.


This change is Reviewable

Currently, the next query does not consider path expiration time.
(see # 2979)
This PR reduces the query interval for sciond in the sig_short_exp_time
expiration test.
@oncilla oncilla force-pushed the pub-reduce-sciond-timeout branch from 4cd8d82 to d63980d Compare September 4, 2019 14:50
@oncilla oncilla marked this pull request as ready for review September 4, 2019 15:14
@oncilla oncilla requested a review from scrye September 4, 2019 15:15
@oncilla oncilla added bug Something isn't working and removed bug Something isn't working labels Sep 4, 2019
@oncilla oncilla added this to the Q3S3 milestone Sep 4, 2019
Copy link
Contributor

@scrye scrye left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm:

Reviewed 1 of 1 files at r1.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

@oncilla oncilla merged commit 98aa0be into scionproto:master Sep 4, 2019
@oncilla oncilla deleted the pub-reduce-sciond-timeout branch September 4, 2019 15:22
oncilla added a commit to oncilla/scion that referenced this pull request Oct 1, 2019
oncilla added a commit to oncilla/scion that referenced this pull request Oct 2, 2019
oncilla added a commit that referenced this pull request Oct 2, 2019
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 #2979

Also, revert band-aid fix introduced by #3089
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants