Opened 5 years ago

Last modified 2 years ago

#11327 needs_revision defect

Dir auths should choose Fast and Guard flags by consensus weight if they don't measure

Reported by: arma Owned by: TvdW
Priority: High Milestone: Tor: unspecified
Component: Core Tor/Tor Version: Tor: 0.2.7
Severity: Normal Keywords: needs-proposal tor-dirauth
Cc: aagbsn, asn Actual Points:
Parent ID: Points: 4
Reviewer: Sponsor: SponsorU-can

Description

In #8435 we made directory-authorities-that-run-bwauths stop voting Fast or Guard for relays they hadn't measured yet.

But as I pointed out in https://trac.torproject.org/projects/tor/ticket/8435#comment:13, since only a minority of dir auths run bwauths, the majority of dir auths are still voting Fast and Guard based on descriptor bandwidths.

So while the title of ticket #8435 says "Ignore advertised bandwidths for flags once we have enough measured bandwidths", the ChangeLog entry is more accurate:

    - Directory authorities that have more than a threshold number
      of relays with measured bandwidths now treat relays with unmeasured
      bandwidths as having bandwidth 0. Resolves ticket 8435.

We should at some point actually do the original goal, which is to give Fast to the 7/8s of relays whose consensus weights are highest, and Guard to the 1/2 of relays whose consensus weights are highest and who match the other guard constraints.

Child Tickets

Change History (34)

comment:1 Changed 5 years ago by arma

One suggestion is to look at the previous consensus, and use the number out of it, if we don't have a measurement of our own.

There are two versions of that idea: one is that we use the consensus weight iff we don't run a bwauth, and the other is that we use the consensus weight if we don't run a bwauth or if we do but haven't measured that relay yet. I'm ok with either, but the former sounds a bit cleaner.

comment:2 Changed 5 years ago by nickm

Keywords: 026-triaged-1 added

comment:3 Changed 4 years ago by nickm

Milestone: Tor: 0.2.6.x-finalTor: 0.2.7.x-final

comment:4 Changed 4 years ago by nickm

Status: newassigned

comment:5 Changed 4 years ago by nickm

Keywords: 027-triaged-1-in added

Marking some tickets as triaged-in for 0.2.7 based on early triage

comment:6 Changed 4 years ago by isabela

Keywords: SponsorU added
Points: small/medium
Priority: normalmajor
Version: Tor: 0.2.7

comment:7 Changed 4 years ago by arma

I just closed #12877 as an overlap with this one.

comment:8 Changed 4 years ago by aagbsn

Cc: aagbsn added

comment:9 Changed 4 years ago by nickm

Milestone: Tor: 0.2.7.x-finalTor: 0.2.8.x-final

comment:10 Changed 4 years ago by nickm

Keywords: SponsorU removed
Sponsor: SponsorU

Bulk-replace SponsorU keyword with SponsorU field.

comment:12 Changed 4 years ago by TvdW

Status: assignedneeds_review

comment:13 Changed 4 years ago by arma

Severity: Normal
Status: needs_reviewneeds_revision

I just pushed some more commits that refactor that function and comment it better. They're in my 'ticket11327' branch.

(I think they are on top of tvdw's latest branch, but somebody should check it to be sure.)

And now that I've refactored it, a question becomes clearer:

       if (node->rs && node->rs->bandwidth_kb) {
+        /* Case 3: Not running a bwauth, and we know a weight from the
+         * consensus. Use that one (so e.g. we won't vote Fast on a relay
+         * that the consensus has given very low weight to). */
         bw_kb = node->rs->bandwidth_kb;
       }

Is the 'if' part the right way to check that we have a consensus? It seems like "we have a consensus but it lists this relay's weight as 0" will be mistaken as "no we don't have a consensus". Nick did the ri, rs, etc refactorings long ago, so he is the perfect person to answer/improve this part.

And then once we're set there, I could imagine another refactoring where we take out the bw_kb variable and just do a 'return' for each case.

comment:14 Changed 3 years ago by nickm

Owner: set to TvdW
Status: needs_revisionassigned

setting TvdW as the owner of this needs_revision ticket.

comment:15 Changed 3 years ago by nickm

Status: assignedneeds_revision

comment:16 Changed 3 years ago by TvdW

A while ago someone (I think arma/nickm) mentioned we should look into whether we can simply disable Fast/Guard voting if an authority doesn't have enough measurements yet.

After implementing this thought and testing it it turns out this may not be a good idea: the network would lose all fast/guard flags if there are insufficient bandwidth authorities. Also, Chutney would stop working.

I think this is better handled on a consensus method level, or we could choose to get rid of the Fast flag completely and have the client determine it instead.

In any case, I think having the authorities take data from a previous consensus is not the correct solution to this problem.

comment:17 Changed 3 years ago by nickm

Milestone: Tor: 0.2.8.x-finalTor: 0.2.9.x-final

These seem like features, or like other stuff unlikely to be possible this month. Bumping them to 0.2.9

comment:18 Changed 3 years ago by isabela

Sponsor: SponsorUSponsorU-can

comment:19 Changed 3 years ago by TvdW

Quick update from IRC: we talked about alternatives and one thing that came up is to have the consensus generation process take care of this instead, as only there do we have enough information to make the right call about whether to take from all bandwidth authorities or only the ones with bandwidth scanners.

This would effectively introduce first and second class dirauths: first class dirauths vote for these flags, and the second class dirauths' opinion only counts if there aren't enough voters around.

comment:20 Changed 3 years ago by nickm

Keywords: tor-guards-revamp added

comment:21 Changed 3 years ago by isabela

Points: small/medium2

comment:22 Changed 3 years ago by asn

Cc: asn added

comment:23 Changed 3 years ago by nickm

Keywords: nickm-deferred-20160905 added
Milestone: Tor: 0.2.9.x-finalTor: 0.2.???

Deferring many tickets that are in needs_revision with no progress noted for a while, where I think they could safely wait till 0.3.0 or later.

Please feel free to move these back to 0.2.9 if you finish the revisions soon.

comment:24 Changed 3 years ago by teor

Milestone: Tor: 0.2.???Tor: 0.3.???

Milestone renamed

comment:25 Changed 2 years ago by nickm

Keywords: tor-03-unspecified-201612 added
Milestone: Tor: 0.3.???Tor: unspecified

Finally admitting that 0.3.??? was a euphemism for Tor: unspecified all along.

comment:26 Changed 2 years ago by nickm

Keywords: tor-03-unspecified-201612 removed

Remove an old triaging keyword.

comment:27 Changed 2 years ago by nickm

Keywords: 027-triaged-in added

comment:28 Changed 2 years ago by nickm

Keywords: 027-triaged-in removed

comment:29 Changed 2 years ago by nickm

Keywords: 027-triaged-1-in removed

comment:30 Changed 2 years ago by nickm

Keywords: 026-triaged-1 removed

comment:31 Changed 2 years ago by nickm

Keywords: nickm-deferred-20160905 removed

comment:32 Changed 2 years ago by dgoulet

Keywords: tor-dirauth added; tor-auth removed

Turns out that tor-auth is for directory authority so make it clearer with tor-dirauth

comment:33 Changed 2 years ago by nickm

Keywords: needs-proposal added; tor-guards-revamp removed
Points: 24

comment:34 Changed 2 years ago by nickm

See also #8435 , where some progress was made.

Note: See TracTickets for help on using tickets.