Opened 9 years ago

Closed 5 years ago

Last modified 2 years ago

#671 closed Enhancement (fixed)

torrent queuing

Reported by: jorge Owned by: jordan
Priority: Normal Milestone: 2.40
Component: Transmission Version: 1.03
Severity: Normal Keywords: patch feature-disparity
Cc: 1100101@…, mickael.hoareau+transmission@…, grzegorz.dubicki@…, tom@…, hgabreu@…, ryuji@…, Obi_Wan_84@…, monkeyman23555@…, Shu, giovannibajo@…, atommixz@…, altrov3@…, leho@…, federicoleva@…

Description

I would like Transmission could queue multiple torrents and have only one (or more) active at a time.

I have wrote a (very preliminar) patch that allows queueing and setting the seed ratio in Gtk+ client. Actually it is not enough tested (or not tested at all), it pretends to be an example rather than "real" code.

Attachments (20)

transmission-queue.diff.gz (2.9 KB) - added by jorge 9 years ago.
Queue and seed cap ratio
add-ratio-limiting.diff (8.2 KB) - added by swtaarrs 8 years ago.
Updated patch
ratioLimitRPC.patch (3.0 KB) - added by journey4712 8 years ago.
rpc calls to go with ratio limit patch
ratioLimitRemote.patch (3.7 KB) - added by journey4712 8 years ago.
Update transmission-remote to report and modify the ratio limit
patch.addActiveTorrentQueue (11.8 KB) - added by journey4712 8 years ago.
replace with unified diff
patch.ratioLimitAndQueue (29.7 KB) - added by journey4712 8 years ago.
all previous patches rolled into one
r6960_queue.patch (16.6 KB) - added by dzimi 8 years ago.
Applyable patch for r6960 - only queue
r7107_queue.patch (16.0 KB) - added by journey4712 8 years ago.
updated queue patch for 7109 - fixes pause all/resume all issue
r7107_ratioLimit.patch (12.9 KB) - added by journey4712 8 years ago.
ratio limit and related RPC/remote patches updated for r7107
r8650_queue.patch (18.3 KB) - added by Elbandi 7 years ago.
updated queue patch for 8650 - add remote things
r8650_webfilter.patch (2.9 KB) - added by Elbandi 7 years ago.
queue filter
queue_v2-upstream.patch (21.2 KB) - added by Elbandi 7 years ago.
for branch 1.75
queue_r9887.patch (22.1 KB) - added by Elbandi 7 years ago.
for branch 1.80 beta 4
queue_fix.patch (752 bytes) - added by KyleK 7 years ago.
queue.patch (60.5 KB) - added by Longinus00 6 years ago.
queue implementation with a real queue
queue.2.patch (113.8 KB) - added by Longinus00 6 years ago.
queue.4.patch (107.4 KB) - added by charles 6 years ago.
queue.3.patch + typo fix in daemon + more gtk prefs work
queue.3.patch (128.5 KB) - added by charles 6 years ago.
exactly the same content, but resynced to r11380.
utorrent-queue.jpg (38.6 KB) - added by charles 6 years ago.
uTorrent's preferences dialog's queue tab
queueUi.patch (6.0 KB) - added by deltasquare4 5 years ago.
WebUI patch to r12627 implementing queue modification

Download all attachments as: .zip

Change History (188)

Changed 9 years ago by jorge

Queue and seed cap ratio

comment:1 Changed 9 years ago by charles

  • Milestone changed from None Set to Sometime
  • Status changed from new to assigned

comment:2 Changed 8 years ago by sell

(Spam deleted)

Last edited 6 years ago by charles (previous) (diff)

comment:3 Changed 8 years ago by charles

swtaarrs: this is a good start, but if we've got hooks for limiting in libtransmission, we ought to do the ratio-limit-exceeded/stop there rather than in the gtk code.

comment:4 Changed 8 years ago by swtaarrs

Alright, that was my original plan but putting the code in the gtk part seemed like a good way to get an initial version going. I can work on the proper version later tonight, do you have any suggestions on where in libt would be a good place to do the checking? I don't have the code with me right now but I think I remember that whichever function called peerGotBytes (I think that's the name) looked like a good candidate.

Changed 8 years ago by swtaarrs

Updated patch

comment:5 Changed 8 years ago by swtaarrs

The updated patch I just added does the checking with a tr_timer in the core as Charles suggested, instead of in the ui. So, the only code this adds to the ui is code for setting the ratio limit and whether it's active or not. If there's a better place than torrent.[ch] to to put the queuePulse stuff I can move it, or give them better names.

comment:6 Changed 8 years ago by perham

is the queueing function going to be included into next version? even if it's in svn, I'm ok with that.

Changed 8 years ago by journey4712

rpc calls to go with ratio limit patch

comment:7 Changed 8 years ago by journey4712

Attached patch that updates the rpc to have calls to set/get the ratio limit. The rest of the patch still applies cleanly and works with 6698

Changed 8 years ago by journey4712

Update transmission-remote to report and modify the ratio limit

comment:8 Changed 8 years ago by journey4712

  • Component changed from GTK+ Client to libtransmission

patch.addActiveTorrentQueue is intented to be patched after add-ratio-limiting.diff It implements 2 active torrent queues for seeds and downloads. Max can be set through rpc interface, and is saved in settings.json

Changed 8 years ago by journey4712

replace with unified diff

Changed 8 years ago by journey4712

all previous patches rolled into one

comment:9 Changed 8 years ago by dzimi

This patch don't work on rev 6734. Works only to time, when you hit pause in webUI. Then, If you reasume torrents (disable pause) queue don't work and allow transmission-daemon to seed all torrents.

comment:10 Changed 8 years ago by charles

  • Keywords patch added

Changed 8 years ago by dzimi

Applyable patch for r6960 - only queue

comment:11 Changed 8 years ago by dzimi

this patch - r6960_queue.patch - requires attention. It has been adapted to r6960. I have no idea why the queue stops working when you hit pause and reasume from WebUI.

Changed 8 years ago by journey4712

updated queue patch for 7109 - fixes pause all/resume all issue

Changed 8 years ago by journey4712

ratio limit and related RPC/remote patches updated for r7107

comment:12 Changed 8 years ago by Erektium

this would be a cool feature

comment:13 Changed 8 years ago by Erektium

when will queueing be implemented

comment:14 Changed 8 years ago by charles

  • Summary changed from Queueing and setting the seed ratio to Add support for torrent queuing in libtransmission

Revising this ticket -- seed ratio is now in its own ticket, #1787

comment:15 Changed 7 years ago by charles

@journey4712: I'd like to give this ticket another look. Could you sync the patch with svn trunk?

comment:16 Changed 7 years ago by livings124

#1790 refers to adding this to the web ui.

comment:17 Changed 7 years ago by charles

ticket #2210 has been closed as a duplicate of this ticket.

Changed 7 years ago by Elbandi

updated queue patch for 8650 - add remote things

comment:18 Changed 7 years ago by charles

http://brainstorm.ubuntu.com/idea/19800/ has 44 votes for this feature

Changed 7 years ago by Elbandi

queue filter

comment:19 Changed 7 years ago by Elbandi

one thing is missing: priorising the torrents

comment:20 Changed 7 years ago by charles

  • Summary changed from Add support for torrent queuing in libtransmission to torrent queuing in libtransmission

comment:21 Changed 7 years ago by Snake98

Is it possible to get this patch applied to the nightly build for testing?

comment:22 Changed 7 years ago by Elbandi

the original patch, and mine has 2 bug:

  • if running torrent is removed, doesnt decrement the ActiveCount?.
  • in "activeSeedCount == maxSeedActive" situation, if a downloading torrent is going to complete => activeSeedCount > maxSeedActive (i try this to handle, but i always get a crashed T)

So i rewrite this: drop the activeSeedCount/activeDownloadCount, and stop the torrent in bandwidthPulse. (like seedratio)

Changed 7 years ago by Elbandi

for branch 1.75

comment:23 Changed 7 years ago by KyleK

  • Cc 1100101@… added

comment:24 Changed 7 years ago by motumboe

  • Cc motumboe@… added

comment:25 Changed 7 years ago by charles

  • Summary changed from torrent queuing in libtransmission to torrent queuing

Changed 7 years ago by Elbandi

for branch 1.80 beta 4

comment:26 Changed 7 years ago by mhoareau

  • Cc mickael.hoareau+transmission@… added

Patch posted by Elbandi works fine.

I don't have time for parsing and understanding the C code (and I haven't read C code for years) but if someone can post a patch in order to return a correct status for a queued torrent, I will post a patch for the web interface (I'm a webdev and I am very familiar with jquery)

comment:27 Changed 7 years ago by charles

Elbandi: thanks for the patch! I'll look at this immediately post-1.80

mhoareau: sounds great! Thanks for the offer.

As an aside, adding patches doesn't seem to update a ticket's "last changed" date the way that adding a comment does, so this slipped past my radar until mhoareau's comment... Elbandi, if you have any other new patches, you might want to add a comment to them :)

comment:28 Changed 7 years ago by KyleK

mhoareau: The web interface should actually already get the correct status for a queued torrent.

Elbandi: You added the necessary JS code for a "Queued" filter to the .js files, but missed a single line in the .html:

         <li><a href="#queued" id="filter_queued_link">Queued</a></li>

With that the filter actually shows up on the filter bar :)

comment:29 Changed 7 years ago by Elbandi

Hmm, that line really missing from my patch, dunno why, because r8650_webfilter.patch got that.

Otherwise, this is not a realy FIFO queue, just a download/seed limit. doesnt save the queue state to resume file, and onQueueTimer is choose the first torrent with queue state.

charles: whould you like to see a real queue, or this is good?

comment:30 Changed 7 years ago by KyleK

I discovered a logical problem with the method onQueueTimer(). Say I have maxSeeds set to 3 and maxDownloads to 5. I have 4 torrents loaded into Transmission, 3 are seeding, 1 is downloading. Now I add a 5th torrent. The function onQueueTimer() iterates of the list of torrents, BUT: if the first torrent in that list is seeding, then it will never get any further, it'll just reset the timer. Hence, the download limit can never be reached.

The attached patch fixes the problem for me.

Changed 7 years ago by KyleK

comment:31 Changed 7 years ago by charles

  • Milestone changed from Sometime to 1.90

very tentatively milestoning this for 1.90..

comment:32 Changed 7 years ago by KyleK

The provided code already works pretty well. What's still missing is priorization, which I would simply implement as a top-to-bottom list. That means of course that the user has to be able to move torrents up and down that list.

Also, the check if there are queued torrents is currently timer-based. Maybe this can be rewritten so Transmission only testes for queued torrents if the state of another torrent changed.

I'll just add this here as some kind of ToDo? list. Maybe I'll tackle one problem or other, if I can spare the time.

comment:33 Changed 7 years ago by Longinus00

I only very briefly skimmed the C code but it seems like there's a number for max downloading and max seeding torrents. Doesn't it make more sense to have a max number of downloading torrents and a max number or torrents total?

comment:34 Changed 7 years ago by grzegorzdubicki

  • Cc grzegorz.dubicki@… added

comment:35 Changed 7 years ago by wereHamster

Ugh, iterating over all torrents each second... Don't you think it's a bit overkill? libT already exposes callbacks which are invoked when torrents have finished downloading. Can't you use those?

comment:36 Changed 7 years ago by wereHamster

  • Cc tom@… added

comment:37 Changed 7 years ago by livings124

  • Milestone changed from 1.90 to Sometime

comment:38 Changed 7 years ago by eXpressionist

Have any solutions for daemon?

comment:39 Changed 7 years ago by praveenmarkandu

  • Cc praveenmarkandu@… added

comment:40 Changed 7 years ago by motumboe

  • Cc motumboe@… removed

comment:41 Changed 6 years ago by dekatrion

  • Cc kbmurata@… added

comment:42 Changed 6 years ago by hgabreu

  • Cc hgabreu@… added

Changed 6 years ago by Longinus00

queue implementation with a real queue

comment:43 Changed 6 years ago by charles

  • Keywords feature-disparity added

comment:44 Changed 6 years ago by KyleK

Wow, Longinus00, that patch looks awesome! Unfortunately I'm out of town over the weekend but I'm gonna test this patch as soon as I'm back home.

Thanks for the hard work!

comment:45 Changed 6 years ago by charles

  • Milestone changed from Sometime to 2.00

This is a nice big-ticket item for 2.00. The fact that Someone Else is writing/testing/debugging it is even more appealing.

Thanks for digging into this ticket Longinus00

comment:46 Changed 6 years ago by Longinus00

Firstly, I'd like to say that the starting point for this patch was Elbandi's patch but I've since modified it quite drastically, only a few lines remain recognizable.

The patch is still a work in progress and currently more of a development preview than anything else. I'll try and update it semi regularly but the newest code will be on my github transmission page. Not all features are finished and consider all current UI changes as tentative.

You currently need to use transmission-remote/manually edit settings.json to set all the options but most everything queue related can be handled in the gtk client. Come find me on irc if you have any questions or suggestions.

comment:47 Changed 6 years ago by krx

  • Cc krx@… added

comment:48 Changed 6 years ago by dekatrion

  • Cc kbmurata@… removed

comment:49 Changed 6 years ago by user00265

  • Cc ryuji@… added

comment:50 Changed 6 years ago by garrych

  • Cc garrych@… added

comment:51 Changed 6 years ago by charles

  • Milestone changed from 2.00 to Sometime

Bumping for 2.00. This will probably be in 2.10

comment:52 Changed 6 years ago by ObiWan

  • Cc Obi_Wan_84@… added

comment:53 Changed 6 years ago by ObiWan

The patch is looking good.

comment:54 Changed 6 years ago by monkeyman23555

  • Cc monkeyman23555@… added

comment:55 Changed 6 years ago by jw5801

  • Cc jw5801@… added

Changed 6 years ago by Longinus00

comment:56 Changed 6 years ago by Longinus00

  • Milestone changed from Sometime to 2.20
  • Owner changed from charles to Longinus00
  • Status changed from assigned to new

comment:57 Changed 6 years ago by livings124

To go over how the mac version works with queues:

http://img709.imageshack.us/img709/2568/screenshot20101003at329.png

There are two separate, completely-independent queues. The download queue counts non-complete transfers, the seeding queue counts complete transfers. Each queue works by counting the active number of transfers that are not marked as error or pass the above specified idle time. On each pulse, if that number is equal or greater than the set limit, nothing happens; otherwise, the next flagged waiting torrent(s) is started. Once transfers are started, the queue will never pause them.

Since the queue code is currently just in the Mac code, there is no "queued" state, like there is paused, active, etc. Rather a paused (and technically active as well, but that would have no meaning) can be flagged with the boolean for waiting to start. It will be considered by the app as a paused transfer, much like any other paused transfer, but in the interface the progress bar will be a grayed-blue (or green for the seeding queue) instead of the standard gray. Those paused, flagged transfers are those considered to be started by the queue.

The gui treats a flagged, paused transfer as somewhat active. That means that in the gui it can be paused, which removes the "waiting" flag. Hitting resume all sets the waiting flag for all transfers, and then sets as many transfers the queue will allow as truly active. There is an option to "resume transfer without waiting", which sets the transfer as active regardless if there's room in the queue. That transfers is counted towards the queue limit - if there are more active transfers than the queue specifies, nothing is stopped regardless; instead it will just take more transfers to be stopped until more queue transfers are started. This keeps the number of queue control options small, as well as the queue behavior less confusing (no tracking of which transfers are part of the queue, etc. is necessary).

I believe that covers the Mac queue behavior. If more clarification is needed, please let me know.

comment:58 Changed 6 years ago by Longinus00

What happens if the download queue is set to a number greater than the seeding queue? Say max downlaods is 10 and max seeds is 2. What happens when those torrents finish downloading?

comment:59 Changed 6 years ago by livings124

Both queues are company independent. If a transfer is a download but becomes a seed (or vice versa) and the seed queue is full, it will be caused and set to "waiting to seed."

comment:60 Changed 6 years ago by Longinus00

But you mentioned that the queue will never pause transfers?

comment:61 Changed 6 years ago by livings124

This is inbetween queues, I suppose. This is the only case where that will happen iirc.

comment:62 Changed 6 years ago by Longinus00

What happens to 'force started' torrents, do they lose their 'force start' status?

comment:63 Changed 6 years ago by livings124

There is no "force start" status. If you force start a transfer, it starts. It doesn't set a flag or anything, so I suppose it would stop when it hits the seeding queue.

comment:64 Changed 6 years ago by charles

livings124, Longinus00, what's the next step for this ticket?

comment:65 Changed 6 years ago by phoudiap

good idea!

comment:66 Changed 6 years ago by KyleK

I just played around with this feature after applying patch v3 to the current trunk.

First, I issued these commands:

transmission-remote --s -qd -qnb
transmission-remote -md 2
transmission-remote -t 8,9 -qmt

While the rank of torrents 8 & 9 changed to 0 & 1 (side note: this should probably not be zero-based?), they were still queued, and two other torrents where downloading.

I then issued another command:

transmission-daemon -t 8,9 -qmu

This time, everything worked as expected: the 2 torrents I moved to the top earlier now started downloading, the others were stopped/queued.

comment:67 follow-up: Changed 6 years ago by Longinus00

The queue only starts/stops torrents every minute, starting/stopping torrents every time the users changes the ordering of the queue is not a good idea.

I'm pretty sure most of the other lists in transmission are zero based (trackers, filelist) so I don't see why the queue shouldn't be.

comment:68 in reply to: ↑ 67 ; follow-up: Changed 6 years ago by KyleK

Replying to Longinus00:

The queue only starts/stops torrents every minute, starting/stopping torrents every time the users changes the ordering of the queue is not a good idea.

Why would it not be a good idea? I figure users will change queues not that often. (And to play devil's advocate: uTorrent changes the status of torrents instantly when the user moves them inside the queue).

I'm pretty sure most of the other lists in transmission are zero based (trackers, filelist) so I don't see why the queue shouldn't be.

Torrents and trackers have 1-based IDs, which are used to reference them when using transmission-remote. Even if the rank is never used in a command, it is visible to the user and should be 1-based as well.

comment:69 in reply to: ↑ 68 Changed 6 years ago by Longinus00

Replying to KyleK:

Why would it not be a good idea? I figure users will change queues not that often. (And to play devil's advocate: uTorrent changes the status of torrents instantly when the user moves them inside the queue).

Because torrents changing a torrent from rank 20 to rank 10 would require 10 reevaluations of the queue using the up/down commands.

Torrents and trackers have 1-based IDs, which are used to reference them when using transmission-remote. Even if the rank is never used in a command, it is visible to the user and should be 1-based as well.

Trackers do not have 1 based IDs. The rank won't be visible to the user in the newer versions of the patch, that was just for testing.

comment:70 Changed 6 years ago by livings124

Longinus00: Assuming the queue doesn't stop transfers once started, why should any of this matter?

comment:71 Changed 6 years ago by Longinus00

I've worked around the issue by making operations set timeouts which should prevent rpc calls from spamming queue processing calls. I'll upload that code in a few days once I've played around with it some more.

I'm still fully against a queue that doesn't stop transfers, there's too many situations where that can cause issues. If you really don't want the mac version to stop transfers then I can add in an #ifndef SYS_DARWIN or two. You can also mimic the mac client cutoff behavior by setting the cutoffspeed at 0KBps.

comment:72 Changed 6 years ago by livings124

Just make sure the behavior is written out somewhere and straight-forward.

comment:73 Changed 6 years ago by Longinus00

I'm not sure what you mean? I find it more confusing when the queue options says "x active downloads" yet the client has > x active downloads.

comment:74 follow-up: Changed 6 years ago by livings124

You don't know what I mean when I say to write out how it will behave and make sure it behaves in a straight-forward manner?

I believe having it have to cycle through transfers is very, very confusing, especially if I have 3 torrents that are all stalled that are going to be continually looped through.

Will your queue allow you to force-start transfers when the queue is full? Will the queue need to keep track of which were forced to start, and have to distinguish that in the code and interface? Sure, the Mac queue can allow more than the limit be active if some are stalled or forced to start, but the behavior is still straight forward and simple. To support auto-stopping of transfers the logic is going to get very confusing.

comment:75 in reply to: ↑ 74 ; follow-up: Changed 6 years ago by Longinus00

Replying to livings124:

You don't know what I mean when I say to write out how it will behave and make sure it behaves in a straight-forward manner?

I believe having it have to cycle through transfers is very, very confusing, especially if I have 3 torrents that are all stalled that are going to be continually looped through.

I have no idea what you mean by this.

Will your queue allow you to force-start transfers when the queue is full?

Yes

Will the queue need to keep track of which were forced to start, and have to distinguish that in the code and interface?

Yes, yes and no.

Sure, the Mac queue can allow more than the limit be active if some are stalled or forced to start, but the behavior is still straight forward and simple.

The user still sees "maximum of x active downloads". Giving the user what he expects seems the most straightforward. Maybe the wording should be 'minimum of x active downloads'?

To support auto-stopping of transfers the logic is going to get very confusing.

Not at all. You're talking as if the queue code hasn't even been written yet. Like I said, making it not stop transfers is only going to require two #ifndef's for a total of 4 commented out lines of code. More cpu time will be spent figuring out if a torrent can be skipped or not than deciding to stop a torrent.

comment:76 in reply to: ↑ 75 ; follow-up: Changed 6 years ago by livings124

Replying to Longinus00:

Replying to livings124:

You don't know what I mean when I say to write out how it will behave and make sure it behaves in a straight-forward manner?

I believe having it have to cycle through transfers is very, very confusing, especially if I have 3 torrents that are all stalled that are going to be continually looped through.

I have no idea what you mean by this.

If I have three queued transfers all with no peers, will the client keep on pausing and resuming them in a circle? The Mac implementation would (eventually) have all three active so that one could eventually become active.

Will your queue allow you to force-start transfers when the queue is full?

Yes

Will the queue need to keep track of which were forced to start, and have to distinguish that in the code and interface?

Yes, yes and no.

If it's not indicated in the UI, won't the user wonder why there are too many transfers running? What if they want it to then be part of the queue count? The Mac client avoids this by not auto pausing and counting all active transfers as part of the count.

Sure, the Mac queue can allow more than the limit be active if some are stalled or forced to start, but the behavior is still straight forward and simple.

The user still sees "maximum of x active downloads". Giving the user what he expects seems the most straightforward. Maybe the wording should be 'minimum of x active downloads'?

How is it straightforward if your queue allows transfers outside of the queue that are never part of the queue, especially if it's not indicated in the UI. I think it's very confusing if the client is pausing and resuming transfers all over the place, all the time. I can think of a bunch of cases that are confusing. For example, say I have transfers 1 2 and 3. 1 is active, the others paused. 1 is auto-paused, and 2 is started. I then add transfer 4 to the queue, and move it's order to before 1. If 2 is auto-paused, does 4 start, or 3? What happens if then the next-active one is paused? How does it determine the order? The Mac client avoids all of this by just always starting the first queued transfer in the list - since it doesn't auto-pause, you don't have this confusing mess.

To support auto-stopping of transfers the logic is going to get very confusing.

Not at all. You're talking as if the queue code hasn't even been written yet. Like I said, making it not stop transfers is only going to require two #ifndef's for a total of 4 commented out lines of code. More cpu time will be spent figuring out if a torrent can be skipped or not than deciding to stop a torrent.

The Mac client will adopt whatever the new code is. I don't know why you're talking about CPU - although I am concerned that your implementation requires a delay/interval between actions. Why did you ask for a description and questions of the Mac implementation if you had no intent of following the core design with no discussion to flesh things out.

comment:77 in reply to: ↑ 76 ; follow-up: Changed 6 years ago by Longinus00

Replying to livings124:

If I have three queued transfers all with no peers, will the client keep on pausing and resuming them in a circle? The Mac implementation would (eventually) have all three active so that one could eventually become active.

We've had this exact same discussion on irc before and I already explained to you before that stalled torrents don't take up a slot in the queue.

If it's not indicated in the UI, won't the user wonder why there are too many transfers running? What if they want it to then be part of the queue count? The Mac client avoids this by not auto pausing and counting all active transfers as part of the count.

It's not required that the ui say anything, but it can certainly say "forced downloading" or something instead of "downloading".

How is it straightforward if your queue allows transfers outside of the queue that are never part of the queue, especially if it's not indicated in the UI. I think it's very confusing if the client is pausing and resuming transfers all over the place, all the time. I can think of a bunch of cases that are confusing. For example, say I have transfers 1 2 and 3. 1 is active, the others paused. 1 is auto-paused, and 2 is started.

Why would 1 be paused?

I then add transfer 4 to the queue, and move it's order to before 1. If 2 is auto-paused, does 4 start, or 3? What happens if then the next-active one is paused? How does it determine the order? The Mac client avoids all of this by just always starting the first queued transfer in the list - since it doesn't auto-pause, you don't have this confusing mess.

I can't answer the rest of this until you clarify why 1 would be paused.

The Mac client will adopt whatever the new code is. I don't know why you're talking about CPU - although I am concerned that your implementation requires a delay/interval between actions.

There is no requirement for doing anything, I choose to add a timeout for two reasons:

  1. stopping/starting/etc. x torrents requires x calls to torrent_stop/start/etc. Instead of running through the queue x times in quick succession it's much more efficient to just do it once.
  2. The queue looks at the transfer speed of a torrent torrent to help if figure out if it should be skipped or not and transfer speeds are meaningless over very short intervals.

Why did you ask for a description and questions of the Mac implementation if you had no intent of following the core design with no discussion to flesh things out.

I've followed basically every other part of the mac implementation so there's no need for hyperbole. We've had discussions about stopping torrents before yet you seem not to have remembered any of them. You also have obviously not taken the time to glance over the code I've posted or you wouldn't be asking these same questions. If you don't like what I've written up you can just tell me not to bother posting it here instead of bringing up the same few arguments over and over again.

Changed 6 years ago by charles

queue.3.patch + typo fix in daemon + more gtk prefs work

comment:78 follow-up: Changed 6 years ago by KyleK

To add my 2 cents:

  1. An interval of 1 minute between queue changes is way too much in my opinion. Once queueing makes it into the interfaces (especially web), users will want to see immediate results of their actions (e.g. when moving torrents up/down the queue).

Can't this be done in an event-driven way rather than relying on timers?

  1. I personally would prefer if the queue rank stays visible in transmission-remote (and is available via RPC, for the web interface), but this is something that I can probably patch back in myself if you really don't want it.
  1. I agree with Longinus that the queue mechanism should be able to stop torrents. Why wouldn't it?

If I have 2 of 3 torrents downloading, and move the last one to the top of the queue, I expect one of the torrents that were downloading before to stop. This is how a queue works in my opinion. Everything else will be confusing to the users. (As a result, force-started torrents need a flag that identifies them as force-started, which should be reflected in any of the UIs somehow).

comment:79 Changed 6 years ago by krx

  • Cc krx@… removed

comment:80 in reply to: ↑ 77 Changed 6 years ago by livings124

Replying to Longinus00:

Replying to livings124:

If I have three queued transfers all with no peers, will the client keep on pausing and resuming them in a circle? The Mac implementation would (eventually) have all three active so that one could eventually become active.

We've had this exact same discussion on irc before and I already explained to you before that stalled torrents don't take up a slot in the queue.

So do the torrents auto-stop or not? The way you say it, it's both? That makes no sense.

If it's not indicated in the UI, won't the user wonder why there are too many transfers running? What if they want it to then be part of the queue count? The Mac client avoids this by not auto pausing and counting all active transfers as part of the count.

It's not required that the ui say anything, but it can certainly say "forced downloading" or something instead of "downloading".

My point is that's overkill. If the UI doesn't show anything, won't users say "why is Transmission not honoring my queue? If you have a queue that autostops if it's over the limit, then the UI must distinguish and give options for control of these forced transfers - it's a ugly mess and confusing to the user either way.

How is it straightforward if your queue allows transfers outside of the queue that are never part of the queue, especially if it's not indicated in the UI. I think it's very confusing if the client is pausing and resuming transfers all over the place, all the time. I can think of a bunch of cases that are confusing. For example, say I have transfers 1 2 and 3. 1 is active, the others paused. 1 is auto-paused, and 2 is started.

Why would 1 be paused?

Do they auto stop or not? If I have a torrent limit, why wouldn't one be paused? Are you saying that the queue enforces the limit in some (unexplained) way, but not the other. How does the user know if it's following the queue limit or not?

I then add transfer 4 to the queue, and move it's order to before 1. If 2 is auto-paused, does 4 start, or 3? What happens if then the next-active one is paused? How does it determine the order? The Mac client avoids all of this by just always starting the first queued transfer in the list - since it doesn't auto-pause, you don't have this confusing mess.

I can't answer the rest of this until you clarify why 1 would be paused.

The Mac client will adopt whatever the new code is. I don't know why you're talking about CPU - although I am concerned that your implementation requires a delay/interval between actions.

There is no requirement for doing anything, I choose to add a timeout for two reasons:

  1. stopping/starting/etc. x torrents requires x calls to torrent_stop/start/etc. Instead of running through the queue x times in quick succession it's much more efficient to just do it once.
  2. The queue looks at the transfer speed of a torrent torrent to help if figure out if it should be skipped or not and transfer speeds are meaningless over very short intervals.

Who cares if there's multiple calls. It's the right thing to do. It should be event driven, not timer driven. I agree with the speed/time limit feature, but that's separate.

Why did you ask for a description and questions of the Mac implementation if you had no intent of following the core design with no discussion to flesh things out.

I've followed basically every other part of the mac implementation so there's no need for hyperbole. We've had discussions about stopping torrents before yet you seem not to have remembered any of them. You also have obviously not taken the time to glance over the code I've posted or you wouldn't be asking these same questions. If you don't like what I've written up you can just tell me not to bother posting it here instead of bringing up the same few arguments over and over again.

I have looked over a big part of the code. I still cannot understand the behavior of both a mix of auto-stop and not. And yes, I did know about the force start question I asked - I was pointing out was confusion and difficulty it leads to. I write the same few arguments because you never give me a clear, good answer. I'm hoping this will lead to improvements to your queue implementation, which is why I wish you were more receptive to constructive criticism.

Last edited 6 years ago by livings124 (previous) (diff)

comment:81 Changed 6 years ago by garrych

  • Cc garrych@… removed

comment:82 Changed 6 years ago by Longinus00

Replying to livings124:

So do the torrents auto-stop or not? The way you say it, it's both? That makes no sense.

I've already told you before that stalled torrents don't count as taking up a queue slot.

My point is that's overkill. If the UI doesn't show anything, won't users say "why is Transmission not honoring my queue? If you have a queue that autostops if it's over the limit, then the UI must distinguish and give options for control of these forced transfers - it's a ugly mess and confusing to the user either way.

How is it an ugly mess?

Do they auto stop or not? If I have a torrent limit, why wouldn't one be paused? Are you saying that the queue enforces the limit in some (unexplained) way, but not the other. How does the user know if it's following the queue limit or not?

How is it unexplained when it's clearly explained in the options screen? I can change the wording if it makes you feel better.

Who cares if there's multiple calls. It's the right thing to do. It should be event driven, not timer driven. I agree with the speed/time limit feature, but that's separate.

So dumping 10 torrents into the watchdir should spam 10 calls to the queue? Why walk through the queue list 10 times?

I have looked over a big part of the code. I still cannot understand the behavior of both a mix of auto-stop and not. And yes, I did know about the force start question I asked - I was pointing out was confusion and difficulty it leads to. I write the same few arguments because you never give me a clear, good answer. I'm hoping this will lead to improvements to your queue implementation, which is why I wish you were more receptive to constructive criticism.

If you compare my previous patches you'll see that I've done virtually everything that I was told to by you and charles up to this auto stopping torrent thing (which I've said multiple times I can define out in the mac version). I've even implemented what KyleK wanted in regards to quicker queue response.

I'm highly suspect of how much of my patch you have actually read through since virtually all the queue handling logic is in a single function yet you still don't, apparently, understand how it works and come up with all sorts of nonsensical and impossible situations. I don't find it very constructive when people are blatantly false in their criticisms. Why don't I walk you through the start/stop code in the queue since you seem unwilling or unable to do it yourself.

 	655	    for( i = 0; i < count; ++i ) 
 	656	    { 
 	657	        tr_torrent * tor = torrents[i]; 

Walk through an array of torrents presorted by queue ranking.

 	658	 
 	659	        if( !tr_torrentIsQueued( tor ) 
 	660	            || tor->error == TR_STAT_LOCAL_ERROR 
 	661	            || tor->verifyState != TR_VERIFY_NONE ) 
 	662	            continue;

Ignore torrents that are forced/paused/verifying/have an error.

 	663	 
 	664	        if( download && !tr_torrentIsSeed( tor ) ) 
 	665	        { 

Only concern ourselves with downloading torrents.

	666	            if( maxDown > 0 ) 

Do we have room in the queue?

 	667	            { 
 	668	                if( tor->isRunning && skip && testSkipTorrent( tor, slowCount, slowCutoff, TR_DOWN ) ) 
 	669	                    continue; 

If the user wishes, slow/stalled torrents don't count against the queue and are skipped.

 	670	 
 	671	                if( !speedLimit ) 
 	672	                    tr_torrentStartQueued( tor ); 
 	673	 
 	673	 
 	674	                --maxDown; 
 	675	            } 

Start the torrent if we aren't hitting a speedlimit (user option) and decrement the number of slots left in the download queue.

 	676	            else if( tor->isRunning ) 
 	677	                tr_torrentStopQueued( tor ); 
 	678	        } 

If there were no spaces in the download queue then stop this and all following torrents. Putting these two lines in an ifdef (like I've offered to do multiple times) would completely revert the behavior to the current mac behavior.

There, the entirety of the actual start/stop logic (the seeding logic is exactly the same) in less than 25 lines including newlines. This code has hardly changed at all in the last 8 months so you've had plenty of time to look at it. Now I'd love to hear how this logic (which you've supposedly looked over) could lead to any of your ridiculous scenarios and how you mentioning them is constructive.

comment:83 in reply to: ↑ 78 Changed 6 years ago by Longinus00

Replying to KyleK:

To add my 2 cents:

  1. An interval of 1 minute between queue changes is way too much in my opinion. Once queueing makes it into the interfaces (especially web), users will want to see immediate results of their actions (e.g. when moving torrents up/down the queue).

Can't this be done in an event-driven way rather than relying on timers?

As I stated in an earlier reply to livings I worked this out by buffering events. It's also not practical to make a queue only event driven because there's no way to know if a torrent is slow/stalled without testing it. I'll upload it after I've tested it some more if you want.

  1. I personally would prefer if the queue rank stays visible in transmission-remote (and is available via RPC, for the web interface), but this is something that I can probably patch back in myself if you really don't want it.

Absolutely everything (and even more) is available through the rpc interface. Remember that the QT client communicates through rpc exclusively.

The reason I'm taking the queueranks out of remote -l is because, from what I've seen, many scripts that parse the remote's output are extremely fragile; adding or changing a field in -l will probably break everything. The rank is still available in -i if you really want to get it that way. I highly recommend using the qt or web, when it's done, interfaces to control transmission instead of the remote which is just a big mess.

comment:84 follow-up: Changed 6 years ago by livings124

You need to calm down. This is what I meant when I said it was written out and straight-forward. My main concern is over the issue of force-started transfers. Would it be possible to find a way to add them to the regular queue without needing to special-case it, since that is going to add difficulty to displaying it in the interface and a whole bunch of options. Also, yes, 10 torrents in the watchdir should result in 10 calls to the queue code. The Mac client is also going to need some sort of callback so it knows when the queue starts/stops transfers.

comment:85 in reply to: ↑ 84 ; follow-up: Changed 6 years ago by Longinus00

Replying to livings124:

You need to calm down. This is what I meant when I said it was written out and straight-forward. My main concern is over the issue of force-started transfers. Would it be possible to find a way to add them to the regular queue without needing to special-case it, since that is going to add difficulty to displaying it in the interface and a whole bunch of options.

But forced started transfers are a special case. Just changing their status string to something like "Forced download"/"Forced seed" is simple and gets the idea across.

Also, yes, 10 torrents in the watchdir should result in 10 calls to the queue code.

I don't see any benefit to going through the queue 10 times vs. 1 time after they're all added. Just like selecting "start all" should only go through the queue once and not as many times as there are torrents. If this is some mac specific thing I can do it with an ifdef.

The Mac client is also going to need some sort of callback so it knows when the queue starts/stops transfers.

That shouldn't be a problem.

comment:86 in reply to: ↑ 85 ; follow-up: Changed 6 years ago by livings124

Alright, I think we're getting somewhere. :)

Replying to Longinus00:

But forced started transfers are a special case. Just changing their status string to something like "Forced download"/"Forced seed" is simple and gets the idea across.

What about options to put it back in the queue, etc. Just seems a bit much. And "forced download" just seems a bit confusing.

Replying to Longinus00:

I don't see any benefit to going through the queue 10 times vs. 1 time after they're all added. Just like selecting "start all" should only go through the queue once and not as many times as there are torrents. If this is some mac specific thing I can do it with an ifdef.

As long as there's not a noticeable delay between happening I'm fine with this. What I'm worried about is torrents not starting/stopping when expected, then all of a sudden a few seconds later a whole bunch of things happening at once.

Replying to Longinus00:

That shouldn't be a problem.

Thanks

comment:87 in reply to: ↑ 86 Changed 6 years ago by Longinus00

Replying to livings124:

Alright, I think we're getting somewhere. :)

Like I said earlier, my main issue was the "situations" you came up which I'm sure we've gone over in the past.

What about options to put it back in the queue, etc. Just seems a bit much. And "forced download" just seems a bit confusing.

Pausing will unforce a torrent easily enough. If you want it more sophisticated than that then what I've done is a checkmark in the torrent menu (which doubles as the force start button). Checking it on a non force-started torrent tells it to ignore the queue and starts it if it's not already started. Unforceing is just unchecking.

All the other clients that have auto seeding have some sort of forced transferring. I don't know/remember how they deal with the ui issues exactly but it's probably something similar.

As long as there's not a noticeable delay between happening I'm fine with this. What I'm worried about is torrents not starting/stopping when expected, then all of a sudden a few seconds later a whole bunch of things happening at once.

What happens is that anytime an event that could affect the queue goes off (torrents added, manually paused, queued, queue settings changed, etc.) it starts a 2 second timer at the end of which the queue is processed. If another queue event happens in the next 2 seconds the timer is reset. Otherwise the timer is on a 60 second timeout so it can notice slow/stalled torrents.

comment:88 Changed 6 years ago by livings124

Both timers seem way too long. Perhaps half a second and 15 seconds, accordingly.

comment:89 Changed 6 years ago by charles

I'm not sure it makes any difference whether the steady-state timer recalculates every 15 seconds vs every 60 seconds. It seems to me that the main timer issue is to make sure that it's responsive to user interaction, which is handled by the 2 second timer.

comment:90 Changed 6 years ago by livings124

Every minute is a real long time, though. But anyway, I'm more concerned with the 2 second timer being long. 2 seconds is noticeable.

comment:91 Changed 6 years ago by Longinus00

I don't see an benefit to this at all but if you guys want it it's doable. The only issue is that this is going to eat up more memory (two tr_historys per torrent a queue as a ptrarray).

comment:92 follow-up: Changed 6 years ago by KyleK

Do these timers exist in the current patch already? Then we can easily do some experiments and change their values to whatever and see if it has a noticeable impact on CPU/memory/whatever.

I do agree though that the UI has to be responsive.

To add my 2 cents:

An interval of 1 minute between queue changes is way too much in my opinion. Once >>queueing makes it into the interfaces (especially web), users will want to see immediate >>results of their actions (e.g. when moving torrents up/down the queue). Can't this be done in an event-driven way rather than relying on timers?

As I stated in an earlier reply to livings I worked this out by buffering events. It's also >not practical to make a queue only event driven because there's no way to know if a torrent >is slow/stalled without testing it. I'll upload it after I've tested it some more if you >want.

What if stuff like adding a torrent, force-starting it, moving it up/down the queue is handled by events, but checking for slow/stalled torrents is done by the timer? Then this can stay at, say, 60 seconds interval, and everything else is dealt with instantly.

comment:93 Changed 6 years ago by praveenmarkandu

  • Cc praveenmarkandu@… removed

comment:94 Changed 6 years ago by dekatrion

  • Cc kbmurata@… added

comment:95 Changed 6 years ago by dekatrion

  • Cc kbmurata@… removed

comment:96 Changed 6 years ago by snake98

  • Cc snake98@… added

comment:97 Changed 6 years ago by snake98

  • Cc snake98@… removed

comment:98 in reply to: ↑ 92 ; follow-up: Changed 6 years ago by Longinus00

Replying to KyleK:

What if stuff like adding a torrent, force-starting it, moving it up/down the queue is handled by events, but checking for slow/stalled torrents is done by the timer? Then this can stay at, say, 60 seconds interval, and everything else is dealt with instantly.

This is exactly what I've described earlier.

The new version I've uploaded has my event changes. The mac client also no longer pauses torrents so the 'force start' problem no longer exists for it. It also has special provisions so that the following now happens in the mac client exactly as described.

Also, yes, 10 torrents in the watchdir should result in 10 calls to the queue code.

Last edited 6 years ago by Longinus00 (previous) (diff)

comment:99 in reply to: ↑ 98 Changed 6 years ago by livings124

Replying to Longinus00:

The mac client also no longer pauses torrents so the 'force start' problem no longer exists for it.

I'd prefer the Mac client to behave like the other clients. I'm fine with the auto-start behavior as you described it yesterday.

comment:100 Changed 6 years ago by KyleK

I'm confused: queue.3.patch is newer than queue.4.patch? Which one is the most current version of the patch?

comment:101 Changed 6 years ago by charles

Good question. Longinus00, does queue.3.patch supercede queue.4.patch?

comment:102 Changed 6 years ago by Longinus00

I replace previous files because if I didn't the attachments section would be longer than the comments.

Changed 6 years ago by charles

exactly the same content, but resynced to r11380.

comment:103 Changed 6 years ago by charles

Some random thoughts from the current version of queue.3.patch --

  • I agree "Force Start" should be in the menu and context menu, but I wonder how many people unfamiliar with the queueu will be confused with it being next to "Start"
  • IMO we should consider turning on "skip slow torrents" by default.
  • Are there real-world use cases for adding torrents to the top of the queue and the bottom of the queue?
  • On "don't start new torrents if a speed limit is reached" ... Just thinking out loud on this bullet point... I guess the goal here is to let the user decide about the tradeoff between maximizing bandwidth use, and in all torrents a chance to be active. What if we were to leave the active torrents running while we cycle through the queue, and only stop them when one of the torrents we cycle to shows signs of life? That would accomplish both of those goals automatically and let us get rid of one more checkbox. :) .... though, I'd like a second opinion on this so that I'll know if I'm talking out of my ass on this.
Last edited 6 years ago by charles (previous) (diff)

comment:104 follow-up: Changed 6 years ago by charles

I have a question about basic principles: what is the point of the queue? in the implementation level the answer is "to limit the number of transmissions that are running"... but what is the goal of doing that? If all flavors of Transmission had a bug-free queue and a user enabled it, what would be the benefit to that user over having no queue at all?

Changed 6 years ago by charles

uTorrent's preferences dialog's queue tab

comment:105 Changed 6 years ago by KyleK

For me it all comes down to resources:

  1. Bandwidth: If you have limited bandwidth, and want one download to finish faster than the others, you put it on top of the queue. This is basically just a form of prioritization (sic?). I know you can assign torrents to get more bw than others, but it feels ...passive. The user doesn't really have control over it. That might just be me though :)
  1. CPU: this only applies to a limited number of users, but since I have transmission running on an embedded device, running many torrents at high speed will make access to the NAS impossible. By initiating a queue I can (somewhat) limit the resources Transmission uses.

comment:106 Changed 6 years ago by garrych

  • Cc garrych@… added

comment:107 Changed 6 years ago by garrych

  • Cc garrych@… removed

comment:108 Changed 6 years ago by hgabreu

When one has limited bandwidth i.e. when any torrent consumes all your bandwidth, a queue is mandatory! If you let all torrents fighting for space, they are all going to take an eternity to download (even worse for uploads) and it's sure to frustrate the user. I'm waiting a queue implementation in transmission for my whole life :P

comment:109 follow-up: Changed 6 years ago by charles

Comments on the implementation in Longinus00's queue.3.patch circa Nov 2:

  1. tr_sessionSortQueue() and tr_sessionCompactQueue() should be static methods
  1. Bug in fireQueueCallback(): it uses the tor->completeness_func_user_data argument but should be using tor->queue_hit_func_user_data
  1. there appears to be a bug in session.c:
    1. if( tr_bencDictFindInt ( settings, TR_PREFS_KEY_QUEUE_SLOW_COUNT, &i ) )
    2.    session->queueSlowCount = i;
    3.    tr_sessionSetQueueSlowCount( session, i );

Here, line 2 ensures that line 3 is a no-op. If this is intentional, line 3 should be removed. Otherwise, line 2 should be removed. The indentation implies that line 3 is the error, but I don't want to make assumptions.

  1. Calling tr_torrentSetQueued() from tr_torrentCheckSeedLimit() seems odd. Since tor->isStopping is set, we know the torrent will be dequeued shortly via bandwidthPulse()'s call to tr_torrentStop(). So this seems redundant. If it's /not/ redundant and these two calls are necessary for some oddball case, it should be noted in comments.
  1. Adding four buttons to the GTK+ toolbar is excessive -- the queue is important, but are these four buttons going to be used enough to warrant toolbar buttons? IMO they should go in the Torrent menu.
  1. tr_sessionGetQueueSlowCount() is oddly named. It's probably an attempt to avoid excessive verbosity that some session properties have... but still, Count() is misleading. Maybe Minutes() or IntervalMinutes?() instead?
  1. I don't understand the advantage of tr_torrent.{uploadedQueueKBps,downloadedQueueKBps} over calling tr_bandwidthGetPieceSpeed_Bps on the torrent's bandwidth field.
  1. I don't understand why tor->anyDate isn't set anymore in tr_torrentRecheckCompleteness():
        - tor->doneDate = tor->anyDate = tr_time();
        + tor->doneDate = tr_time();
    
    Actually the old approach was suboptimal too -- we should use tr_torrentSetDoneDate().
  1. I'm not comfortable with the way the innards of tr_ptrArray are dinked with in torrentSetQueueRank(). Wouldn't it be simpler/clearer to use tr_ptrArrayErase() + tr_ptrArrayInsert()?
  1. During steady state, processQueue() is only called once per minute. We could eliminate a lot of fiddly code in torrent.c and session.c by building a temporary tr_torrent* array in processQueue() sorted by queueRank, and get rid of tr_session.queueList.
  1. This section in torrent.c needs a comment to explain it. Maybe someone else would understand it but I don't... :)
    if( tor->isQueued )
        tr_torrentStart( tor );
    else
        torrentStart( tor );
    

Longinus00, you've been away from the #transmission channel for an unusually long time, and your last words in the channel were to express frustration about this ticket's slow pace. Are you still monitoring this ticket and/or involved with Transmission? In other words, is it worthwhile for me to spend more time reviewing this patch?

comment:110 in reply to: ↑ 109 ; follow-up: Changed 6 years ago by Longinus00

Replying to charles:

Comments on the implementation in Longinus00's queue.3.patch circa Nov 2:

  1. tr_sessionSortQueue() and tr_sessionCompactQueue() should be static methods

SortQueue? will be useful for moving lots of torrents at a time. CompactQueue? could probably be static.

  1. Adding four buttons to the GTK+ toolbar is excessive -- the queue is important, but are these four buttons going to be used enough to warrant toolbar buttons? IMO they should go in the Torrent menu.

This makes moving torrents around needlessly cumbersome. Say you want to move a torrent up 3 positions. Right now you have to click a button 3 times without moving the mouse. If the movement controls are buried in a menu then not only the amount of clicks double but the mouse will have to be moved. Other options could be removing buttons entirely and implementing drag and drop (I wonder how discoverable this is) or removing the 'move to top/bottom' buttons.

  1. I don't understand the advantage of tr_torrent.{uploadedQueueKBps,downloadedQueueKBps} over calling tr_bandwidthGetPieceSpeed_Bps on the torrent's bandwidth field.

The point is to average the speed over a period of time up to a minute.

  1. I don't understand why tor->anyDate isn't set anymore in tr_torrentRecheckCompleteness():
        - tor->doneDate = tor->anyDate = tr_time();
        + tor->doneDate = tr_time();
    
    Actually the old approach was suboptimal too -- we should use tr_torrentSetDoneDate().

The reason it's not set anymore is because anyDate is changed to be set in setDirty().

  1. I'm not comfortable with the way the innards of tr_ptrArray are dinked with in torrentSetQueueRank(). Wouldn't it be simpler/clearer to use tr_ptrArrayErase() + tr_ptrArrayInsert()?

That would require double the amount of memmoves and the memmoves would probably be for more memory.

  1. During steady state, processQueue() is only called once per minute. We could eliminate a lot of fiddly code in torrent.c and session.c by building a temporary tr_torrent* array in processQueue() sorted by queueRank, and get rid of tr_session.queueList.

This is what it used to be like before it was requested that the queue run an arbitrary number of times in under a millisecond.

  1. This section in torrent.c needs a comment to explain it. Maybe someone else would understand it but I don't... :)
    if( tor->isQueued )
        tr_torrentStart( tor );
    else
        torrentStart( tor );
    

If a torrent finishes and gets verified (moves across partitions) then when it restarts it should respect the queue unless it was forced.

Replying to charles:

  • On "don't start new torrents if a speed limit is reached" ... Just thinking out loud on this bullet point... I guess the goal here is to let the user decide about the tradeoff between maximizing bandwidth use, and in all torrents a chance to be active. What if we were to leave the active torrents running while we cycle through the queue, and only stop them when one of the torrents we cycle to shows signs of life? That would accomplish both of those goals automatically and let us get rid of one more checkbox. :) .... though, I'd like a second opinion on this so that I'll know if I'm talking out of my ass on this.

You're going to need to post some pseudo code because I don't know what you mean by this.

Last edited 6 years ago by Longinus00 (previous) (diff)

comment:111 in reply to: ↑ 110 ; follow-up: Changed 6 years ago by charles

Replying to Longinus00:

Replying to charles:

  1. Adding four buttons to the GTK+ toolbar is excessive -- the queue is important, but are these four buttons going to be used enough to warrant toolbar buttons? IMO they should go in the Torrent menu.

This makes moving torrents around needlessly cumbersome. Say you want to move a torrent up 3 positions. Right now you have to click a button 3 times without moving the mouse. If the movement controls are buried in a menu then not only the amount of clicks double but the mouse will have to be moved. Other options could be removing buttons entirely and implementing drag and drop (I wonder how discoverable this is) or removing the 'move to top/bottom' buttons.

Removing the top/bottom buttons would be consistent with how uTorrent and Deluge do it. Removing them all would be consistent with KTorrent, qBitTorrent, and Monsoon, FWIW.

  1. I don't understand the advantage of tr_torrent.{uploadedQueueKBps,downloadedQueueKBps} over calling tr_bandwidthGetPieceSpeed_Bps on the torrent's bandwidth field.

The point is to average the speed over a period of time up to a minute.

This makes me wonder about the Mac client's approach of "idle for N minutes" which is simple and cheap (tr_stat.idleSecs). queue.3.patch's approach is more sophisticated, but I wonder if the extra functionality it's worth the added complexity.

  1. During steady state, processQueue() is only called once per minute. We could eliminate a lot of fiddly code in torrent.c and session.c by building a temporary tr_torrent* array in processQueue() sorted by queueRank, and get rid of tr_session.queueList.

This is what it used to be like before it was requested that the queue run an arbitrary number of times in under a millisecond.

I don't see that in the code. It looks like the queue runs, in the worst case, once per QUEUE_MIN_INTERVAL_SECS. IMO the code complexity here outweighs the marginal CPU wins that queueList has over building the list dynamically.

  1. This section in torrent.c needs a comment to explain it. Maybe someone else would understand it but I don't... :)
    if( tor->isQueued )
        tr_torrentStart( tor );
    else
        torrentStart( tor );
    

If a torrent finishes and gets verified (moves across partitions) then when it restarts it should respect the queue unless it was forced.

That is not clear from that code :)

comment:112 in reply to: ↑ 111 Changed 6 years ago by Longinus00

Replying to charles:

Replying to Longinus00:

Replying to charles:

  1. Adding four buttons to the GTK+ toolbar is excessive -- the queue is important, but are these four buttons going to be used enough to warrant toolbar buttons? IMO they should go in the Torrent menu.

This makes moving torrents around needlessly cumbersome. Say you want to move a torrent up 3 positions. Right now you have to click a button 3 times without moving the mouse. If the movement controls are buried in a menu then not only the amount of clicks double but the mouse will have to be moved. Other options could be removing buttons entirely and implementing drag and drop (I wonder how discoverable this is) or removing the 'move to top/bottom' buttons.

Removing the top/bottom buttons would be consistent with how uTorrent and Deluge do it. Removing them all would be consistent with KTorrent, qBitTorrent, and Monsoon, FWIW.

So which do you prefer?

  1. I don't understand the advantage of tr_torrent.{uploadedQueueKBps,downloadedQueueKBps} over calling tr_bandwidthGetPieceSpeed_Bps on the torrent's bandwidth field.

The point is to average the speed over a period of time up to a minute.

This makes me wonder about the Mac client's approach of "idle for N minutes" which is simple and cheap (tr_stat.idleSecs). queue.3.patch's approach is more sophisticated, but I wonder if the extra functionality it's worth the added complexity.

This means that slow torrents will clog up the queue.

  1. During steady state, processQueue() is only called once per minute. We could eliminate a lot of fiddly code in torrent.c and session.c by building a temporary tr_torrent* array in processQueue() sorted by queueRank, and get rid of tr_session.queueList.

This is what it used to be like before it was requested that the queue run an arbitrary number of times in under a millisecond.

I don't see that in the code. It looks like the queue runs, in the worst case, once per QUEUE_MIN_INTERVAL_SECS. IMO the code complexity here outweighs the marginal CPU wins that queueList has over building the list dynamically.

This feature was enabled in a previous patch but I forgot to add it into the last one I posted.

--- libtransmission/session.c
+++ libtransmission/session.c
@@ -769,14 +769,6 @@ processQueue( tr_session * session, const tr_bool download, const tr_bool seed )
     }
 }
 
-void
-tr_sessionProcessQueue( tr_session * session )
-{
-    assert( tr_isSession( session ) );
-
-    tr_timerAdd( session->queueTimer, QUEUE_MIN_INTERVAL_SECS, 0 );
-}
-
 /***
 ****
 ***/
@@ -797,6 +789,18 @@ onQueueTimer( int foo UNUSED, short bar UNUSED, void * vsession )
     tr_timerAdd( session->queueTimer, QUEUE_INTERVAL_SECS, 0 );
 }
 
+void
+tr_sessionProcessQueue( tr_session * session )
+{
+    assert( tr_isSession( session ) );
+
+#ifdef SYS_DARWIN
+    onQueueTimer( 0, 0, session );
+#else
+    tr_timerAdd( session->queueTimer, QUEUE_MIN_INTERVAL_SECS, 0 );
+#endif
+}
+
 /***
 ****
 ***/

Either way, removing the queueList is no big deal.

comment:113 Changed 6 years ago by Shu

  • Cc Shu added

comment:114 in reply to: ↑ 104 Changed 6 years ago by DFreeze

Replying to charles:

I have a question about basic principles: what is the point of the queue? in the implementation level the answer is "to limit the number of transmissions that are running"... but what is the goal of doing that? If all flavors of Transmission had a bug-free queue and a user enabled it, what would be the benefit to that user over having no queue at all?

Long time ago, I send in a request basically covering what this ticket does. For me the reason for wanting a queue is that my modem is too cheap to cope with many connections at a time. Downloading using transmission (on my CH3SNAS) results in a choked modem and thus broken internet. Only a reboot fixes it. A queue would limit the number of active torrents, and thus the number of connections - without having to manually feed the torrents one by one. I just fill the queue and transmission works through the list using my preferences.

So, there's a real-world reason for wanting a queue ;-)

Whatever the outcome: thanks a million for such a versatile and agile torrent client. Been a fan since the 0.x days!

comment:115 follow-up: Changed 6 years ago by charles

Okay, next basic question: let's say you're seeding a whole lot of pdfs from legaltorrents and you're using the upload queue. You don't know which torrent will pick up a downloader, or when. By the time the upload queue cycles around to a swarm with a downloader, the downloader will already be finished.

So does an upload queue make sense?

What we're wanting to do is limit the number of peers based on available bandwidth. For seeds, wouldn't that be better to do automatically rather than forcing users to choose which torrents should get seeded first?

comment:116 in reply to: ↑ 115 Changed 6 years ago by Longinus00

Replying to charles:

Okay, next basic question: let's say you're seeding a whole lot of pdfs from legaltorrents and you're using the upload queue. You don't know which torrent will pick up a downloader, or when. By the time the upload queue cycles around to a swarm with a downloader, the downloader will already be finished.

So does an upload queue make sense?

What we're wanting to do is limit the number of peers based on available bandwidth. For seeds, wouldn't that be better to do automatically rather than forcing users to choose which torrents should get seeded first?

What does "upload queue cycles around" mean? I asked you before what "cycle through the queue" meant and you have yet to respond.

comment:117 follow-up: Changed 6 years ago by charles

If I correctly understand the upload queue proposals, it the queue initially has N torrents seeding, and then if they are inactive, more torrents are slowly added in as well. That's what I mean by cycling through.

IMO this kind of ordering makes no sense in real world situations. You want to start off seeding all torrents possible, and then only when you run out of bandwidth should you decide which torrents to loadshed.

comment:118 in reply to: ↑ 117 Changed 6 years ago by Longinus00

Replying to charles:

If I correctly understand the upload queue proposals, it the queue initially has N torrents seeding, and then if they are inactive, more torrents are slowly added in as well. That's what I mean by cycling through.

IMO this kind of ordering makes no sense in real world situations. You want to start off seeding all torrents possible, and then only when you run out of bandwidth should you decide which torrents to loadshed.

Most other(all?) torrent queues implement some sort of seed order adjusting and I implemented something like that in my original queue.patch. After the fight you had with livings you've told me to just copy what the mac client does. Luckily the mac client version lets you turn off the seeding queue and let libtransmission attempt to fight over upload slots(unimplemented) which is more or less what you're describing.

Also, even if it takes some time to start seeding all the torrents it's not like it will miss any further downloads of pdfs from legal torrents so this isn't that huge of a deal anyway. Remembering the "skipped" status across restarts should help.

Last edited 6 years ago by Longinus00 (previous) (diff)

comment:119 Changed 6 years ago by giovannibajo

  • Cc giovannibajo@… added

comment:120 Changed 6 years ago by atommixz

  • Cc atommixz@… added

comment:121 follow-up: Changed 6 years ago by KyleK

The patch needs updating, it doesn't apply cleanly anymore since a couple of trunk commits.

comment:122 in reply to: ↑ 121 Changed 6 years ago by atommixz

Replying to KyleK: This is natural, the patch was written for a specific revision. You should use it with a specific revision, so as not to get problems. No one will overwrite the patch with adaptation to each new revision. It's hard.

comment:123 Changed 6 years ago by livings124

  • Milestone changed from 2.20 to Sometime

comment:124 Changed 6 years ago by jordan

  • Component changed from libtransmission to Transmission
  • Owner changed from Longinus00 to jordan
  • Status changed from new to assigned

comment:125 Changed 6 years ago by zero0n3

  • Cc altrov3@… added

comment:126 Changed 6 years ago by alexanderwei

  • Has this ticket been finished?
  • Is there any official patch for 2.21?

comment:127 Changed 6 years ago by jordan

If the ticket was finished, it would be closed :)

comment:128 follow-up: Changed 5 years ago by primzy

What is the latest version I can patch?

With witch patch file/s?

comment:129 Changed 5 years ago by zero0n3

Hi all can someone explain to us how to install patches? thanks regards

comment:130 Changed 5 years ago by jordan

  • Milestone changed from Sometime to 2.40

jordan * r12607 /trunk/ (38 files in 6 dirs): (trunk) #671 "torrent queuing" -- Preliminary implementation. Covers libtransmission, GTK+ and Qt clients, and basic web client support. The Mac client and transmission-remote have not been touched.

This queue implementation follows the same basic ideas as the Mac client as described in comment:57

Last edited 5 years ago by jordan (previous) (diff)

comment:131 follow-ups: Changed 5 years ago by gunzip

transmission-daemon 2.33+ (12608)

my first run with this i noticed new entries in settings.json

"download-queue-enabled": true, 
"download-queue-size": 10, 
"queue-stalled-minutes": 30, 
"seed-queue-enabled": false, 
"seed-queue-size": 10, 

is the intent to have this enabled by default? if so what does it mean?

when viewing WebGU with Firefox 5.0, there is nothing regarding Queuing apparent, so it's hard for user to judge what is going on.

when using transmission-remote-cli.py, a test torrent seemed to download OK but instead of reaching seed state, it displayed "unknown state" after reaching 100%. i think because of this, the pause/resume keys became non-functional. similarly, in the WebGU the Inspector labeled state as "error".

comment:132 in reply to: ↑ 131 Changed 5 years ago by jordan

Replying to gunzip:

transmission-daemon 2.33+ (12608)

my first run with this i noticed new entries in settings.json

"download-queue-enabled": true, 
"download-queue-size": 10, 
"queue-stalled-minutes": 30, 
"seed-queue-enabled": false, 
"seed-queue-size": 10, 

is the intent to have this enabled by default? if so what does it mean?

Those are the default settings for now, but they might change before 2.40 goes final.

  • "download-queue-enabled" is whether or not to limit how many downloads at once.
  • "download-queue-size" is how many active downloads to allow at once when "download-queue-enabled" is true.
  • "queue-stalled-minutes" -- if a torrent is idle for N minutes, it's considered stalled rather than active, so it's not counted towards "download-queue-size" or "seed-queue-size".
  • "seed-queue-enabled" is whether or not to limit how many seeds at once.
  • "seed-queue-size" is how many active seeds to allow at once when "seed-queue-enabled" is true.

when viewing WebGU with Firefox 5.0, there is nothing regarding Queuing apparent, so it's hard for user to judge what is going on.

The web gui still needs more work. transmission-qt and transmission-gtk are further along wrt exposing these features.

when using transmission-remote-cli.py, a test torrent seemed to download OK but instead of reaching seed state, it displayed "unknown state" after reaching 100%. i think because of this, the pause/resume keys became non-functional.

Some of the RPC values changed, so the third-party remote controls will need to make code changes based on https://trac.transmissionbt.com/changeset/12607/trunk/extras/rpc-spec.txt ... this feature didn't exist in RPC until yesterday.

Last edited 5 years ago by jordan (previous) (diff)

comment:133 Changed 5 years ago by gunzip

OK, the above explanation helps a lot.

As further test i kept download-queue-enabled to true, but set download-queue-size to 2, and then added 3 new torrents. The first 2 torrents started downloading, while third had message "waiting to download" as viewed in the WebUI. I assume the decision on which torrent to put in queue was based on the sorting by name.

after about 10 minutes, i paused the 2nd torrent which had been downloading, and immediately the third which had been in queue kicked in and started to DL. so at first glance, this Queuing feature seems to be working as advertised.

UPDATE

more testing .. added a handful of small torrents at same time, 2 of the 5 started downloading, while other 3 were marked as waiting. as a torrent finished and went to seeding, one in the queue kicked in and started to DL. this pattern repeated and everything completed as normal but with no more than 2 torrents downloading at any one time.

Last edited 5 years ago by gunzip (previous) (diff)

comment:134 in reply to: ↑ description Changed 5 years ago by leandroong

Replying to jorge:

I would like Transmission could queue multiple torrents and have only one (or more) active at a time.

I have wrote a (very preliminar) patch that allows queueing and setting the seed ratio in Gtk+ client. Actually it is not enough tested (or not tested at all), it pretends to be an example rather than "real" code.

Tomato router developed by shibby has built-in transmission with torrent queing capability. Almost bug free. It has a bittorrent client GUI for setup configuration and option to run built-in or optware transmission. His firmware built-in transmission is always updated, same latest release here. Try it guys. His tomato firmware site is located at http://tomato.groov.pl/K26/build5x-065V/. Note: source code is already there, just request permission from shibby.

comment:135 in reply to: ↑ description Changed 5 years ago by leandroong

Replying to jorge:

I would like Transmission could queue multiple torrents and have only one (or more) active at a time.

I have wrote a (very preliminar) patch that allows queueing and setting the seed ratio in Gtk+ client. Actually it is not enough tested (or not tested at all), it pretends to be an example rather than "real" code.

Here is a sample of tomato router bittorrent GUI image. http://awesomescreenshot.com/043hvb0df

comment:136 Changed 5 years ago by leandroong

There are 2 important modules that run under cron. Namely, btinside and btqueue. Btinside is for keep alive while btqueue is responsible for starting next torrent(s) that were paused, depending on the number of queue maximum allowed by the user, and also it paused torrent(s) running if maximum running is over the maximum allowed by the user.

comment:137 Changed 5 years ago by jordan

leandroong, I'm not sure that I understand your recommending. livings and I have been checking in queue code over the last 2-3 days and gunzip reports that it's "working as advertised."

The screenshot looks great and shibby's approach looks like a smart match for Tomato, but I don't see how cron jobs are a good general purpose solution for other places like transmission-qt running on Windows or for the Mac client.

comment:138 Changed 5 years ago by leandroong

Glad to hear you have working model. I agree 100% percent with what you said. I only show you the current tomato firmware is doing. Only thing I dont like about it, it is cron job that run base on user define minute, not responsive, meaning when current torrent download completed, next torrent schedule will be activated to resume base on define cron minute for queue torrent. Dont worry about it, it can be disabled once your protocol takes place. I hope, you will release your model soon ...

Changed 5 years ago by deltasquare4

WebUI patch to r12627 implementing queue modification

comment:139 in reply to: ↑ 128 ; follow-up: Changed 5 years ago by primzy

Replying to primzy:

What is the latest version I can patch?

With witch patch file/s?


Can any1 plz answer my question.

comment:140 in reply to: ↑ 139 Changed 5 years ago by deltasquare4

Replying to primzy:

Replying to primzy:

What is the latest version I can patch?

With witch patch file/s?


Can any1 plz answer my question.

You don't need to patch anything to have queue support now. As mentioned by jordan, queue has been implemented into libtransmission now. You can just take a checkout of the latest version and build it.

comment:141 Changed 5 years ago by jordan

r12633: #671 "torrent queuing" -- better Web Client support of queue by deltasquare4

comment:142 follow-up: Changed 5 years ago by Elbandi

jordan: pls dont forget to increment the RPC_VERSION macro!

comment:143 Changed 5 years ago by rb07

Jordan:

Testing the current r12638 with T-Qt, queued (waiting) torrents show "Error" below the progress bar, a message like "Queued" would be better, or what the properties inspector shows in state: "Waiting to download".

Also a torrent which is actually seeding shows as state: "Waiting to Seed", that is in the properties inspector, the status line below the progress bar has the usual message (with peer numbers and upload speed).

comment:144 follow-up: Changed 5 years ago by jordan

rb07, I'm not seeing that behavior. Are you positive that both transmission-daemon and transmission-qt are running at the same revision number? If so, could you give me a detailed walkthrough on how to trigger the "Error" message?

comment:145 in reply to: ↑ 142 Changed 5 years ago by jordan

Replying to Elbandi:

jordan: pls dont forget to increment the RPC_VERSION macro!

Thanks! r12641

comment:146 in reply to: ↑ 144 Changed 5 years ago by rb07

Replying to jordan:

Are you positive that both transmission-daemon and transmission-qt are running at the same revision number?

I tested it standalone.

I know its a different story with a different revision daemon, many things don't work.

comment:147 Changed 5 years ago by jw5801

  • Cc jw5801@… removed

comment:148 Changed 5 years ago by jordan

rb07: ah, you're right... I should've seen that. fixed in r12651

comment:149 Changed 5 years ago by rb07

And there's another small problem in the Preferences, changing the time in "Downloads sharing data in the last N minutes are active:" produces this in the console:

unhandled pref: 60

and the value is not really changed.

comment:150 Changed 5 years ago by jordan

fixed in r12661.

comment:151 Changed 5 years ago by hgabreu

Is the queue order being lost whenever transmission is closed? I mean, when I viewing in "Sort by Queue" it seems to be reseted when I close and re-open, is that right? I hope not... oh, and I'm talking about transmission-gtk r12662 on linux.

Also, there shouldn't be some buttons on the toolbar to help out re-ordering the queue?

comment:152 in reply to: ↑ 131 Changed 5 years ago by gunzip

Replying to myself:

transmission-daemon 2.33+ (12608)

my first run with this i noticed new entries in settings.json

"download-queue-enabled": true, 
"download-queue-size": 10, 
"queue-stalled-minutes": 30, 
"seed-queue-enabled": false, 
"seed-queue-size": 10, 

just for the record another new entry appeared in settings.json

"queue-stalled-enabled": true,

transmission-daemon 2.33+ (12662)

comment:153 Changed 5 years ago by gunzip

the WebUI currently shows the status of waiting torrents as "Queued for seeding" or "Queued for download". This is very good and clear.

But when checking status via 'transmission-remote --list', the reported status is incorrect and misleading. for example, i've seen a queud torrent sometimes reported as idle, or seeding. it would be useful to use the same status here as is presently in the WebUI.

transmission-daemon 2.33+ (12665)

comment:154 follow-up: Changed 5 years ago by jordan

gunzip: r12668

comment:155 in reply to: ↑ 154 Changed 5 years ago by gunzip

Replying to jordan:

gunzip: r12668

yep that fixed it .. all is good now

comment:156 Changed 5 years ago by jordan

Are there any details still pending on this ticket?

comment:157 Changed 5 years ago by deltasquare4

I guess transmission-remote still doesn't have interface to interact with queue-related functionality. (e.g. moving up/down)

comment:158 Changed 5 years ago by jordan

  • Resolution set to fixed
  • Status changed from assigned to closed

I don't think transmission-remote needs to have every feature under the sun. It's already pretty bloated for a CLI.

comment:159 Changed 5 years ago by KyleK

Hm. I myself manage my Transmission session almost exclusively through the remote, and I'll miss changing queue values through it.

What ways do currently exist to enable/disable queuing, and to change the max values for downloads/seeds? Will I still have to edit the settings.json and restart Transmission?

comment:160 Changed 5 years ago by atommixz

I also believe that the remote should be able to move the hand up/down. Otherwise, remote not fully. I also know a guy who only uses the remote, I often use it myself. On the other hand, the possible implementation of this in the remote seems to me terrible.

transmission-remote srv -t3456 -up transmission-remote srv -t3455 -up transmission-remote srv -t3454 -up ... LoL. How you do that?

On the other hand, it seems possible transmission-remote srv -t3456 -ontop

comment:161 Changed 5 years ago by leandroong

Missing variables in settings.json "download-queue-enabled": true, "download-queue-size": 10, "queue-stalled-minutes": 30, "seed-queue-enabled": false, "seed-queue-size": 10, "queue-stalled-enabled": true,

Are they disabled?

comment:162 Changed 5 years ago by jordan

leandroong, they should be added when you 2.40b1 writes a new copy of settings.json to disk on shutdown. I'm seeing them here:

$ grep queue ~/.config/transmission/settings.json 
    "download-queue-enabled": true, 
    "download-queue-size": 6, 
    "queue-stalled-enabled": true, 
    "queue-stalled-minutes": 3, 
    "seed-queue-enabled": false, 
    "seed-queue-size": 5, 

comment:163 Changed 3 years ago by lkraav

  • Cc leho@… added

comment:164 Changed 3 years ago by leandroong

Possible to add "queue" category under preference?

comment:165 Changed 2 years ago by Nemo_bis

  • Cc federicoleva@… added

comment:166 Changed 2 years ago by leandroong

Thanks ...

comment:167 Changed 2 years ago by garrych

  • Cc garrych@… added

comment:168 Changed 2 years ago by garrych

  • Cc garrych@… removed
Note: See TracTickets for help on using tickets.