sbws should show relays for diagnostics, even when MIN_REPORT has not been reached

Hi juga,

There's a serious bug in #28563 (moved): sbws is not reporting any relays until MIN_REPORT has been reached.

But we want sbws to always report every relay for diagnostics. If the bandwidth file doesn't have minimum_number_eligible_relays, they should all be marked as "under_min_report=1 vote=0".

https://github.com/torproject/sbws/blob/6070d36022dc2130518dd0c68332166b2bf76c73/sbws/lib/v3bwfile.py#L1352

changed milestone to %sbws: 1.1.x-final

Trac:
Parent Ticket: #28563 (moved)

added changes-version-minor component::core tor/sbws milestone::sbws: 1.1.x-final owner::juga parent::28563 priority::medium resolution::fixed reviewer::nickm sbws-1.0-must-moved-20181128 sbws-110-proposed sbws-11x-final-removed-20190312 severity::normal status::closed tor-bwauth type::defect labels

Here is the spec for this change: https://github.com/torproject/torspec/pull/68/commits/bce5edc7382bfe229f9b56124574b97df1939c1b

Sorry, i forgot about this case, will work on it soon.

Trac:
Status: assigned to accepted

PR: ​https://github.com/torproject/sbws/pull/355

Trac:
Status: accepted to needs_review

Added a last commit to include the bandwidth file generated by the testnet that could be checked in #29727 (moved)

Trac:
Reviewer: N/A to nickm

LGTM. Before merging, a question: -- the last time that we marked anything as vote=0, we also marked it as unmeasued=1, bw=1. Do we need to do that in this case?

Trac:
Status: needs_review to merge_ready

Replying to nickm:

LGTM. Before merging, a question: -- the last time that we marked anything as vote=0, we also marked it as unmeasued=1, bw=1. Do we need to do that in this case?

But the relays are measured. There just aren't enough of them to have a reasonable set of weighted bandwidths across the network.

So the spec says vote=0 under_min_report=1 bw=1 real_bw=N: https://github.com/torproject/torspec/pull/68/commits/bce5edc7382bfe229f9b56124574b97df1939c1b Which means that authorities without #29806 (moved) will vote bandwidth 1 for these relays.

But we might want legacy authorities to vote bandwidth=(the measured bandwidth) or bandwidth=(the reported bandwidth) for these relays.

If 3 or more bandwidth authorities are already voting measured bandwidths for most relays, it doesn't matter what we choose. Even if we choose bw=1, the unmeasured relays just have a slightly lower median in their vote.

But if we're just re-starting the bandwidth authorities after many of them have failed, here's what happens when we vote bw=

• 1: measured relays get a consensus weight of 1 once 3 authorities know about them
  • then there is a big change in weight once 2 authorities have measured 60% of relays, and start voting measured bandwidths
• the reported bandwidth: measured relays get a consensus weight of their reported bandwidth once 3 authorities have measured them
  • then there is a smaller change to the measured weight once 2 authorities have measured 60% of relays, and start voting measured bandwidths
  • relays can lie about their bandwidth more easily
• the measured bandwidth:measured relays get a consensus weight of their measured bandwidth once 3 authorities have measured them
  • there is no change once 2 authorities have measured 60% of relays, and start voting measured bandwidths

So I think we should put measured bandwidths in the bandwidth file in this case. (And we don't need real_bw.) I'll do a draft spec update, What do you think, juga, nickm?

Trac:
Status: merge_ready to needs_revision

Replying to teor: [...]

So I think we should put measured bandwidths in the bandwidth file in this case. (And we don't need real_bw.) I'll do a draft spec update, What do you think, juga, nickm?

Yeah, i think it's makes more sense to start voting with the actual measured (and filtered and scaled) bandwidth.

nickm said by irc that also agrees with teor's comment in https://trac.torproject.org/projects/tor/ticket/29853#comment:7.

2 fixups to change that, 1 fixup on i mistake i made, 1 fixup to update the test. Waited for CI which passes.

Trac:
Status: needs_revision to needs_review

I don't understand why the changes here didn't cause/require changes in the test files?

Replying to nickm:

I don't understand why the changes here didn't cause/require changes in the test files?

Yeah, sorry, this code is quite confusing, it really needs refactor/renaming and better comments.

So, we have the following cases:

• case 0: the number of eligible relays is over the 60% of the consensus, nothing is set.
• case 1: after filtering, there are not eligible relays, they will be set "vote=0 under_min_report = 1" and they will also have "unmeasured=1 bw=1" because they are not eligible.
• case 2: after scaling, the number of relays is less than 60% in the consensus, they will be set "vote=0 under_min_report =1", the won't have "unmeasured=1", but some of them may have "bw=1" just because that's the bandwdith after scaling

I added a test to check the 3 cases.

The changes in this branch, did require changes in other tests, in concrete:

• test_torflow_scale: i mocked consensus to pretend the network has only 1 relay so that the filtered relays are over the 60% and the scaling expected results continue working. Nothing regarding under_min_report changes here.

• test_from_results_read i hardcoded under_min_report because update_progress would be None and if not min_perc will set under_min_report

Ah. That makes sense. It looks okay to me now.

Trac:
Status: needs_review to merge_ready

squashed rebased and force pushed, then merged.

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

Move sbws 1.1.0 tickets into 1.1.x-final, to match Tor's milestone scheme.

Trac:
Milestone: sbws: 1.1.0 to sbws: 1.1.x-final

closed

mentioned in issue #29854 (moved)

moved to tpo/network-health/sbws#29853 (closed)