Fetch descriptors from both CollecTor instances

As discussed at the last team meeting, we should attempt to make Onionoo more robust by fetching descriptors from both CollecTor instances.

I started working on this by extending DescriptorSource and am currently trying it out. As soon as I have something to review, I'll post it here.

This change won't require a protocol change and can go into the next release.

changed milestone to %Onionoo 1.12.0

added component::metrics/onionoo milestone::Onionoo 1.12.0 owner::karsten priority::medium resolution::fixed reviewer::iwakeh severity::normal status::closed type::enhancement labels

Please review commit c7f62a3 in my task-25700 branch.

Trac:
Status: assigned to needs_review

Trac:

Somewhat related, though mostly as a motivation why this ticket matters, here's a graph of temporary downtimes of the primary CollecTor host in the first quarter of 2018:

Trac:
Reviewer: N/A to iwakeh

Just curious: what are the downtimes for collector2?

Much lower. But I can't make a graph of that. I took the data for this graph from consensus tarballs by plotting time differences between consensus valid-after time and the file's last-modified time. We don't have a similar archive from collector2, AFAIK.

Queues get added twice here.

When constructing recentDir the code is not necessarily platform independent, maybe use Paths.get.

As Onionooo should be made easier to configure soon and we decided on having one config file (in #24041 (moved)) maybe the construction of directories and supplying the hosts could be put in one place? This would also avoid repeating the path concatenation. Something like a list of host URL and in directory? (Just prepare and enable future config file use, not yet implementing it, of course.)

Trac:
Status: needs_review to needs_revision

Replying to karsten:

Much lower. But I can't make a graph of that. I took the data for this graph from consensus tarballs by plotting time differences between consensus valid-after time and the file's last-modified time. We don't have a similar archive from collector2, AFAIK.

Ah, thanks! No, collector2 doesn't archive.

I just made the first two changes, but while looking into the third change I discovered a potential bug: it might be that we're reading and parsing the entire set of recent descriptors 7 times and throwing away all but one descriptor type in each execution. I need to look closer at that, but I have to leave for lunch now.

Replying to karsten:

I just made the first two changes, but while looking into the third change I discovered a potential bug: it might be that we're reading and parsing the entire set of recent descriptors 7 times and throwing away all but one descriptor type in each execution. I need to look closer at that, but I have to leave for lunch now.

False alarm. I overlooked a line that prevents just that. Phew.

Alright, please find commit 5c96f5c in my task-25700 branch. I think that addresses all things you mentioned above. If not, please consider providing a fix, so that we can get this merged by end of today, if possible. I'm also currently running a local test.

Trac:
Status: needs_revision to needs_review

FWIW, local test succeeded.

Replying to karsten:

FWIW, local test succeeded.

Great, thanks! This looks fine, and, will make introducing the configuration improvement easier. Merge ready.

Trac:
Status: needs_review to merge_ready

Great! Squashed and merged to master. Closing. Thanks!

Trac:
Resolution: N/A to fixed
Status: merge_ready to closed

closed

mentioned in issue #25711 (moved)