qbittorrent / qBittorrent

qBittorrent BitTorrent client
https://www.qbittorrent.org
Other
27.75k stars 3.92k forks source link

Don't merge trackers for private torrents #2928

Closed build-system closed 8 years ago

build-system commented 9 years ago

EDIT by @sledgehammer999: See final decision here: https://github.com/qbittorrent/qBittorrent/issues/2928#issuecomment-98186429

ORIGINAL POST FOLLOWS

Since version 2.2.0, qBittorrent checks whether the torrent you're adding is already on the list. If it is, then it automatically merges the trackers of both torrents under a single torrent. While this is a useful feature, there are situations where it is undesirable.

Many trackers keep download and upload statistics for every user. Torrents downloaded from these trackers come with only one tracker URL and often have the private flag set. The same content may be available on multiple trackers and the user can seed on all of them — a practice known as cross-seeding. However, when all trackers are using the exact same torrent file, qBittorrent will merge the trackers under a single torrent.

This is undesirable because the client will report upload and download statistics incorrectly. If you upload some gigabytes to a peer from one tracker, this will be announced to all trackers. From the tracker's point of view, the user is cheating ratio by pretending to upload data that wasn't actually uploaded and therefore in grave violation of the rules. This feature can thus result in the loss of an ill-advised user's account. People who are aware of this problem recommend the use of another client in order to work around the issue. It would be nice if they didn't have to resort to that.

A good way to avoid this problem is to simply ask the users whether they want to merge the torrents instead of making that decision for them. Another solution: don't merge torrents with the private flag set. Even an option buried deep under the Advanced section would be nice.

chrishirst commented 9 years ago

1/ The hash IDs need to be different for two two or more torrent to be treated as separate jobs, and if the payloads ARE identical, that is never going to happen.

2/ it is libtorrent that handles the identification and downloading of torrents, so this request should be put to them and implemented before qBitTorrent can even consider implementing something like this.it

sledgehammer999 commented 9 years ago

The only viable option here is to not merge torrent for private trackers(and display an explicit message for that). The infohash is used by qbt to identify the different torrents in the session. And I am not even sure if libtorrent supports the having the same torrent in the session, 2 times with different trackers. You might be interested to make a suggestion in libtorrent-rasterbar's bugtracker, to have an option to somehow post different stats on each tracker for private torrents. (Based on from where it got it's peers) Or something like that. PS: I am editing your title and message to reflect my decision.

build-system commented 9 years ago

Indeed, it appears that this is the behavior of libtorrent:

If the torrent you are trying to add already exists in the session (is either queued for checking, being checked or downloading) add_torrent() will throw libtorrent_exception which derives from std::exception unless duplicate_is_error is set to false. In that case, add_torrent() will return the handle to the existing torrent.

I'm a bit curious, though. There are posts on the Internet which cite Deluge as a client which can properly cross-seed torrents. I have never used it so I'm not sure if that is accurate, but I know it is based on the same libtorrent as qBittorrent. I wonder if it got managed to get around the problem somehow.

This comes to mind:

if uuid is specified, it is used to find duplicates. If another torrent is already running with the same UUID as the one being added, it will be considered a duplicate. This is mainly useful for RSS feed items which has UUIDs specified.

Maybe this uuid can be modified before it is passed to libtorrent.

@chrishirst, the torrents are exactly the same except for the trackers. I don't know why the private torrent's trackers aren't used as input for the hash function. This behavior is fine for public trackers, but they are critical in the case of private torrents.

DoumanAsh commented 9 years ago

Ummm... to me it seems to be a bit crappy... Why would you need that? I understand that private trackers are shitty, but still...

Regardless, i think we can have dialogue for merging, no need for duplicate torrents. And current behaviour of libtorrent is right. After all you're downloading exactly the same files... Isn't it strange to download/seed the same files separately?

@sledgehammer999 I would set title like "Allow user decision to merge trackers for private torrents" Doubt that everyone would wish to just restrict it :)

chrishirst commented 9 years ago

If anyone does need to peer the same payload to two separate 'private' trackers why not simply run two clients seeding from the same files? Obviously you cannot download in the same way, but a good way to improve the enforced ratio on private trackers is to seed without downloading anything..

build-system commented 9 years ago

@DoumanAsh, why this is needed is explained in this issue. It is hinted at in the private torrents bittorrent protocol extension I linked:

To promote sharing, private trackers often maintain statistics about registered users and restrict access to certain or all torrents for users that do not adequately upload.

Not just private trackers, either. Any trackers which maintain user accounts. Even open signup ones.

The fact is merging the trackers of identical private torrents is going to cause the misreporting of these statistics. From the point of view of the tracker, this is indistinguishable from a ratio cheating program that is made to send fake statistics to it.

Pretty much all such trackers contain the warning in their rules: your torrent must have only this tracker's URL in it. If the torrent files themselves are otherwise identical, qBittorrent will not allow such a situation to occur and therefore cross-seeding of files is hampered. If you have two private torrents that are tracked by two different trackers, you must pick the one you want to seed — you cannot seed both in qBittorrent.

@chrishirst, I'm sure qBittorrent users like myself would rather simply run qBittorrent instead of having to install and run a second client and maintain a separate set of torrents. The waste of system resources and added cognitive load are not welcome.

Yes, it is a great way to improve ratio. It would be nice if we could take advantage of it by using qBittorrent.

DoumanAsh commented 9 years ago

Well... i was just checking issues and when i saw "Don't merge trackers for private torrents" i was a bit surprised. Anyway option would be fine, after all the speed is more important than some crappy statictic, but in my opinion: screw these private trackers :)

chrishirst commented 9 years ago

I'm sure qBittorrent users like myself would rather simply run qBittorrent instead of having to install and run a second client and maintain a separate set of torrents. The waste of system resources and added cognitive load are not welcome.

Of course, but as it is unlikely to be possible within the constraints of the various protocol engines and/or clients that use the hash ID to communicate and transfer pieces/blocks with other peers, trackers, DHT , PeX, etc. there are few or no choices available.

chrishirst commented 9 years ago

Just a thought while making a brew, ...

One way would be to launch an independent instance of a libtorrent session for the second private tracker.

build-system commented 9 years ago

I have read the source code. Indeed, there are duplicate checks prior to adding torrents to the session; if a duplicate is discovered, the trackers and URL seeds are merged using the QBtSession::mergeTorrents method. At first I thought this could be modified to take private torrents into account but the session::add_torrent function will also check for duplicates and return the existing handle should it find one.

The way to work around that is via add_torrent_params::uuid. When we find a duplicate torrent, we can ask the users whether they want to merge the torrents using a dialog. If they answer yes, we can simply modify the uuid and make the torrent unique again. I think this is the simplest solution for the time being and it should work whether libtorrent is patched or not.

With that said, how do I get in touch with libtorrent developers? I see they have a bug tracker but this is not exactly a bug. I'd just like to ask them what they think of this issue.

DoumanAsh commented 9 years ago

Either via mail-list or bug report on http://www.libtorrent.org/

build-system commented 9 years ago

I posted a message to their mailing list requesting feedback on the solution I offered as well as asking if they could make any changes on their end.

DoumanAsh commented 9 years ago

The way to work around that is via add_torrent_params::uuid. When we find a duplicate torrent, we can ask the users whether they want to merge the torrents using a dialog. If they answer yes, we can simply modify the uuid and make the torrent unique again. I think this is the simplest solution for the time being and it should work whether libtorrent is patched or not.

I would also suggest option to set this behaviour in configuration(i.e. always separate torrent handlers for private or all, or not) Anyway i'm not really familiar with C++ code so i will leave this thinking to someone more experienced with qBt code

build-system commented 9 years ago

I have been browsing the libtorrent repository and have tracked down the code which checks for duplicates within a session before adding torrents. It is in src/session_impl.cpp, line 4630:

// is the torrent already active?
boost::shared_ptr<torrent> torrent_ptr = find_torrent(*ih).lock();
if (!torrent_ptr && !params.uuid.empty()) torrent_ptr = find_torrent(params.uuid).lock();

// if we still can't find the torrent, look for it by url
if (!torrent_ptr && !params.url.empty()) {
  torrent_map::iterator i =
    std::find_if(m_torrents.begin(), m_torrents.end(),
                 boost::bind(&torrent::url,
                              boost::bind(&std::pair<const sha1_hash,
                                           boost::shared_ptr<torrent> >::second,
                                           _1)) == params.url);
  if (i != m_torrents.end()) torrent_ptr = i->second;
}

if (torrent_ptr) {
  if ((params.flags & add_torrent_params::flag_duplicate_is_error) == 0) {
    if (!params.uuid.empty() && torrent_ptr->uuid().empty())
      torrent_ptr->set_uuid(params.uuid);
    if (!params.url.empty() && torrent_ptr->url().empty())
      torrent_ptr->set_url(params.url);
    if (!params.source_feed_url.empty() && torrent_ptr->source_feed_url().empty())
      torrent_ptr->set_source_feed_url(params.source_feed_url);
    return torrent_handle(torrent_ptr);
  }

  ec = errors::duplicate_torrent;
  return torrent_handle();
}

So, it first tries to look for the torrent by the given uuid. If it can't be found, it tries to look for it by url.

Later on, near the end of the same function we have:

if (!params.uuid.empty() || !params.url.empty())
  m_uuids.insert(std::make_pair(params.uuid.empty() ? params.url : params.uuid,
                                torrent_ptr));

m_uuids is declared as:

std::map<std::string, boost::shared_ptr<torrent>> m_uuids;

We can see that if the function is given either a non-empty uuid or a non-empty url, these are used to identify the torrent. In case both are passed, the uuid is favored.

At the beginning of the function, there are checks looking for magnet: and file:// URLs, suggesting it refers to the URL that was used by the client to load the torrent and therefore a parameter that is often, if not always, passed.

What all this means to me is that uuid and url are practically synonymous. The uuid of the torrent is its url, unless an uuid was explicitly passed by the client.

With this knowledge, I can think of a very simple way to implement this:

  1. qBittorrent already checks if the torrent is already in the session
  2. If it is...
    1. We ask users whether they want to merge
    2. If they say yes...
      1. Merge trackers
    3. If they say no...
      1. Construct a new string that uniquely identifies the torrent
      2. Set uuid to the string
      3. Add the torrent

Point 2 > iii > a is all that remains for consideration. One could use the url parameter, followed by a space and an index number. Examples of uuids as they would end up being stored by libtorrent:

file:///C:/example.torrent
file:///C:/example.torrent 2
file:///C:/example.torrent 3
file:///C:/example.torrent 4

The first one was passed to libtorrent as an url, the other ones were passed as uuids because they triggered the duplicate check and the user decided to add duplicate torrents.

sledgehammer999 commented 9 years ago

I think arvidn's response made it very clear that currently libtorrent cannot differentiate between torrents with the same hash but different trackers. For anyone caring here is the mailing list thread: http://sourceforge.net/p/libtorrent/mailman/message/34078917/

build-system commented 9 years ago

Indeed. After arvidn's explanation, I wonder if there's any client out there that can do it.

My suggested changes above were all based on an incomplete understanding of the problem and therefore incorrect. I'm closing this issue as there's nothing that can be done.

DoumanAsh commented 9 years ago

No no, i think we should allow user's choice for whatever merge private torrent or not. At least that will leave some freedom to user to decide how he wish do deal with such issue

sledgehammer999 commented 9 years ago

Yes, the original problem can't be solved. But the problem described in the (edited) title can be remedied.

build-system commented 9 years ago

By not merging trackers, you mean doing nothing?

Yeah, I guess that would be a clear improvement. Currently, qBittorrent merges the trackers and the user must immediately delete the tracker from the torrent's tracker list in order to avoid consequences. Not merging trackers for private torrents would make life easier.

lamskoy commented 8 years ago

Would be awesome feature to see in qBittorrent! Have to use uTorrent instead of qBittorrent coz last one has no such feature.

lamskoy commented 8 years ago

Nice feature. But there's potential issue - torrent can belong to private tracker even if torrent is not marked as private.

Possible to change logic in this way?

  1. if it's private torrent - display message that it was not possible to merge trackers.
  2. if it's not private one - ask user "Do you want to merge trackers?" Yes / No If yes - merge trackers, no - don't to anything
sledgehammer999 commented 8 years ago

Open new issue.

lamskoy commented 8 years ago

Oh I created pull request already for this issue

lamskoy commented 8 years ago

Added https://github.com/qbittorrent/qBittorrent/pull/4801

dwarfdurin commented 6 years ago

Still forcing to not merge trackers even if the torrent is private isn't the right way, I'd like to see settings option to change it to allow merging trackers for any torrent, no matter if its flagged private or not(private+private, non-private+private). uTorrent has that and statistics are working well, still it would be better to leave it to user's decission about that behaviour. Default action can be set as it says in that issue closing words that qBittorrent won't merge trackers for private flagged torrents, but a way to change it for user's desire is something many of us would welcome.