Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't force the trackers to scrape when rTorrent first starts #31

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

stickz
Copy link

@stickz stickz commented Nov 27, 2021

rTorrent will do this gradually, so we can start connecting to peers right away. This patch shaves approximately 5 minutes off the start time with thousands of torrents. It also addresses or resolves a whole host of issues associated with forced tracker updates at startup. Including these rakshasa/rtorrent#1044 and crashing at startup because of too many broken trackers.

rTorrent will do this gradually, so we can start connecting to peers right away. This patch shaves approximately 5 minutes off the start time with thousands of torrents. It also addresses or resolves a whole host of issues associated with forced tracker updates at startup.
@jesec
Copy link
Owner

jesec commented Nov 29, 2021

How would the client know the peers, if it does not connect to the tracker?

I am OK with the idea to reduce startup time. However, the current implementation would delay rTorrent from connecting to the trackers for all torrents added, including the new torrents added later by the user:

torrent::Object
Manager::try_create_download(const std::string& uri,
int flags,
const command_list_type& commands) {
auto result = torrent::Object::create_value();
// If the path was attempted loaded before, skip it.
if ((flags & create_tied) && !(flags & create_raw_data) &&
!is_network_uri(uri) && !is_magnet_uri(uri) && !is_data_uri(uri) &&
!file_status_cache()->insert(uri, flags & create_throw))
return result;
// Adding download.
DownloadFactory* f = new DownloadFactory(this);

Most users would not be happy to see their new downloads "stuck" for a prolonged period of time.

Additionally, most users expect their unfinished downloads loaded from the session directory to immediately resume downloading. Some users may as well expect the torrents to immediately join the swarm.

@stickz
Copy link
Author

stickz commented Nov 30, 2021

The alternative with thousands of torrents is to have the torrent client freeze for about 10 minutes, while it connects to all the trackers. During this period of time, no torrent activity can happen. And if there's too many dead trackers, rtorrent just crashes.

I tested this with network.max_open_sockets.set = 10000. It only took a couple extra minutes for 5000 torrents with UDP trackers to connect to the trackers, fetch the peers, join the swarm etc. The current implementation works good in that regard.

You did bring up a good point though about new torrents added by the user. I'm going to improve this feature with an initial load variable in the DownloadFactory class. When called from main.cc it will be set to true. This way, new torrents (added later by the user) will not skip connecting to trackers and fetching peers.

I'm also going to make this feature optional and disabled by default. I plan to add a command that can be used in .rtorrent.rc like trackers.skip_startup_scrape to turn this feature on. This way users can decide about enabling it.

@jesec
Copy link
Owner

jesec commented Nov 30, 2021

Note that any change to tracker related codes is high-risk, and would be subject to extensive review and testing. The change could be rejected outright on project direction grounds (instead of technical grounds).

The distribution does not wish to introduce change to consensus-layer, or otherwise cause any undesirable divergence of "in-swarm" behavior from vanilla rTorrent 0.9.8.

In any case, I do agree that the torrent client should not crash. I think it would be nice to find a proper solution to the problem (instead of hiding the issue with delayed scraping). Startup time is a valid issue as well, but I would prefer a lower risk approach (maybe non-blocking DNS resolution for UDP trackers).

Additionally, the current approach could make rTorrent to stop sending START events to trackers. That's a significant consensus behavior change.

@stickz
Copy link
Author

stickz commented Dec 1, 2021

These issues with rTorrent may never be resolved properly. Since this is a change to the consensus layer, how would you feel about making it optional and disabled by default? And adding a warning to the sample .rtorrent.rc config file that this change is experimental? This way users could use the change at their own risk.

The software basically becomes unusable when too many torrents are added to it. I think the problem needs to be treated like a zero-day exploit with a cover up first. Then if someone comes along and submits a better solution, it could be deprecated.

If you're okay with proceeding on these grounds, I'm willing to improve this pull request further. Starting by addressing the problem with new torrents added by the user, being delayed in connecting to trackers. It will just take some time to do so.

Elegant996 added a commit to Elegant996/rtorrent that referenced this pull request May 14, 2023
Elegant996 added a commit to Elegant996/rtorrent that referenced this pull request May 23, 2023
@Elegant996
Copy link

@jesec This became part of the vanilla rtorrent the other month per rakshasa/rtorrent@c86f0d0, this should bring it back in scope with the project.

Elegant996 added a commit to Elegant996/rtorrent that referenced this pull request Jul 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants