Opened 4 years ago

Closed 3 months ago

#19828 closed defect (fixed)

Extend descriptorCutOff in CollecTor's RelayDescriptorDownloader by 6 hours

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


CollecTor's RelayDescriptorDownloader only downloads server and extra-info descriptors that have been published up to 24 hours before the current system time. This makes sense, so that missing descriptors that cannot be obtained are not retried forever.

However, there are cases when a valid consensus or vote references a server descriptor that was published over 24 hours ago:

  • CollecTor may run at any time of the hour, at which point the valid-after time of current consensuses and votes may already be up to 1 hour behind the current system time.
  • The votes that a consensus is based on are generated 10 minutes before the valid-after time, and they may contain server descriptors that have been published in the past 24 hours.
  • Directory authorities may serve an older consensus than the current consensus, say, one that is already 2 hours older than the current one.

All in all, CollecTor should attempt to fetch descriptors that are 27:10 hours old, or let's say 30 hours for simplicity and to account for cases we didn't consider here.

The downside is that missing descriptors will be retried for 6 more hours, but that doesn't seem to be that much of a problem, given that missing descriptors will be retried in batches of up to 96.

Here's a trivial patch:

diff --git a/src/main/java/org/torproject/collector/relaydescs/ b/src/main/java/org/torproject/collector/relaydescs/
index f4e38f4..21b1ee4 100644
--- a/src/main/java/org/torproject/collector/relaydescs/
+++ b/src/main/java/org/torproject/collector/relaydescs/
@@ -185,7 +185,9 @@ public class RelayDescriptorDownloader {
    * Cut-off time for missing server and extra-info descriptors, formatted
    * "yyyy-MM-dd HH:mm:ss". This time is initialized as the current system
-   * time minus 24 hours.
+   * time minus 30 hours (24 hours for the maximum age of descriptors to be
+   * referenced plus 6 hours for the time between generating votes and
+   * processing a consensus).
   private String descriptorCutOff;
@@ -330,7 +332,7 @@ public class RelayDescriptorDownloader {
     long now = System.currentTimeMillis();
     this.currentValidAfter = format.format((now / (60L * 60L * 1000L))
         * (60L * 60L * 1000L));
-    this.descriptorCutOff = format.format(now - 24L * 60L * 60L * 1000L);
+    this.descriptorCutOff = format.format(now - 30L * 60L * 60L * 1000L);
     this.currentTimestamp = format.format(now);
     this.downloadAllDescriptorsCutOff = format.format(now
         - 23L * 60L * 60L * 1000L - 30L * 60L * 1000L);

Child Tickets

Change History (13)

comment:1 Changed 4 years ago by karsten

Note: We could easily move this ticket to 1.1.0 and resolve it together with #8799.

comment:2 Changed 4 years ago by iwakeh

Milestone: CollecTor 1.0.0CollecTor 1.1.0

Changed milestone as suggested.

comment:3 Changed 4 years ago by iwakeh

Owner: set to iwakeh
Status: newassigned

assigning these issues.

comment:4 Changed 4 years ago by iwakeh

Milestone: CollecTor 1.1.0CollecTor 1.2.0

comment:5 Changed 4 years ago by iwakeh

Parent ID: #8799

comment:6 Changed 3 years ago by karsten

Milestone: CollecTor 1.2.0

We're planning to put out 1.2.0 in 5 days from now, and including this ticket in that release seems too ambitious. Removing from milestone for now.

comment:7 Changed 3 years ago by iwakeh

Parent ID: #8799

removing the parent ticket, as it seems unrelated and still belongs milestone 1.2.0

comment:8 Changed 3 years ago by iwakeh

Owner: changed from iwakeh to metrics-team

Move to metrics-team as these are not worked on by me during the next week.

comment:9 Changed 7 months ago by karsten

Priority: LowMedium

Changing priority to medium. We should get it fixed at the same priority as the other tickets in this component.

comment:10 Changed 5 months ago by karsten

Status: assignedneeds_review

I just tried out the patch and compared it to an unpatched version. The effect is that the patched version downloaded 14 additional server descriptors and 14 additional extra-info descriptor that were published between 24:03 and 24:30 hours before the current system time. I still think it makes sense to apply this change.

Please review commit 555ad13 in my task-19828 branch which is the same patch as above, based on master, plus a change log entry.

comment:11 Changed 5 months ago by karsten

Cc: metrics-team added

comment:12 Changed 4 months ago by karsten

Status: needs_reviewmerge_ready

This trivial patch can as well go in without review. Setting to merge_ready, will merge as time permits.

comment:13 Changed 3 months ago by karsten

Resolution: fixed
Status: merge_readyclosed

Merged. Closing.

Note: See TracTickets for help on using tickets.