Bug 308579 - Queue order is not downloading order when adding a torrent
Summary: Queue order is not downloading order when adding a torrent
Status: RESOLVED FIXED
Alias: None
Product: ktorrent
Classification: Applications
Component: general (show other bugs)
Version: unspecified
Platform: Gentoo Packages Linux
: NOR normal
Target Milestone: ---
Assignee: Joris Guisson
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-10-18 00:19 UTC by shellixyz4test
Modified: 2013-02-04 17:30 UTC (History)
3 users (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description shellixyz4test 2012-10-18 00:19:06 UTC
When adding a torrent the new torrent goes on top of the queue.
Seems weird to me. Should go to the end IMHO but it's not the point.

The problem is even if the torrent is first in the queue it's not downloaded first. it's only start downloading when I move some torrent in the queue that the real order is applied and the top torrent in the queue starts downloading.

This is with version 4.2.1. I did not test with 4.3.0


Reproducible: Always

Steps to Reproduce:
1. add 2 torrents
2. add an other torrent (this torrent is first in the queue but not downloading)
3. exchange torrents in positions 2 and 3
Actual Results:  
the last torrent added starts downloading

Expected Results:  
the last added torrent is first in the queue so it should start downloading at step 2 not 3
Comment 1 john Terragon 2012-10-22 04:24:26 UTC
I can confirm the exact behavior you described, both with 4.2 and 4.3 branches.
Moreover, say you queued x torrents using only magnet links. If after that you 
queue another torrent, but this time you use a .torrent file, then the relative order 
of the x old torrents in the queue will be completely changed in a seemingly random 
way.

I'm using debian unstable.
Comment 2 shellixyz4test 2012-10-29 19:09:47 UTC
(In reply to comment #1)
> I can confirm the exact behavior you described, both with 4.2 and 4.3
> branches.
> Moreover, say you queued x torrents using only magnet links. If after that
> you 
> queue another torrent, but this time you use a .torrent file, then the
> relative order 
> of the x old torrents in the queue will be completely changed in a seemingly
> random 
> way.
> 
> I'm using debian unstable.

The same thing happened to me today only with .torrent files. Two downloads remaining on top of the queue have been moved to the bottom when adding a third torrent (the queue is relatively long, 100 items).

Seems the code appending torrents to the queue is broken in some way.
Comment 3 shellixyz4test 2012-11-05 19:54:40 UTC
I started to look at the code but it seems to do the right thing when a torrent is added:

queuemanager.cpp:654 in function torrentAdded
foreach (TorrentInterface * otc,downloads) {                                         
  int p = otc->getPriority();       
  otc->setPriority(p+1);            
}                                         
tc->setAllowedToStart(start_torrent);     
tc->setPriority(0);                       
rearrangeQueue();

which sets priority of the added torrent to 0 and increment by 1 other torrents priority.

I don't know yet what's going on. I unsuccessfully tried to inverse the priorities to see if I could get the added torrent to go to the end of the queue but all I was getting is a ktorrent without ability to start torrents.

I'll take a deeper look but i'll appreciate any help from regular devellopers about how the queue is working and tips about debugging ktorrent as it is the first time I take a look at the source code.
Comment 4 john Terragon 2012-11-06 21:51:44 UTC
From what I can see from the code snippet you posted, this priority queue seems to be quite problematic...
First of all, each insertion costs O(n), where n is the number of torrents in the queue. Sure, the torrents in the queue will probably never be more than  a couple of hundreds (I'm also counting the ones that have been finished, which seem to remain in the queue) but one can simply implement the priority queue with some O(log n) tree-based dictionary from the base c++ libraries at zero cost and be done with it.

Second, what's the meaning of the integer storing the priority?I can't tell from the code snippet if a larger integer means a higher priority or vice-versa. Because if we are in the second case the FIFO order is not respected (the new torrent gets priority 0 and jumps in front of all the other ones). On the other hand, if we are in the first case that would explain why the "visual" queue shown in client window doesn't match the actual download order: the visual queue sorts torrents by increasing priority number while the actual downloading order is the opposite, i.e. in decreasing order by priority number (torrents with larger numbers first).
And maybe these conflicting sorting orders are the cause of the other misbehaviour, when one change a torrent priority by moving it down in the visual queue and the actual downloading order is suddenly reversed.

I'm just guessing from the code you posted.
Comment 5 WiseLord 2012-12-11 18:15:18 UTC
I have the same problem.
It seems new torrent is placed to the end of "real queue", but GUI of queue (Queue Manager) shows the situation incorrectly (in inverted order). And this "real queue" works correctly until it receive signal from Queue Manager (when rearranging torrents in it).
Comment 6 Joris Guisson 2012-12-22 12:25:31 UTC
Git commit a80d92a76f41c5e70291561ef7382aa253e1be0b by Joris Guisson.
Committed on 22/12/2012 at 13:24.
Pushed by guisson into branch 'master'.

Fix queue order not reflecting reality in some circumstances

M  +1    -0    ChangeLog
M  +23   -24   ktorrent/tools/queuemanagermodel.cpp
M  +1    -0    ktorrent/tools/queuemanagermodel.h

http://commits.kde.org/ktorrent/a80d92a76f41c5e70291561ef7382aa253e1be0b
Comment 7 john Terragon 2013-01-28 23:03:21 UTC
I'm sorry but it doesn't seems to be completely fixed in 4.3.1.
I didn't yet try all the cases but at least in the following it still seems to behave in the wrong way:

say your queue only allow one active torrent at a time.

1) add a torrent via magnet link -> it goes on pos 1 in the queue and it starts
2) then add a torrent via torrent file -> it goes on pos 1 in the queue but it doesn't replace the 
    previous one as the active torrent 

So, if the meaing of the "pos" field in the queue is the usual one (i.e. the torrent that occupies it is the one active) then
a) either ktorrent puts the second torrent in pos 1 and then it starts it (it would not be a queue but anyways)
b) or ktorrent puts the second torrenti in pos 2 and doesn't start it (the correct semantic for a queue)

Neither one of these is happening.
Comment 8 john Terragon 2013-01-29 06:10:56 UTC
Now I'm starting to think that the fix that  Joris Guisson pushed didn't make it into 4.3.1 because the queue is behaving exactly as before.
A more complete case:

1) you add, say, 5 torrents to the queue, each one via magnet link (the same would happen with torrent files I think)
2) they end up in reverse order in the queue: i.e. the one that was queued as first is in pos 5, the one that was queued as second is in position 4 ecc
3) if the queue remains untouched the downloads would proceed as follows: the torrent in pos 5 is downloaded first, then the one in pos 4 ecc. So up until now the only problem would be that
the queue does not respect the usual meaining for "position number one, number two" ecc 
4) however, if you start moving torrents in the queue (using the arrows in the GUI) things get messy again. For example, let's say that you swap the torrents in position 3 and 2 while the one in pos 5 is still active (and the only downloading). If the problem with the queue was just that positions are inverted wrt the usual meaning, then after the swap we would just have the torrents in pos 3 and 2 swapped and the one in pos 5 would still be the active one. Instead, what does happen is that the torrents in pos 3 and 2 are indeed swapped but the torrent in pos 1 (the last one added) suddenly becomes the active one.
Comment 9 john Terragon 2013-01-29 06:12:48 UTC
Now I'm starting to think that the fix that  Joris Guisson pushed didn't make it into 4.3.1 because the queue is behaving exactly as before.
A more complete case:

1) you add, say, 5 torrents to the queue, each one via magnet link (the same would happen with torrent files I think)
2) they end up in reverse order in the queue: i.e. the one that was queued as first is in pos 5, the one that was queued as second is in position 4 ecc
3) if the queue remains untouched the downloads would proceed as follows: the torrent in pos 5 is downloaded first, then the one in pos 4 ecc. So up until now the only problem would be that
the queue does not respect the usual meaining for "position number one, number two" ecc 
4) however, if you start moving torrents in the queue (using the arrows in the GUI) things get messy again. For example, let's say that you swap the torrents in position 3 and 2 while the one in pos 5 is still active (and the only downloading). If the problem with the queue was just that positions are inverted wrt the usual meaning, then after the swap we would just have the torrents in pos 3 and 2 swapped and the one in pos 5 would still be the active one. Instead, what does happen is that the torrents in pos 3 and 2 are indeed swapped but the torrent in pos 1 (the last one added) suddenly becomes the active one.
Comment 10 Joris Guisson 2013-02-04 17:30:20 UTC
The commit only happened on the master branch, so the fix is not part of 4.3.1