Opened 2 years ago

Last modified 14 months ago

#19836 assigned defect

Prepare relay descriptor downloader for consensuses published at :30 of the hour

Reported by: karsten Owned by: metrics-team
Priority: Low Milestone:
Component: Metrics/CollecTor Version:
Severity: Normal Keywords:
Cc: iwakeh Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

Consensuses have always been published once per hour, so with valid-after time :00 in the past. This might change in the future, as has been discussed in the context of consensus diffs, but which might also happen independent of those.

However, CollecTor is not ready for such a change, because currentValidAfter in RelayDescriptorDownloader is always initialized with :00 of the hour. This would prevent us from accepting a consensus published at :30. Of course, when we make such a change we should also accept other valid-after times that :00 and :30.

Testing a fix might require setting up a testing network using Chutney and archiving network data produced by that. That is, we could also create a testing environment and write unit tests for this, which we should do anyway. But that shouldn't block us here, unless the fix is more complex than I currently anticipate.

Child Tickets

Change History (3)

comment:1 in reply to:  description ; Changed 2 years ago by iwakeh

Replying to karsten:

Consensuses have always been published once per hour, so with valid-after time :00 in the past. This might change in the future, as has been discussed in the context of consensus diffs, but which might also happen independent of those.

However, CollecTor is not ready for such a change, because currentValidAfter in RelayDescriptorDownloader is always initialized with :00 of the hour. This would prevent us from accepting a consensus published at :30. Of course, when we make such a change we should also accept other valid-after times that :00 and :30.

Is :30 just an example or will there always be a set of valid values like for example {30,00,25}?

Testing a fix might require setting up a testing network using Chutney and archiving network data produced by that. That is, we could also create a testing environment and write unit tests for this, which we should do anyway. But that shouldn't block us here, unless the fix is more complex than I currently anticipate.

I think, the testing should be doable by (j)unit tests.
The (system) test environment using Chutney or similar might be useful in general, but should not be necessary here.

comment:2 in reply to:  1 Changed 2 years ago by karsten

Replying to iwakeh:

Replying to karsten:

Consensuses have always been published once per hour, so with valid-after time :00 in the past. This might change in the future, as has been discussed in the context of consensus diffs, but which might also happen independent of those.

However, CollecTor is not ready for such a change, because currentValidAfter in RelayDescriptorDownloader is always initialized with :00 of the hour. This would prevent us from accepting a consensus published at :30. Of course, when we make such a change we should also accept other valid-after times that :00 and :30.

Is :30 just an example or will there always be a set of valid values like for example {30,00,25}?

Quoting from dir-spec.txt:

   Authorities divide time into Intervals.  Authority administrators SHOULD
   try to all pick the same interval length, and SHOULD pick intervals that
   are commonly used divisions of time (e.g., 5 minutes, 15 minutes, 30
   minutes, 60 minutes, 90 minutes).  Voting intervals SHOULD be chosen to
   divide evenly into a 24-hour day.

Testing a fix might require setting up a testing network using Chutney and archiving network data produced by that. That is, we could also create a testing environment and write unit tests for this, which we should do anyway. But that shouldn't block us here, unless the fix is more complex than I currently anticipate.

I think, the testing should be doable by (j)unit tests.
The (system) test environment using Chutney or similar might be useful in general, but should not be necessary here.

Okay.

comment:3 Changed 14 months ago by karsten

Owner: set to metrics-team
Status: newassigned
Note: See TracTickets for help on using tickets.