Bug in handling choke groups

[chros73]

I. Summary I just found it by accident (while I have been working on fixing partially done donwloads) that dealing with choke heuristics has been dramatically changed since libtorrent v0.13.0 but rtorrent (client) part left untouched since then (last good version is 0.8.9-0.12.9 in this regard).

• until 0.8.9 it worked as a per download setting
• since 0.9.0 it should work as a group setting
• choke groups feature is enabled and working (means it's actively used by resource_manager) in libtorrent since v0.13.0
  • only 1 group is created, called default
• rtorrent hasn't been modified (just a bit to don't get broken)
  • if you want to modify group for a download then it affects all the downloads
  • only 1 group is used (default) for all the torrents, means upload_leech heuristics is used for seeding as well
  • setting choke_group.up.max doesn't work

It works fine by default until somebody wants to change the config - there's a default choke group - (a hack was applied for integrating into the client).

II. Bugs

• 1. all the old choke_heuristics code is unnecessary in command_download.cc
• 2. there are unnecessary code in resume(), confirm_finished() part of core/download_list.cc
• 3. protocol.choke_heuristics.up.seed hasn't been updated to upload_seed in command_network.cc , but I think it's not needed anymore
• 4. the new d.group property isn't saved and not initialized with the download in core/download_factory.cc , old code can be removed
• 5. d.group.name command gives back index instead of the name of the group
• 6. UI only displays the index of the group not the name in ui/download.cc
• 7. all the old choke_heuristics code is unnecessary in libtorrent@torrent/download.h and libtorrent@torrent/download.cc

III. Question

• 8. what does USE_CHOKE_GROUP 0 mean in this context in command_groups.cc? What isn't and what is used in this case? :)

Edit: I've updated the description of the issue. Edit2: I've updated the description of the issue, again.

[chros73]

Wow, this is indeed an interesting feature! Thanks for mentioning it @rakshasa (here), otherwise I'd have never noticed it :)

I'll create a pull request for this that will also include Fixing partial download support mods (these 2 issues are depending on each other).

I. Summary This is a powerful feature that should be used if you deal with group of torrents (e.g. you use throttle groups) like here. It allows to set 3 major property per group (2 others (up, down) per property, except for tracker.mode):

• choke_group.tracker.mode : decides on how aggressive a tracker should be
• choke_group.up.heuristics : set the heuristics used when deciding on which peers to choke and unchoke
• choke_group.up.max : set the max total number of unchoked peers for all torrents in this choke group

The last property is the really interesting one! It restricts the number of unchoked peers in a group!

II. Fixes for mentioned bugs

• 1. removed old code from command_download.cc
• 2. added new logic and removed old code from core/download_list.cc@resume() and core/download_list.cc@confirm_finished()
• 3. removed old code from core/download_list.cc
• 4. removed old code from core/download_factory.cc
• 5. fixed d.group_name in command_download.cc
• 6. fixed and modified UI on Info screen in ui/download.cc
• 7. removed old code from libtorrent@torrent/download.*
• 8. enabled real choke group code in command_groups.cc and got rid of the hack code (also in core/download.h)

III. Additions

• renamed default choke group default to default_leech in libtorrent@manager.cc
• added a new default choke group called default_seed in main.cc
• added new helper UI command convert.group has been created in command_ui.cc (similarly to convert.throttle)
• modify UI on Info screen to include all the statistics that belongs to a group
  • Choke group: default_seed [upload_seed, download_leech, aggressive] [Max --/--]
  • Choke group stat: [Size 149] [Unchoked 18/0] [Queued 0/0] [Total 18/0] [Rate 952.3/ 0.0 KB]

IV. Other fixes

• fixed saving total_skipped between sessions in core/download_factory.cc and core/download_store.cc (used on Info screen)

V. Choke groups in action I'll update the wiki page if it will be merged, but until then here's a teaser: let's extend the Favoring group of torrents example to include public (!!!) torrents as well! :)

• we add one more throttle group for public torrents, called tardyup, along with slowup
• let's create 4 more choke groups (slowup_leech, slowup_seed, tardyup_leech, tardyup_seed) along with the built-in 2 ones (default_leech, default_seed)
  • name them after the corresponding throttle groups!
• assign the different groups to the proper download with the corresponding throttle group all the time!
  • it's just 4 lines of code when the above naming convention is used! :)
• we can also modify the choke_group.up.max setting on the fly (!) with a help of an external script (like we did with the throttle speed)
  • external script mod isn't included, so this part work work in this way

# Define throttle groups
throttle.up = slowup,600
throttle.up = tardyup,300

# Setting up choke groups that restricts the number of unchoked peers in a group
# Modify default choke groups for specail group
choke_group.up.heuristics.set = default_leech,upload_leech_experimental
choke_group.tracker.mode.set  = default_leech,aggressive
choke_group.tracker.mode.set  = default_seed,aggressive
# Set up choke groups for slowup group
choke_group.insert = slowup_leech
choke_group.insert = slowup_seed
choke_group.up.heuristics.set = slowup_leech,upload_leech
choke_group.up.heuristics.set = slowup_seed,upload_seed
choke_group.down.max.set = slowup_leech,200
choke_group.up.max.set   = slowup_leech,200
choke_group.up.max.set   = slowup_seed,125
# Set up choke groups for tardyup group
choke_group.insert = tardyup_leech
choke_group.insert = tardyup_seed
choke_group.up.heuristics.set = tardyup_leech,upload_leech
choke_group.up.heuristics.set = tardyup_seed,upload_seed
choke_group.down.max.set = tardyup_leech,150
choke_group.up.max.set   = tardyup_leech,150
choke_group.up.max.set   = tardyup_seed,75

# helper method: Sets choke group to one of the default ones if there's no throttle or throttle is special one (NULL) otherwise sets it to one of the throttle name ones
method.insert = d.modify_choke_group, simple, "branch=((and,((d.throttle_name)),((not,((equal,((d.throttle_name)),((cat,NULL)))))))),((d.group.set,(cat,(d.throttle_name),_,(d.connection_current)))),((d.group.set,(cat,default_,(d.connection_current))))"
# Modify choke group when a torrent is resumed (even after hashchecking or after rtorrent is restarted) or finished or partially restarted
method.set_key = event.download.resumed, modify_resumed_choke_group, "d.modify_choke_group="
method.set_key = event.download.finished, modify_finished_choke_group, "d.modify_choke_group="
method.set_key = event.download.partially_restarted, modify_partially_restarted_choke_group, "d.modify_choke_group="

# helper method: gets one of the below info with the help of getLimits.sh script (Variables are inside the script, you have to edit those values there!)
method.insert = get_limit, simple|private, "execute.capture=\"$cat=$pyro.bin_dir=,getLimits,$cfg.postfix=,.sh\",$argument.0=,$argument.1=,$argument.2=,$argument.3="
# Adjust max setting on the fly
schedule2 = adjust_slots_slowup,  47, 100, "choke_group.up.max.set=slowup_seed,\"$get_limit=$cat=slslots,$convert.kb=$throttle.global_up.rate=,$convert.kb=$throttle.up.rate=slowup,$convert.kb=$throttle.up.rate=tardyup\""
schedule2 = adjust_slots_tardyup, 48, 100, "choke_group.up.max.set=tardyup_seed,\"$get_limit=$cat=taslots,$convert.kb=$throttle.global_up.rate=,$convert.kb=$throttle.up.rate=slowup,$convert.kb=$throttle.up.rate=tardyup\""

[chros73]

Let's modify the last bullet point (and the last 5 lines in the above config snippet) with the help of our new shiny math commands:

• ditch our external script completely
• define all the required variables in rtorrent config
• rewrite all existing methods and add new ones
• include tardyup throttle group in calculations

Adding couple of lines to the config, we get an extremely powerful system that is working like a charm! :)

## Variables for getting upload rate limit for throttle groups and max number of upload/download slots for choke groups
# Max, Min value of uprate limit throttle in KB and Max number of upload/download slots during leeching and seeding for slowup throttle group
method.insert = cfg.slowup.uprate.max,      value,  1600
method.insert = cfg.slowup.uprate.min,      value,    75
method.insert = cfg.slowup.slots.leech.max, value,   200
method.insert = cfg.slowup.slots.seed.max,  value,   125
# Max, Min value of uprate limit throttle in KB and Max number of upload/download slots during leeching and seeding for tardyup throttle group
method.insert = cfg.tardyup.uprate.max,     value,  1200
method.insert = cfg.tardyup.uprate.min,     value,    25
method.insert = cfg.tardyup.slots.leech.max,value,   150
method.insert = cfg.tardyup.slots.seed.max, value,    75

## Variables for getting global downrate limit
# Max, Min value of global downrate in KB
method.insert = cfg.global.downrate.max,    value,  9999
method.insert = cfg.global.downrate.min,    value,  8000
# Threshold values for global and special group uprate in KB
method.insert = cfg.global.upall.threshold, value,  1600
method.insert = cfg.global.upmain.threshold,value,  1100

# Min value of uprate per upload slot (unchoked peers) in KB
method.insert = cfg.global.slot.uprate.min, value,     5

# helper method: get current uprate in KB for special group
method.insert = get_uprate_main, simple, "math.max=0,(math.div,(math.sub,(throttle.global_up.rate),(throttle.up.rate,slowup),(throttle.up.rate,tardyup)),1024)"
# helper methods: get new uprate limit in KB (based on special group uprate) for slowup and tardyup throttle groups
method.insert = get_uprate_slowup,  simple|private, "math.min=(cfg.slowup.uprate.max),(math.max,(cfg.slowup.uprate.min),(math.sub,(cfg.slowup.uprate.max),(get_uprate_main),(math.div,(throttle.up.rate,tardyup),1024)))"
method.insert = get_uprate_tardyup, simple|private, "math.min=(cfg.tardyup.uprate.max),(math.max,(cfg.tardyup.uprate.min),(math.sub,(cfg.tardyup.uprate.max),(get_uprate_main),(math.div,(throttle.up.rate,slowup),1024)))"
# Set new uprate limit for slowup and tardyup throttle groups in every 20 seconds
schedule2     = adjust_throttle_slowup,  14, 20, "throttle.up=slowup,(cat,(get_uprate_slowup))"
schedule2     = adjust_throttle_tardyup, 15, 20, "throttle.up=tardyup,(cat,(get_uprate_tardyup))"

# helper method: get new global downrate limit in KB (based on special group uprate and given threshold values)
method.insert = get_downrate_global, simple|private, "branch=(and,((greater,((math.div,(throttle.global_up.rate),1024)),((cfg.global.upall.threshold)))),((greater,((get_uprate_main)),((cfg.global.upmain.threshold))))),(cfg.global.downrate.min),(cfg.global.downrate.max)"
# Set new global downrate limit in every 60 seconds
schedule2     = adjust_throttle_global_down_max_rate, 54, 60, "throttle.global_down.max_rate.set_kb=(get_downrate_global)"

# helper methods: get new upload slots limit for slowup_seed and tardyup_seed choke groups (based on their current uprate and the given cfg values)
method.insert = get_slots_slowup,  simple|private, "math.min=(cfg.slowup.slots.seed.max),(math.max,(math.div,(cfg.slowup.uprate.min),(cfg.global.slot.uprate.min)),(math.div,(throttle.up.rate,slowup),1024,(cfg.global.slot.uprate.min)))"
method.insert = get_slots_tardyup, simple|private, "math.min=(cfg.tardyup.slots.seed.max),(math.max,(math.div,(cfg.tardyup.uprate.min),(cfg.global.slot.uprate.min)),(math.div,(throttle.up.rate,tardyup),1024,(cfg.global.slot.uprate.min)))"
# Set new upload slots (unchoked peers) limit for slowup_seed and tardyup_seed choke groups in every 100 seconds
schedule2     = adjust_slots_slowup,  47, 100,"choke_group.up.max.set=slowup_seed,(cat,(get_slots_slowup))"
schedule2     = adjust_slots_tardyup, 48, 100,"choke_group.up.max.set=tardyup_seed,(cat,(get_slots_tardyup))"

[pyroscope]

Quiet some of these should probably be method.insert.value.

[chros73]

Quiet some of these should probably be method.insert.value

I had to look up what you mean:

• method.insert.value is a shorthand for method.insert = <name>, value (without any additional flag, e.g. static, private, const)

You probably thought of one of the method.insert = <name>, simple rows. We discussed this about half a year ago that these should be type simple. Now I took a look why:

• rpc::convert_to_value will throw an exception stating not able to convert string type to value
• system_method_generate_command2 is called when a method is simple type, so it's behaviour is completely different

These types actually are not like a return type of a method but indicators about what type of data is stored in a certain method and how rtorrent should handle it.

[chros73]

Also note that choke_group.up.max.set= and throttle.up= accepts only string type not value (hence the usage of cat command in these cases), the latter only handles "value" in KB, while throttle.global_down.max_rate.set*= accept both value and string type. :)

[chros73]

@rakshasa what is multi type for?

[pyroscope]

multi is an array type that is e.g. used for events (they take "multi"ple handler keys).

[trim21]

it's funny d.group is not saved, because choke groups themselves are not saved.

If a choke group is added by rpc but not config, and if we save d.group to session, its choke group will be missing after restart.

But if we save choke group to session, choke_group.insert in config file will fail.