Opened 10 years ago

Closed 10 years ago

#2485 closed task (fixed)

Review Tor Weather's use of TorCtl

Reported by: mikeperry Owned by: mikeperry
Priority: Medium Milestone:
Component: Metrics/Tor Weather Version:
Severity: Keywords:
Cc: Actual Points: 2
Parent ID: Points: 2
Reviewer: Sponsor:


Just a note to perform a quick review of Tor Weather's use of TorCtl to make sure it appears sane.

Child Tickets

Change History (7)

comment:1 Changed 10 years ago by mikeperry

Owner: changed from mikeperry to kaner
Status: newassigned

Ok. I've finished a quick review of weather's use of TorCtl. It is pretty minimal in its use for the most part. There are some places where it manually parses descriptor and ns batches, but for the most part they seem ok. In one place, I noticed the check for "opt hibernating" was an without the prefix.

I also noticed that the bandwidth it was checking was the descriptor bandwidth and not the consensus bandwidth (connection.get_network_status()). We probably should be using the consensus values for things like determining the fastest nodes for t-shirts. But for slow node alerts, we may want to stick to using descriptor values.. Or maybe still use consensus values for them, too.

Finally, it looks like weather is heavily relying on descriptor presence. For this to work out, it really should set FetchUselessDescriptor 1 in its torrc, to ensure that it actually downloads descriptors for hibernating and downed nodes.

comment:2 Changed 10 years ago by Sebastian

It should also be ensured that weather doesn't rely on an "opt " prefix to any keyword anywhere, because those are scheduled to be removed and are, according to the specification, always optional.

comment:3 Changed 10 years ago by mikeperry

Actual Points: 2
actualpointsdone: 2
pointsdone: 2

comment:4 Changed 10 years ago by kaner

Fixed item 1 (included Sebastian's idea as well) and added documentation for item 3. See 245524b398566a1de572bbaee7122c4ceeb47a9c.

Item 2 (bandwidth checking) is still open. I'm unsure whether or not to update the "pointsdone" stuff.

comment:5 Changed 10 years ago by mikeperry

Kaner: ignore the pointsdone stuff. I should have created a new ticket for you to use for actually doing work. This ticket should have been used only to document what I did.

comment:6 Changed 10 years ago by mikeperry

Owner: changed from kaner to mikeperry

Kaner, I've opened Weather ticket #2526 for the bandwidth test, and marked it as an enhancement. I'm going to reassign this ticket to me and close it, for Agile accounting purposes (see for this, if you're interested).

comment:7 Changed 10 years ago by mikeperry

Resolution: fixed
Status: assignedclosed
Note: See TracTickets for help on using tickets.