arvidn / libtorrent

an efficient feature complete C++ bittorrent implementation
http://libtorrent.org
Other
5.17k stars 994 forks source link

Improving tracker announce logic and alerts #6844

Closed ghost closed 2 years ago

ghost commented 2 years ago

Please provide the following information

libtorrent version (or branch): RC2_0 platform/architecture:

compiler and compiler version:

please describe what symptom you see, what you would expect to see instead and how to reproduce it.

This is a problem faced by users with lots of torrents with multiple trackers. The issue is in regards to performance degradation and startup delays due to announce and alert spam in torrent client namely qBittorrent. Some versions ago announce queueing was added which doesn’t solve the problem properly due to some other drawbacks.

By default qBit does 50 concurrent announces and announces to all trackers in different tiers.

The problem arises when someone has lots of trackers, lets say 50 trackers per torrent. Has 4 local endpoints (only one has internet, others are localhost and some disconnected VPNs or wifi adapters). So this kind of combination results in 200 alerts at a time out of which 150 are useless or meaningless to process on client side since they actually do not have any connection. This could be avoided if user listened on a specific IP address but by default qBt listens on all interfaces as we can’t expect everyone to be tweaking such advanced settings. End result is wasted CPU/RAM trying to announce and process alerts on offline interfaces.

My 1st suggestion is, if a specific interface fails 5 times in a row due to no route to internet, skip announcing to it for the next 10 minutes. This should apply to all announces in queue and future announces within the timeframe.

2nd suggestion is to emit only one alert per tracker because as to this day I haven’t seen a client make actual use of the information from different endpoints. What they do is mark the tracker as working if any of the endpoints work and doesn’t care about the failing ones. If none of the endpoints work, emit a single error alert after the last endpoint fails. If any of the endpoints work, don’t emit any error alerts. Or at least make this configurable so a client can choose if they want to display error information from failed endpoints on the GUI even if one endpoint is working(which is what the user cares about).

arvidn commented 2 years ago

The source endpoints that can't reach the tracker are considered, but are not supposed to be attempted to connect to the tracker. However, they still post alerts saying that they did not announce.

I'm a little bit surprised that this would cause performance issues. Posting alerts is very cheap. They're relatively small and all allocated in contiguous memory that should be very quick to traverse.

The most realistic solution I can think of to limit the announces would be to classify these kinds of announce alerts differently, to make it possible to filter them with the alert_mask.

arvidn commented 2 years ago

considering this some more, I think the announce_skipped error is primarily useful for debugging and trouble shooting. But there's already debug logging alerts that can be enabled to see this.

Perhaps it makes sense to just not consider it an error and not post a tracker_error_alert.

How does this look? https://github.com/arvidn/libtorrent/pull/6851

ghost commented 2 years ago

I'm a little bit surprised that this would cause performance issues. Posting alerts is very cheap. They're relatively small and all allocated in contiguous memory that should be very quick to traverse.

Posting alert is cheap but processing thousands of junk alerts on client UI thread results in performance degradation.

ghost commented 2 years ago

How does this look? #6851

Seems to be working. Though I can't test if It improves any performance. Nevertheless it's a good change.

Seeker2 commented 2 years ago

Especially at startup, it'd be more useful to announce to 1-2 trackers per torrent initially just to get more torrents running quickly. Subsequent passes could announce more trackers per torrent at once.

glassez commented 2 years ago

Perhaps it makes sense to just not consider it an error and not post a tracker_error_alert.

How does this look? #6851

But announce_skipped still affect the "state" of announce_entry, doesn't it? If Yes, then #6851 looks bad, since it prevents application from know about this state changed event in the same way as in other cases but requires some polling of libtorrent using blocking calls.

glassez commented 2 years ago

The source endpoints that can't reach the tracker are considered, but are not supposed to be attempted to connect to the tracker. However, they still post alerts saying that they did not announce.

Could you explain it, please? Can you just skip such endpoints from being created? Or is there something affecting that is not known at the time of their creation?

arvidn commented 2 years ago

Could you explain it, please? Can you just skip such endpoints from being created? Or is there something affecting that is not known at the time of their creation?

technically you may have multiple trackers that resolve to different IPs that are reachable by different network. For example, you could have one tracker for internet peers and a separate tracker on the local network for internal peers.

But announce_skipped still affect the "state" of announce_entry, doesn't it?

I believe so. This is the reason I'm hesitant. Perhaps it would be possible to also suppress the "start" of announcing to hide both events affecting the announce_entry state, but it seems risky.

@summerqB Do you have any more tangible evidence about the cost of filtering alerts that aren't interesting? I'm skeptical that such cost is inherent in the filtering, but might be some problematic design decisions for performing the filtering.

glassez commented 2 years ago

technically you may have multiple trackers that resolve to different IPs that are reachable by different network. For example, you could have one tracker for internet peers and a separate tracker on the local network for internal peers.

Sorry, it still doesn't look like an answer to my question. Take, for example, a tracker "http://tracker.org ". Before the announcement, you update related endpoints (create/remove announce_endpoint items), and then you mark some of them as skipped. What is the specific reason preventing you from simply not creating announce_endpoints that would otherwise be skipped?

glassez commented 2 years ago

Perhaps it would be possible to also suppress the "start" of announcing to hide both events affecting the announce_entry state

Damn. I'm talking about the fact that removing one alert from the "complete set" is a destructive idea, and in response you suggest deleting another one?

glassez commented 2 years ago

Do you have any more tangible evidence about the cost of filtering alerts that aren't interesting? I'm skeptical that such cost is inherent in the filtering, but might be some problematic design decisions for performing the filtering.

There are no problems with filtering alerts that aren't interesting. @summerqB doesn't seem to have analyzed the problem deeply enough. The real problem is that the existing set of related alerts provides insufficient information, so you have to request the missing information in another way. And the worst part is that the only way is a blocking call to torrent_handle::trackers(). Just the day before, I completed refactoring the qBittorrent code, which allows avoiding this call as much as possible under current conditions (further improvements are possible only after making appropriate changes to libtorrent).

arvidn commented 2 years ago

Damn. I'm talking about the fact that removing one alert from the "complete set" is a destructive idea, and in response you suggest deleting another one?

I think this would be equivalent to not creating the announce_endpoint to begin with. But I think the reason the "skipped" decision is deferred is because the tracker hostname needs to be resolved before you know whether it's reachable or not. e.g. the tracker might resolve to an IP on your local network

ghost commented 2 years ago

@arvidn I just noticed one strange behaviour that was put in place in qBt maybe many version back.

qBt was set to force reannounce all trackers everytime a listen_succeeded_alert was posted.

https://github.com/qbittorrent/qBittorrent/blob/86b6bcc1d9f05d3d05769fc84692f85292a9c00a/src/base/bittorrent/session.cpp#L5189

I think such alerts are only posted on startup and whenever a interface gets refreshed(goes down then comes back?).

Do you think such behaviour is necessary? Does libtorrent have any built in code to speed up tracker recovery after interface is back up?

Also when session is being started, these alerts may be posted from multiple interfaces and they are posted twice for TCP and UTP. So if qBt does so many force reannounces at a time, that would end up in startup delays or other issues?