Opened 9 months ago

Closed 9 months ago

#24681 closed enhancement (fixed)

Make the default fallback weight in Tor 10.0

Reported by: teor Owned by: teor
Priority: Medium Milestone: Tor: 0.3.3.x-final
Component: Core Tor/Tor Version: Tor: 0.2.8.1-alpha
Severity: Normal Keywords: fallback, easy, intro, 032-backport-maybe, 031-backport-maybe, 030-backport-maybe, 029-backport-maybe, review-group-28
Cc: Actual Points: 0.1
Parent ID: #22271 Points: 0.1
Reviewer: pastly Sponsor:

Description (last modified by teor)

This is a follow-up to #24679.

  • update the default weight in parse_dir_fallback_line() to 10.0
  • update the man page to reflect the new default

10.0 gives us:

  • 0.5% clients bootstrapping off an authority when all fallbacks are up, * 2% when 25% are down (our replacement threshold)
  • 7% when 40% are down (our worst case scenario)

Child Tickets

Change History (16)

comment:1 Changed 9 months ago by teor

Keywords: easy intro added

comment:2 Changed 9 months ago by teor

Description: modified (diff)

comment:3 Changed 9 months ago by teor

Owner: set to teor
Status: newassigned

comment:4 Changed 9 months ago by teor

Actual Points: 0.1
Description: modified (diff)
Keywords: 031-backport-maybe 030-backport-maybe 029-backport-maybe 028-backport-maybe added
Points: 0.1
Status: assignedneeds_review
Version: Tor: 0.2.8.1-alpha

Please see my branch ticket24681_028, which is based on maint-0.2.8, just in case we decide to backport that far.

I actually ended up changing DirAuthorityFallbackRate to 0.1, which is a one-line config change. (The comment and the man page change are longer.)

Given how simple the patch is, I think we should backport it all the way back to whatever versions we plan to release with the new fallback list.

comment:5 Changed 9 months ago by nickm

Keywords: review-group-28 added

comment:6 Changed 9 months ago by teor

0.2.8 is EOL, so we won't be backporting to it.

comment:7 Changed 9 months ago by teor

Keywords: 028-backport-maybe removed

0.2.8 is EOL, so we won't be backporting to it.

comment:8 Changed 9 months ago by pastly

Status: needs_reviewmerge_ready

Code LGTM. No comment on whether or not 0.1 is a good value other than I trust teor's judgement and math.

comment:9 Changed 9 months ago by teor

(0.1 has worked for us so far, it's the equivalent of "weight=10" in the current fallback list, which we want to remove in #24679.)

comment:10 Changed 9 months ago by teor

Reviewer: pastly
Sponsor: Sponsor8-can
Status: merge_readyneeds_review

I'd like nickm to review this too.

comment:11 Changed 9 months ago by nickm

This will fail "make check-spaces"; otherwise it seems fine to me. I'm guessing that we should merge this when we make the change that does the "weight=10" removal in fallback_dirs.inc, and not before?

comment:12 Changed 9 months ago by nickm

I've fixed the wide line in a new branch teor_ticket24681_028, which I'll push to my public repository once git-rw is back up.

comment:13 in reply to:  11 Changed 9 months ago by teor

Replying to nickm:

This will fail "make check-spaces";

Oh, oops!

otherwise it seems fine to me. I'm guessing that we should merge this when we make the change that does the "weight=10" removal in fallback_dirs.inc, and not before?

We can merge this earlier or at same time as we merge the weight removal branch in #22271 to master.
And we can backport this earlier or at the same time as we backport the newly generated list with no weights in #24801.

But the timing is really not that important, if we do it out of sync and try authorities at 1x or 0.01x the fallback rate, no-one is likely to notice.
If I had to pick one, I'd go for lower load on the authorities, which is "merge this ticket before the other tickets".
(Clients already try authorities after 3 fallback failures, so having them in the fallback list is probably redundant. Except in configs where we remove all the default fallbacks!)

comment:14 Changed 9 months ago by nickm

okay, that's persuasive. merging this to 0.2.9 and forward.

comment:15 Changed 9 months ago by nickm

Done!

comment:16 Changed 9 months ago by teor

Resolution: fixed
Status: needs_reviewclosed

This was merged to 0.2.9 and later, so we're done here.

Note: See TracTickets for help on using tickets.