Reported dirreq-v3-stats are 0.00%
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.
Activity
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.
- routerstatus_sl_choose_by_bandwidth() which calls
- router_pick_directory_server_impl() which calls
- router_pick_directory_server() which calls
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.
- router_get_my_share_of_directory_requests() calls
Trac:
patch1564.txt
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?
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.
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.
Trac:
Points: N/A to N/A
Actualpoints: N/A to N/A
Milestone: Calculate directory request shares to N/A
