bwauths learn to recognize Unmeasured=1 in consensus line and treat it differently

In #2286 (moved) we're going to change directory authorities to say something like "w Bandwidth=100 Capped=1" for relays that don't have enough Measured lines.

Torflow's BWAuthority needs to learn not to get snookered by the 100 into thinking that the relay's peers are all the other "Bandwidth=100" relays.

added component::core tor/torflow owner::mikeperry priority::high resolution::fixed status::closed type::task labels

Trac:
Cc: N/A to mikeperry

Trac:
Parent: N/A to #2286 (moved)

aagbsn: The change that needs to happen here is in the ConsensusTracker code. It calculates a consensus to descriptor ratio in _read_routers() for each router. Add some parsing code to pull out the "Capped" flag for routers, and those routers should get a hard-coded ratio of 1.0.

The RatioPercentileRestriction should then handle things as normal, without modifications.

aagbsn, any chance we could convince you to work on this? I hear it's easy. :)

Trac:
6131_capped_flag.patch

Set ratio_rank = 1 if Capped=1 is parsed from NS "w " line

Trac:
Status: new to needs_review

Two points from the review:

1. You transform the ratio_r[i] object into an integer 1 for the capped case. This will cause explosions. Better than trying to hack the ratio_rank directly, you should alter the ratio_r sorting to use a Router.get_unmeasured_bw() method to return the NetworkStatus bw value instead of desc_bw if unmeasured is true... Then the ratio will work out to 1 that way.

2. Please change "Capped" to "Unmeasured". See https://trac.torproject.org/projects/tor/ticket/2286#comment:45 for why.

Trac:
Status: needs_review to needs_revision

Replying to mikeperry:

Two points from the review:

1. You transform the ratio_r[i] object into an integer 1 for the capped case. This will cause explosions.

Wait? Do you mean ratio_rank? Isn't that supposed to be an integer? If not, what about TorCtl.py:1647

   for i in xrange(len(ratio_r)): ratio_r[i].ratio_rank = i 

Better than trying to hack the ratio_rank directly, you should alter the ratio_r sorting to use a Router.get_unmeasured_bw() method to return the NetworkStatus bw value instead of desc_bw if unmeasured is true... Then the ratio will work out to 1 that way.

2. Please change "Capped" to "Unmeasured". See https://trac.torproject.org/projects/tor/ticket/2286#comment:45 for why.

Ok.

Trac:
6131_capped_flag_2.patch

Attempt 2

Trac:
Status: needs_revision to needs_review

Two more things:

1. w.groups(2) doesn't appear to do what you think it does. At least not in my python... I tested your regex in a shell your if statement always ends up true, regardless of if Capped=1 is present or not.

2. The consensus w keyword is ultimately going to be Unmeasured=1, not Capped=1. You need to update the regex for that, too.

Trac:
Status: needs_review to needs_revision

2. Whoops, good catch.
3. Hmm.

aagbsn@debian:~/code/torflow/TorCtl$ python
Python 2.6.6 (r266:84292, Dec 26 2010, 22:31:48) 
[GCC 4.4.5] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> import re
>>> nsline = "w Bandwidth=100 Unmeasured=1"
>>> m = re.search(r"^w Bandwidth=(\d+)(?:\s(Unmeasured)=1)?", nsline, re.M)
>>> m.groups()
('100', 'Unmeasured')
>>> nsline = "w Bandwidth=100"
>>> m = re.search(r"^w Bandwidth=(\d+)(?:\s(Unmeasured)=1)?", nsline, re.M)
>>> m.groups()
('100', None)
>>> nsline = "w Bandwidth=100 Unmeasured=0"
>>> m = re.search(r"^w Bandwidth=(\d+)(?:\s(Unmeasured)=1)?", nsline, re.M)
>>> m.groups()
('100', None)
>>> 

Trac:
6131_capped_flag_3.patch

attempt 3

Did attempt 3 look ok?

Trac:
Status: needs_revision to needs_review

I think this looks good. Merge it and be sure it starts running on a bw auth for a while, and verify those results seem sane?

Karsten has scripts for comparing the bw auth votes in the metrics repo somewhere. Perhaps he can point you at them. In the past, I've found them very useful for ensuring new bandwidth auth changes produced results in line with older code running on other bw auths.

Trac:
Cc: mikeperry to mikeperry, karsten

Replying to mikeperry:

Karsten has scripts for comparing the bw auth votes in the metrics repo somewhere. Perhaps he can point you at them. In the past, I've found them very useful for ensuring new bandwidth auth changes produced results in line with older code running on other bw auths.

Was that #2394 (moved)? If so, the code is in the metrics-tasks repo in task-2394/.

Trac:
Summary: bwauths learn to recognize Capped=1 in consensus line and treat it differently to bwauths learn to recognize Unmeasured=1 in consensus line and treat it differently

moria1's bwauth is now running with this patch

Removing parent because we're about to merge #2286 (moved).

We should merge this patch into torflow soon!

Trac:
Parent: #2286 (moved) to N/A

moria1's bwauth is still running this patch. The world hasn't ended yet.

Can we please now merge this? #2286 (moved) is merged .

If we can't, what's blocking this?

Trac:
Priority: normal to major

Trac:
Cc: mikeperry, karsten to mikeperry, karsten, aagbsn

I can't commit this code because: W access for pytorctl DENIED to aagbsn.

mikeperry?

Trac:
Status: needs_review to assigned
Owner: aagbsn to mikeperry

Pushed.

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

closed

mentioned in issue #6133 (moved)

moved to tpo/network-health/torflow#6131 (closed)