Opened 10 years ago

Closed 10 years ago

Last modified 8 years ago

#1564 closed defect (fixed)

Reported dirreq-v3-stats are 0.00%

Reported by: karsten Owned by: karsten
Priority: Medium Milestone:
Component: Core Tor/Tor Version:
Severity: Keywords: tor-relay
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

Since late May 2010, the dirreq-v3-stats that directory mirrors report are very low or even 0.00%. Mike has identified the cause for this bug:

For the love of Chaos, router_get_my_share_of_directory_requests() is
the dirtiest hack I've seen in a while, no wonder it broke :)

Your problem is that the global variables you rely on are not set by
smartlist_choose_by_bandwidth_weights(), which is the new
implementation of the weighted selection algorithm. They are only set
by smartlist_choose_by_bandwidth(). You could update
smartlist_choose_by_bandwidth_weights() to set those variables, but
man, there's got to be a better way :)

We should investigate new approaches to calculate directory request shares from the descriptor archives instead of believing in what directories tell us. Afterwards, we can decide whether to fix dirreq-v3-stats in Tor, or to take them out entirely.

Child Tickets

Attachments (1)

patch1564.txt (1.7 KB) - added by karsten 10 years ago.
Possible patch

Download all attachments as: .zip

Change History (9)

comment:1 Changed 10 years ago by karsten

Let me recap what happens here:

  • router_get_my_share_of_directory_requests() calls
    • router_pick_directory_server() which calls
      • router_pick_directory_server_impl() which calls
        • routerstatus_sl_choose_by_bandwidth() which calls
          • smartlist_choose_by_bandwidth_weights() first and if that fails
          • smartlist_choose_by_bandwidth(), where the latter sets
            • sl_last_weighted_bw_of_me and
            • sl_last_total_weighted_bw that we use to calculate shares.

Mike: Your suggestion is to have smartlist_choose_by_bandwidth_weights() set the two global variables? Sounds fine to me. Does the attached *untested* patch do what you have in mind?

What alternatives are there? We need a fix very soon, like today. And this whole dirreq-share code needs to stay in Tor for a couple more months only until we have a good way to calculate shares from other information and are confident that the new approach gives us good numbers. Let's not move too much code around to make this happen.

Changed 10 years ago by karsten

Attachment: patch1564.txt added

Possible patch

comment:2 Changed 10 years ago by mikeperry

At a glance, I think the patch looks ok. though I still think this probably should be computed offline using consensus docs.. The main reason I think it should be done offline is because using these global variables isn't very amenable to modeling both 0.2.1.x clients and 0.2.2.x clients using the linear combination of those respective results.

Since we know it can be computed offline, it's not like we're losing any data to wait to do this right, are we?

comment:3 Changed 10 years ago by karsten

Let's merge this patch now in order to get our estimates of recurring users back. We can still work on other approaches like computing shares offline or finding even better user number estimates. But right now, the directory shares are our only way to estimate user numbers, so let's fix them first.

Jake tested this patch on trusted which then reported non-zero directory request shares again.

The patch is in branch fix-1564 in my public repository.

comment:4 Changed 10 years ago by nickm

Looks okay to me; merged.

comment:5 Changed 10 years ago by karsten

Resolution: fixed
Status: newclosed

comment:6 Changed 9 years ago by karsten

Milestone: Calculate directory request shares

Removing the milestone. This isn't how milestones are supposed to work. "Calculate directory request shares" is a task or maybe a project, but not a milestone. The next time we should use a parent ticket instead. I'm not creating one now, because nobody cares anymore.

comment:7 Changed 8 years ago by nickm

Keywords: tor-relay added

comment:8 Changed 8 years ago by nickm

Component: Tor RelayTor
Note: See TracTickets for help on using tickets.