Opened 6 years ago
Last modified 6 years ago
#5883 new Enhancement
Remove 'paused' event from announce
Reported by: | hexxy | Owned by: | jordan |
---|---|---|---|
Priority: | Normal | Milestone: | None Set |
Component: | libtransmission | Version: | 2.84 |
Severity: | Normal | Keywords: | |
Cc: |
Description
https://trac.transmissionbt.com/browser/trunk/libtransmission/announcer-http.c#L47
This code is logically broken, a hack, and should be removed. I is breaking on trackers that are adhering to the BT specification regarding events.
1) 'paused' event is not part of the BitTorrent? specification (https://wiki.theory.org/BitTorrentSpecification#Tracker_Request_Parameters) . There's no logical reason to announce a 'pause' to the tracker instead of a stop event. If you want a user to removed from the pool since they aren't seeding, announce the 'stopped' event (that's what it is there for). Also, not all trackers supported the paused event.
2) This code is obviously a hack. The logic is now split between here and tr_announce_event_get_string. This code should *NEVER* have been put here. If it was implemented it should have been in tr_announce_event_get_string
3) This code is fundamentally flawed. The logic is that if the torrent is a partial seed and is not stopped it is paused. This is causing partial seeders to send 'paused' events with every announce even when it is running.
Change History (6)
comment:1 follow-up: ↓ 2 Changed 6 years ago by x190
comment:2 in reply to: ↑ 1 ; follow-up: ↓ 3 Changed 6 years ago by hexxy
Replying to x190:
http://www.bittorrent.org/beps/bep_0021.html
Tracker Announce
In order to tell the tracker that a peer is a partial seed, it MUST send an event=paused parameter in every announce while it is a partial seed.
The referenced document is still a 'draft' status and is in no way part of the standardization. Nor should it ever be. The definition of an event is when an action occurs. Sending a 'paused' status on every announce is not an event, it is a status.
Using an existing variable for something other than it's intended and described purpose (aka as a hack) is the lazy developer's way out.
A partial seeder can be determined by the tracker using the following logic when calculating deltas. If download_delta = 0 and upload_delta > 0 and left > 0 then partial seeder.
Also keep in mind the 'paused' event is NOT supported for UDP trackers. Implementing a (badly thought through) draft proposal for only one announce protocol is also inconstant behavior.
comment:3 in reply to: ↑ 2 Changed 6 years ago by cfpp2p
Replying to hexxy:
The referenced document is still a 'draft' status and is in no way part of the standardization.
There are a lot of draft BitTorrent Enhancement Proposals currently and widely in use by many different clients. http://www.bittorrent.org/beps/bep_0000.html
Num Title 5 DHT Protocol 6 Fast Extension 7 IPv6 Tracker Extension 10 Extension Protocol 12 Multitracker Metadata Extension 15 UDP Tracker Protocol 16 Superseeding 17 HTTP Seeding (Hoffman-style) 18 Search Engine Specification 19 HTTP/FTP Seeding (GetRight-style) 21 Extension for Partial Seeds 22 BitTorrent Local Tracker Discovery Protocol 24 Tracker Returns External IP 26 Zeroconf Peer Advertising and Discovery 27 Private Torrents 28 Tracker exchange 29 uTorrent transport protocol 30 Merkle tree torrent extension 31 Tracker Failure Retry Extension 32 IPv6 extension for DHT 33 DHT scrape 34 DNS Tracker Preferences 35 Torrent Signing 36 Torrent RSS feeds 38 Finding Local Data Via Torrent File Hints 39 Updating Torrents Via Feed URL 40 Canonical Peer Priority 41 UDP Tracker Protocol Extensions 42 DHT Security Extension 43 Read-only DHT Nodes 44 Storing arbitrary data in the DHT
Nor should it ever be. The definition of an event is when an action occurs. Sending a 'paused' status on every announce is not an event, it is a status.
What do you mean? As x190 already referenced and again exactly quoted from http://www.bittorrent.org/beps/bep_0021.html "In order to tell the tracker that a peer is a partial seed, it MUST send an event=paused parameter in every announce while it is a partial seed."
Using an existing variable for something other than it's intended and described purpose (aka as a hack) is the lazy developer's way out.
I don't understand. (and why the insult?) Please see r10149 #2888 #1379
A partial seeder can be determined by the tracker using the following logic when calculating deltas. If download_delta = 0 and upload_delta > 0 and left > 0 then partial seeder.
Also keep in mind the 'paused' event is NOT supported for UDP trackers. Implementing a (badly thought through) draft proposal for only one announce protocol is also inconstant behavior.
As far as I know all of the major bittorrents include bep21 Also: http://www.bittorrent.org/beps/bep_0015.html and http://www.bittorrent.org/beps/bep_0041.html
comment:4 follow-up: ↓ 5 Changed 6 years ago by hexxy
Maybe I'm barking up the wrong tree and I should look at having that draft pulled. Here's my reasoning behind that.
Let's start with the field name we are using here, 'event'. 'event' is designed to tell the tracker of a particular event happening (e.g. torrent being started, or completing). Event should only be filled in when there is a status change in the torrent such as it being completed. When a torrent is completed 'event' is set to completed and sent ONCE. When a torrent is started 'event' is set to 'started' and is sent ONCE. When a torrent is stopped 'event' is set to 'stopped' and sent ONCE. See a pattern here yet? The events are only sent once and at the time of the event. 'event' was never meant to be used as a status indicator. What do I mean by status indicator? I mean when the same value is sent over and over to indicate a constant 'status' in this case we are using the value 'paused'.
On that note let's talk about using the value of 'paused'. Where did this value come from? Why was this arbitrary value chosen? Why do I say arbitrary? because it is arbitrary. 'paused' you would think would indicate the user clicked 'paused' or it was 'paused' due to scheduling. Nope. In this case 'paused' means partial seed. Wait, what? Why are we using 'paused' to indicate partial seeding? Why not use a variable called 'partial_seed' and set it to 1? Why not use a more descriptive 'event' value even, like 'partial'. I can tell you why. The developer(s) who came up with that 'draft BEP' were lazy and didn't want to implement a new status the correct way. Instead they chose to re-use an existing field 'event' for something other than it's original intended use. Not only that they got double lazy and didn't even give a forethought to the idea that maybe a more descriptive value should be used instead of 'paused'.
So yes. I have a problem with the fact that a) the field is being used for something other than it's intended purpose and b) the value is being used is a completely arbitrary and non-descript of the ACTUAL reason it's being sent.
Like i said in my last comment there is absolutely no reason for this Draft BEP due to: A partial seeder can be determined by the tracker using the following logic when calculating deltas. If download_delta = 0 and upload_delta > 0 and left > 0 then partial seeder.
comment:5 in reply to: ↑ 4 Changed 6 years ago by x190
Replying to hexxy:
Maybe I'm barking up the wrong tree and I should look at having that draft pulled.
I agree. Good Luck convincing Bittorrent Inc. :-)
comment:6 Changed 6 years ago by CHepx
- Priority changed from Highest to Normal
- Severity changed from Blocker to Normal
- Type changed from Bug to Enhancement
i totally agree with hexxy. the paused event is senseless and useless since, as pointed out by hexxy already, partial seeders can easily be discovered by doing a trivial calculation. just because there's a draft somewhere doesn't mean it has to be followed blindly. drafts are drafts for a reason.
http://www.bittorrent.org/beps/bep_0021.html
"Tracker Announce
In order to tell the tracker that a peer is a partial seed, it MUST send an event=paused parameter in every announce while it is a partial seed."
https://trac.transmissionbt.com/ticket/1379#comment:8
"if the tracker doesn't recognize the `event' value, it's simply ignored, but will still return a set of peers anyway.
For example, opentracker ignores everything except "stopped" and "completed"."