Bug 179456 - KTorrent does not allow you to pause individual torrents
Summary: KTorrent does not allow you to pause individual torrents
Status: RESOLVED FIXED
Alias: None
Product: ktorrent
Classification: Applications
Component: general (other bugs)
Version First Reported In: unspecified
Platform: Compiled Sources Linux
: NOR wishlist
Target Milestone: ---
Assignee: Joris Guisson
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-01-03 08:45 UTC by Adam Forsyth
Modified: 2010-03-07 17:34 UTC (History)
1 user (show)

See Also:
Latest Commit:
Version Fixed In:
Sentry Crash Report:


Attachments
patch to add the pause torrent feature to ktorrent (55.04 KB, patch)
2009-01-03 09:33 UTC, Adam Forsyth
Details
updated pause torrent patch (55.68 KB, patch)
2009-02-06 16:36 UTC, Adam Forsyth
Details
updated patch for svn r933301 (55.27 KB, patch)
2009-02-28 23:17 UTC, Adam Forsyth
Details
Pause Torrent Part 1: Rename Pause KTorrent to Suspend Ktorrent (48.32 KB, patch)
2009-12-18 00:53 UTC, Adam Forsyth
Details
Pause Torrent Part 1: Rename Pause KTorrent to Suspend Ktorrent (42.16 KB, patch)
2009-12-18 01:29 UTC, Adam Forsyth
Details
Pause Torrent Part 2: Add Basic Functionality to libbtcore (11.71 KB, application/octet-stream)
2009-12-19 22:07 UTC, Adam Forsyth
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Adam Forsyth 2009-01-03 08:45:29 UTC
Version:           svn r904136 (using Devel)
OS:                Linux
Installed from:    Compiled sources

In other BitTorrent programs, you can pause individual torrents without stopping the torrent with the tracker or disconnecting from peers.

KTorrent's pause function is really more of a Suspend / Resume function.

Since when a torrent is paused no data is received, it should be possible to move files within a multi-file torrent or the torrent as a whole while the torrent is paused rather than stopped.
Comment 1 Adam Forsyth 2009-01-03 09:33:46 UTC
Created attachment 29855 [details]
patch to add the pause torrent feature to ktorrent

This patch adds basic pause torrent functionality to KTorrent. It also renames the current queue pause function to "Suspend / Resume" which more closely describes what it does and allows for the word "pause" to be used unambiguously.

It's changes to libbtcore are very nearly identical to those I've used without issue since June 2008. I didn't submit it then because it removed the queue suspend functionality completely. I have just now had time to update to the latest KTorrent -- I was using a svn build from October -- and rewrite the patch for the current codebase.

The patch, in addition to stopping network traffic, saves the current chunks to disk so that the files can safely be moved. I will to convert the move file / move data functions to use the pause function rather than stop the torrent in a couple of days when I get a chance.

I did not add any of the actions to the toolbars or context menu in ktorrentui.rc. I use KTorrent at a very low resolution so it's better if someone with a more usual resolution decided where to add more buttons. Personally, I add Pause Torrent / Unpause Torrent and Pause All / Unpause All to a toolbar and Pause / Unpause All in Current View to the context menu so I can easily pause all uploads or downloads.

As it is now, the patch can be committed without it changing anything visible to the user other than the name of the Queue Suspend / Resume functionality. This is really the only feature I ever used in another BitTorrent client that is not in KTorrent 3, now that there is a syndication plugin.
Comment 2 Joris Guisson 2009-01-03 12:16:37 UTC
I will check it out and test it.
Comment 3 Joris Guisson 2009-01-05 14:16:06 UTC
It looks like there is an icon missing (kt-unpause).

Anyway, patch looks promising and is very complete. I have made some small changes here and there :
- Sending a not interested message to each peer when a torrent is paused
- Make sure that moving files of a paused torrent doesn't cause it to be stopped
- Make sure that the stats get updated so that a paused torrent reports correct stats

Still need to sort out the icons and the menus, pause and unpause need to have different icons then suspend and resume of the queue. 
Comment 4 Adam Forsyth 2009-01-06 00:01:02 UTC
Yeah, I didn't really pay much attention to the icons (but where it says kt-unpause I think I just have kt-start), as I just removed the suspend / resume from the menus since I don't use it.

The Move File / Move Data stuff should be trivial to change over to use pause instead of stop, if I remember correctly from when I originally wrote the patch. I just ran out of time and wanted to post the patch over the weekend.

I forgot about the stats thing; good catch. I never addressed that with my original version of the patch either.

I don't think sending "Not Interested" is the right thing to do. Peers will disconnect from people they are not interested in who are also not interested in them. We need to make sure to maintain the peer connections so that we can return to normal speeds as quickly as possible upon unpausing.
Comment 5 Joris Guisson 2009-01-14 10:07:41 UTC
This patch is going to be shifted to 3.3, 3.2 is to close now, and the queue manager needs to take pausing into account, which will lead to significant changes.
Comment 6 Adam Forsyth 2009-01-14 19:54:05 UTC
I think that pausing a torrent should NOT be taken into account by the queue manager. As far as the queue is concerned the torrent is running, and the stalled timer should be reset (since the torrent is purposely stalled). It SHOULD hold it's spot in the queue, as this makes sense conceptually (pausing as opposed to stopping the torrent). If you want another torrent to download then you need to give it a higher priority in the queue.

I'm not concerned what version the feature gets included in, but I don't think it's relevant to the queue manager.

Looking at the code, it doesn't look like I remembered to reset the stalled timer in this version of the patch. Just adding "stalled_timer.update();" to the "if(stats.paused)" section of "torrentcontrol->update()" I believe is the correct (as I intended it) behavior.
Comment 7 Adam Forsyth 2009-02-06 16:36:38 UTC
Created attachment 31044 [details]
updated pause torrent patch

This patch applies cleanly to 3.3dev. It just uses the kt-pause and kt-start icons (as the suspend function does). It pauses the torrent rather than stopping it to move files. While paused, the torrent will never report as stalled. It renames the old pause / resume function to suspend / resume.

It does not alter the queue manager in any way: as far as the queue manager is concerned the torrent is still running. If you want the queue manager to advance another torrent in the queue, the higher ordered torrent should be stopped, not paused.

It does not send a not interested message to peers. Sending a not interested message to peers can cause them to drop their connections, which this patch was specifically meant to avoid.

It does not add the new functionality to any menus / toolbars so it can safely be committed before any decisions in re icons are made.

It does not fix the stats reporting... I've never looked at that so I'll let you add that part :).
Comment 8 Adam Forsyth 2009-02-28 23:17:58 UTC
Created attachment 31706 [details]
updated patch for svn r933301

See above. Still needs stats correction.

Updated to work with changes to UI. It does not change anything visible except to the user except renaming the current Pause function to Suspend and adding Pause/Unpause and Pause All/Unpause All functions to the "Configure Toolbar" window.
Comment 9 Adam Forsyth 2009-12-18 00:53:18 UTC
Created attachment 39133 [details]
Pause Torrent Part 1: Rename Pause KTorrent to Suspend Ktorrent

See my post on the KTorrent Forums for more info:
http://ktorrent.org/forum/viewtopic.php?f=1&t=3371
Comment 10 Adam Forsyth 2009-12-18 01:29:13 UTC
Created attachment 39134 [details]
Pause Torrent Part 1: Rename Pause KTorrent to Suspend Ktorrent

See my post on the KTorrent Forums for more info:
http://ktorrent.org/forum/viewtopic.php?f=1&t=3371

Fix: Accidentally changed the Changelog in the last one.
Comment 11 Joris Guisson 2009-12-18 20:49:27 UTC
Yes, lets get this bug moving again, I have been letting it slip for to long.

The patch looks OK, you have been a bit overzealous with the replacing of paused by suspended (it isn't really necessary in the logviewer) but I can live with that.

What I'm going to add to this patch is changing "Stop all torrents" in the bandwidth scheduler dialogs into "Suspend all running torrents", so it is consistent with the tooltip of the suspend action.

I'm also going to change the suspend icon. If we are going to add pausing of torrents, the current icon should be used for that. Will see if I can find an existing one, otherwise I will have to ask the KDE artists.
Comment 12 Joris Guisson 2009-12-18 21:07:37 UTC
OK, this patch is committed in rev 1063560
Comment 13 Adam Forsyth 2009-12-19 22:07:49 UTC
Created attachment 39178 [details]
Pause Torrent Part 2: Add Basic Functionality to libbtcore
Comment 14 Adam Forsyth 2009-12-19 22:15:13 UTC
Comment on attachment 39178 [details]
Pause Torrent Part 2: Add Basic Functionality to libbtcore 

This is a forward port of the basic libbtcore part of the pause torrent patch. It does not include changing the Move Data / Move File functionality to use pause instead of stop.
Comment 15 Joris Guisson 2009-12-20 16:51:46 UTC
Patch committed in 1064304, however made several modifications:

- When pausing, peers are choked, all requests are canceled and we tell them we are not interested (IMHO the right thing to do)
- Fix a crash due to ChunkDownloadView not being notified of the fact that all ChunkDownload objects are destroyed.
- Added some GUI stuff to test it
Comment 16 Adam Forsyth 2009-12-20 17:51:40 UTC
You've decreased the usefulness of the patch by making those changes to peers and requests when paused. I haven't looked at your changes, but the whole idea is to reduce the overhead of pausing and unpausing a torrent so it can regain speed quickly after unpausing. The biggest issue is that telling peers we're not interested will cause peers who are not interested in us to kill the connection. Having to reconnect to peers is what this patch was written to avoid: you might as well just "suspend" the torrent.
Comment 17 Joris Guisson 2009-12-21 10:51:34 UTC
Is there any evidence that clients do this ?

According to http://wiki.theory.org/BitTorrentSpecification:

"It is important for the client to keep its peers informed as to whether or not it is interested in them. This state information should be kept up-to-date with each peer even when the client is choked. This will allow peers to know if the client will begin downloading when it is unchoked (and vice-versa)."

We should stick to the standard.
Comment 18 Adam Forsyth 2009-12-22 00:04:34 UTC
My reading of that section is different. 

As we could AT ANY MOMENT unpause and request a block (chunk), we should remain interested. The pause function should, as nearly as possible, simply reduce our download speeds to zero as far as peers are concerned. An argument can be made that we should choke the peer, something I have in my notes to look at. This was just a forward port of my old patch's libbtcore section, not meant to be complete.

Just because you are interested does not mean you will ALWAYS request from that peer immediately, it just means you MAY, I think that it is consistent to remain interested.

From the spec:
"A block is downloaded by the client when the client is interested in a peer, and that peer is not choking the client. A block is uploaded by a client when the client is not choking a peer, and that peer is interested in the client."

Again, at any moment, we may request a block.

Also look at PeerManager::killUninterested().
Comment 19 Joris Guisson 2009-12-22 10:05:00 UTC
killUninterested is not called anywhere, it is a leftover of an experiment to get better speeds a long time ago. I should remove it.

And according to http://wiki.theory.org/BitTorrentSpecification:
    * interested: Whether or not the remote peer is interested in something this client has to offer. This is a notification that the remote peer will begin requesting blocks when the client unchokes them. 

So clearly, if we do not intend to ask blocks, we should not tell the peer we are interested.
Comment 20 Adam Forsyth 2009-12-22 18:51:20 UTC
I think your argument is circular: Because we're doing things to take us out of a state where we're an active peer, we should send not interested / choke. However, it's the sending not interested / choking that puts us in an inactive state to begin with. If we simply temporarily pause our data stream, we've not done anything to remove us from an active state, so we don't need to send not interested etc.

I also disagree that it's breaking the spec, the paused function as I wrote it does not stop us from being ready to request chunks, which is what interested means. The exact meaning of the wording of the spec can be argued forever, but this follows the spirit.

The upside of remaining interested is a quicker return to normal function, and decreased chance of peers disconnecting from us. Can you describe to me the downside to the peer of a connection remaining interested while in between requests, as long as at any moment it may be requesting?
Comment 21 Joris Guisson 2009-12-25 13:07:37 UTC
The standard is pretty clear to me, if you are not going to ask for blocks, you are not interested, and the other side should not unchoke you.

> The upside of remaining interested is a quicker return to normal function, and
> decreased chance of peers disconnecting from us. Can you describe to me the
> downside to the peer of a connection remaining interested while in between
> requests, as long as at any moment it may be requesting?

The downside is that the other side could be serving some other peer, which is better for the swarm (higher availability of chunks)
Comment 22 Joris Guisson 2010-03-07 17:34:33 UTC
SVN commit 1100480 by guisson:

Use pause functionality instead of stopping and restarting a torrent when doing jobs 

BUG: 179456

 M  +1 -0      ChangeLog  
 M  +2 -2      libbtcore/torrent/jobqueue.cpp  


WebSVN link: http://websvn.kde.org/?view=rev&revision=1100480