Opened 12 years ago

Closed 10 years ago

#1869 closed Enhancement (fixed)

New status for torrents that reached the seed ratio

Reported by: KyleK Owned by: livings124
Priority: Normal Milestone: 2.00
Component: Transmission Version: 1.52
Severity: Normal Keywords:
Cc:

Description

This applies to the trunk version, which has the new seed ratio functionality implemented.

Currently, torrents get paused after they reach the seed ratio. In the web client, the progress bar is geyed out and the status is set to "Pause". This can be confusing, as you don't know why the torrent is paused (Did I pause it manually? Did Transmission pause it because it ran out of disk space, or can't write to the destination folder?)

I propose a new status "Finished" for this particular case.

Attachments (10)

status_patch_v1.patch (4.2 KB) - added by KyleK 12 years ago.
First version
statusFinishedRemote.diff (4.4 KB) - added by Longinus00 11 years ago.
Grabbing the session when calling "--info" has the secondary benefit of letting us return true values for "ratio limit" and "upload/download limit" if they are set to global
statusFinishedGtk.diff (3.4 KB) - added by Longinus00 11 years ago.
statusFinishedWeb.diff (800 bytes) - added by Longinus00 11 years ago.
statusFinishedWeb2.patch (808 bytes) - added by Longinus00 11 years ago.
deal with TR_RATIO_NA
statusFinishedGtk2.patch (2.4 KB) - added by Longinus00 11 years ago.
realized that there is percentRatio which lets me avoid all this gruntwork
finished.diff (10.0 KB) - added by livings124 11 years ago.
For your consideration
0001-add-finished-string-to-remote.patch (1.2 KB) - added by Longinus00 11 years ago.
format fix
0002-add-finished-to-qt-client.patch (2.5 KB) - added by Longinus00 11 years ago.
format fix
0003-add-finished-to-gtk-client.patch (1.0 KB) - added by Longinus00 11 years ago.
format fix

Download all attachments as: .zip

Change History (98)

comment:1 Changed 12 years ago by KyleK

  • Version changed from 1.50 to 1.50+

Changed 12 years ago by KyleK

First version

comment:2 Changed 12 years ago by KyleK

  • Component changed from Web Client to Transmission
  • Owner Gimp deleted

Here's the first version of a patch for this enhancement. It adds a new torrent state "Finished" to libtransmission, the daemon (remote.c) and the web client.

GTK and the Mac client need to be updated as well. Does the CLI use the status also? I'll look into that tomorrow.

comment:3 Changed 12 years ago by livings124

I don't think this should be a torrent state. It might be better to copy how the mac ui does this - have a boolean for "finished seeding" that is false until the ratio is met. When the transfer is then started, it becomes false again.

comment:4 Changed 12 years ago by KyleK

But that means adding another RPC variable that clients have to evaluate. I wanted to avoid that. What's wrong in it being a state in particular?

comment:5 Changed 12 years ago by livings124

Because that mucks up the meaning of the states.

comment:6 Changed 12 years ago by charles

I haven't thought about this enough to have an opinion on it one way or another, but how would adding a new state muck up the meaning of the current states?

comment:7 Changed 12 years ago by livings124

The states, as I see it, is the actual state the transfer is in (stopped, checking, etc.). "Finished" is more of an extra condition - it's done seeding. If we add more features, there might be more states that might overlap. "Finished" is pretty much only needed for display purposes and doesn't really match with the other states.

comment:8 Changed 12 years ago by charles

  • Resolution set to invalid
  • Status changed from new to closed

I liked this ticket more 6 weeks ago than I do now.

Since nobody's reported any confusion about how a torrent got paused, IMO this change is a solution in search of a problem...

comment:9 Changed 12 years ago by livings124

  • Resolution invalid deleted
  • Status changed from closed to reopened

This makes sense to me still, even if not in the way the patch did it.

comment:10 Changed 12 years ago by KyleK

Tell me how you'd like to implement this and I'll see what I can do.

comment:11 follow-up: Changed 12 years ago by livings124

Check how the Mac client does it in Torrent.[hm] with fFinishedSeeding. I've described it earlier in this ticket.

comment:12 in reply to: ↑ 11 Changed 11 years ago by Longinus00

Replying to livings124:

Check how the Mac client does it in Torrent.[hm] with fFinishedSeeding. I've described it earlier in this ticket.

What you proposed won't work for either transmission-remote or the web interface. The only way I can think of to do this without modifying the torrent state or adding in another rpc call is by doing lots of tests with the two seedRatioLimit variables (one from the session and one from the torrent). This logic is what I use in a script which auto-starts torrents as old ones finish seeding and it seems to work well.

Changed 11 years ago by Longinus00

Grabbing the session when calling "--info" has the secondary benefit of letting us return true values for "ratio limit" and "upload/download limit" if they are set to global

comment:13 Changed 11 years ago by livings124

I don't understand how what I described won't work for those 2 UI's. Just set a flag if it's hit, and adjust text accordingly.

comment:14 Changed 11 years ago by Longinus00

I admit I don't grok ObjC so reading torrent.m isn't very illuminating but it sounded like you were proposing having a variable that only gets tripped when a torrent transitions from one state to another.

comment:15 Changed 11 years ago by livings124

Yup, the bool would be set when it switches to seeding. This should be applicable to all UI's though.

comment:16 Changed 11 years ago by Longinus00

If I'm understanding you correctly, this would be pretty much impossible to do with the remote because it will never "see" a state transition. Doing this with the web interface is also not a good idea because you're not guaranteed to always be connected and thus see state transitions.

Changed 11 years ago by Longinus00

Changed 11 years ago by Longinus00

comment:17 Changed 11 years ago by livings124

The transition isn't as important. When you get status, one of the things could be "paused - finished seeding" instead of just "paused", for example.

comment:18 Changed 11 years ago by Longinus00

I think we're going in circles here so could you look at the patches I submitted and let me know if they are what you had in mind? They all have the same logic but the Web one is by far the most clear cut. The gtk code could be refactored some but I want to know if this logic was what you had in mind.

comment:19 follow-up: Changed 11 years ago by KyleK

I've applied both the daemon and the webUI patch. While the former works fine, the latter will never show the "Finished" state because of how libtransmission deals with seed ratios, see source:/trunk/libtransmission/torrent.c#L2609.

Once a torrent has reached its set seed ratio, the mode is changed to unlimited so that when you restart the torrent, it won't immediately pause again because of the ratio.

charles: Maybe this step should be moved somewhere else, e.g. when you restart a finished torrent?

transmission-remote "works" because it ignores the seedratio_unlimited mode altogether, which is not ideal either.

comment:20 Changed 11 years ago by livings124

KyleK, perhaps the behavior of the patch should be changed then. Did you ever look at how the Mac UI does it, as I asked earlier?

comment:21 Changed 11 years ago by KyleK

I looked, but I don't understand Obj-C at all, so it didn't reveal anything to me :)

As Longinus mentioned before though, I don't think the Mac implementation is suitable at all for either the daemon or the WebUI. The Mac client knows directly about the state of its torrents. As soon as a file completes it can set a flag that states "I'm finished seeding". The other clients, which rely on RPC communication alone, cannot do this. They have to work with what the RPC provides for information.

comment:22 Changed 11 years ago by KyleK

I just looked at the Mac's code again (it's really not that hard to read, I must've been drunk, or tired :), and it is just as I thought: the Mac client's Torrent object has a flag fFinishedSeeding that is initially set to false and only set to true when the HitSeedRatioCallback? is fired. It's a simple state change, which, unfortunately, is not possible with the other 2 clients, since those do not have a persistent connection to the actually Transmission session.

I see 2 options here:

  1. a torrent's seeding mode is not set to UNLIMITED once it's reached its seed ratio (rather set it when it's restarted)
  2. an additional RPC parameter that states whether the seed ratio was met or not.

comment:23 Changed 11 years ago by livings124

Number 2 option would be fairly trivial, no?

comment:24 Changed 11 years ago by KyleK

Yes, I would think so. If charles agrees that it is the best approach I'll provide a patch.

comment:25 in reply to: ↑ 19 Changed 11 years ago by Longinus00

Replying to KyleK:

I've applied both the daemon and the webUI patch. While the former works fine, the latter will never show the "Finished" state because of how libtransmission deals with seed ratios, see source:/trunk/libtransmission/torrent.c#L2609.

I'm confused, I'm seeing finished status fine in my web client. This is because it treats any torrent with seed limit unlimited as 'finished'. What is the seed ratio of the torrent and what is the seed ratio limit? Did you remember to copy the changed files to where transmission serves the web interface from?

comment:26 Changed 11 years ago by Longinus00

To clarify the logic that should be happening, seedRatioLimit returns -1 if the torrent has a seedRatioMode set to unlimited. Therefore it will always be less than the ratio returned by upload_ratio unless the ratio is INF in which case uploadRatio = -2 or if nothing is downloaded or uploaded, which returns -1. This actually made me realize a bug in the uploadRatio as implemented by the javascript, an uploadRatio of -1 is shown to the user if load a fresh torrent (so you haven't uploaded/downloaded anything yet) and deselect all the files. Should probably be calling uploadRatio from a function that returns 0 on uploadRatio = -1.

Changed 11 years ago by Longinus00

deal with TR_RATIO_NA

Changed 11 years ago by Longinus00

realized that there is percentRatio which lets me avoid all this gruntwork

comment:27 Changed 11 years ago by Longinus00

Hmm, now that I've discovered percentRatio I guess there's no reason for the other rpc based clients not to use that instead. If it's agreed, I can rework my previous patches to use percentRatio and update the qt client as well.

Livings, percentRatio might be better than that you're currently using for the mac because it's always calculated on a call to tr_torrentStat anyway and is persistent even through restarts of the client (unless the mac already does this?).

comment:28 Changed 11 years ago by Longinus00

Nm, you're already using it. :p

comment:29 Changed 11 years ago by KyleK

My bad. I had modified the code a bit (adding a test for this.seedRatio != -1), which produced the error.

With the new code it works fine. I would suggest moving that test (whether it's finished or not) to a separate function and just evaluate the return value in the case statement.

comment:30 Changed 11 years ago by livings124

  • Owner set to livings124
  • Status changed from reopened to new

I am writing code in libT to add a new variable for finished torrents. A torrent is considered finished if the ratio limit is met and the torrent has no data left to download (and as a result, would be paused). This behavior is different than the Mac client's current behavior. I have also removed the apparently un-liked code that changed the seed ratio setting to unlimited when finished. The Mac client will, as a result, automatically change this to unlimited when manually resuming the torrent.

Changed 11 years ago by livings124

For your consideration

comment:31 follow-up: Changed 11 years ago by Longinus00

Livings, looked over your code and noticed two things:

  1. It seems to me that s->finished is effectively the same as s->percentRatio == 1.
  2. The logic for changing the seed ratio to inf when starting finished torrents should probably be put in torrentStart(). Otherwise, it's impossible for the webui to restart torrents which were finished.

comment:32 follow-up: Changed 11 years ago by livings124

Longinus00:

  1. Similar, but it also looks at if it's seeding. What else would you expect it to be?
  2. Probably so. I'll experiment.

comment:33 in reply to: ↑ 32 Changed 11 years ago by Longinus00

Replying to livings124:

Longinus00:

  1. Similar, but it also looks at if it's seeding. What else would you expect it to be?
  2. Probably so. I'll experiment.
  1. I wasn't being very clear but what I was getting at is instead of introducing a new variable we could just expose percentRatio over rpc. I suppose you don't want torrents with no stop ratio to show as being finished so that would have to be dealt with.
  2. The webui doesn't have the ability to change torrent options so it won't be able to change the stop ratio.

comment:34 Changed 11 years ago by livings124

  1. Fair enough. Of course eventually that will be changeable over the web client. Having this as a separate value ensures a consistent display across all the interfaces and avoids the repetitive calculation in each ui. Either way works, but my thought of making this a variable is that future functionality might be introduced that makes a torrent "finished".
  1. I concur (although we do plan to add that, but still, it does make sense in that function).

comment:35 in reply to: ↑ 31 Changed 11 years ago by charles

Replying to Longinus00:

Livings, looked over your code and noticed two things:

  1. It seems to me that s->finished is effectively the same as s->percentRatio == 1.
  2. The logic for changing the seed ratio to inf when starting finished torrents should probably be put in torrentStart(). Otherwise, it's impossible for the webui to restart torrents which were finished.
  1. Between these two choices, I'd rather have the explicit flag. That way client code won't have to compare floating-point numbers to test for a state. This is a minor point either way though... they do seem to be equivalent, so I'm bikeshedding here.
  1. This is a good idea. IMO this aspect is a bugfix, not just a new feature.

comment:36 Changed 11 years ago by livings124

  • Priority changed from Low to Normal
  • Version changed from 1.51+ to 1.52

comment:37 Changed 11 years ago by livings124

  • Milestone changed from None Set to 2.00

comment:38 Changed 11 years ago by livings124

r10437 implements this for libT, RPC, the web UI, and the Mac client

comment:39 follow-up: Changed 11 years ago by Longinus00

Why did you delete those two lines from rpc-spec.txt?

comment:40 Changed 11 years ago by mechanic

And it would be nice for transmission-remote to display the "finished" state.

comment:41 in reply to: ↑ 39 ; follow-up: Changed 11 years ago by livings124

Replying to Longinus00:

Why did you delete those two lines from rpc-spec.txt?

They were the same as the two lines above them.

comment:42 in reply to: ↑ 41 ; follow-up: Changed 11 years ago by Longinus00

Replying to livings124:

Replying to Longinus00:

Why did you delete those two lines from rpc-spec.txt?

They were the same as the two lines above them.

The two lines weren't the same, the had session-get instead of session-set. If you think that is redundant then maybe line 585 "rename-partial-files" should also be deleted?

comment:43 in reply to: ↑ 42 Changed 11 years ago by livings124

Replying to Longinus00:

Replying to livings124:

Replying to Longinus00:

Why did you delete those two lines from rpc-spec.txt?

They were the same as the two lines above them.

The two lines weren't the same, the had session-get instead of session-set. If you think that is redundant then maybe line 585 "rename-partial-files" should also be deleted?

Ah, you're right. Restored in r10450

Changed 11 years ago by Longinus00

format fix

Changed 11 years ago by Longinus00

format fix

Changed 11 years ago by Longinus00

format fix

comment:44 Changed 11 years ago by charles

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

Longinus00's transmission-remote, Qt, and GTK+ patches applied in r10451 for 2.00

comment:45 Changed 11 years ago by leena

  • Resolution fixed deleted
  • Status changed from closed to reopened

Apparently the bug from ticket:3127 should be a part of this ticket

Edit: Summary from that ticket:

Every so often a torrent will reach the desired ratio (as set by transmissition-remote -t X -sr Y) and complete, and it then it switches over to seeding forever. This started after I updated to a build that incorporated ticket:1869. I have attached the relevant debug log.

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

comment:46 follow-up: Changed 11 years ago by Longinus00

leena, are you running any queue/auto start scripts or anything?

comment:47 in reply to: ↑ 46 Changed 11 years ago by leena

Replying to Longinus00:

leena, are you running any queue/auto start scripts or anything?

No, I have queuing disabled. This does not happen consistently, maybe 1 out of 10 torrents for me.

comment:48 follow-up: Changed 11 years ago by livings124

leena: What is the exact build number of the version that does this? When it changes to "seed forever", is the torrent stop, does it never stop, or does it stop and restart?

comment:49 in reply to: ↑ 48 Changed 11 years ago by leena

Replying to livings124:

leena: What is the exact build number of the version that does this?

Version: 1.92+ (10460)

When it changes to "seed forever", is the torrent stop, does it never stop, or does it stop and restart?

I haven't seen the change happen in person and even if I was looking at Transmission when it happened, the change would probably have happened too quick to see. I thought the debug log would be enough. If not, perhaps more debug statements could be added to help track this down.

comment:50 follow-up: Changed 11 years ago by livings124

leena: So when you see the torrent, is it paused or active?

comment:51 in reply to: ↑ 50 Changed 11 years ago by leena

Replying to livings124:

leena: So when you see the torrent, is it paused or active?

Active.

comment:52 follow-up: Changed 11 years ago by livings124

This didn't happen in 1.92? The only change here was were the ratio was set to "unlimited".

comment:53 in reply to: ↑ 52 Changed 11 years ago by leena

Replying to livings124:

This didn't happen in 1.92? The only change here was were the ratio was set to "unlimited".

No, before it always went inactive when completed, even when the ratio was set to 0.00.

comment:54 Changed 11 years ago by er13

[comment spam deleted]

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

comment:55 follow-up: Changed 11 years ago by livings124

leena: Give r10473 or later a try.

comment:56 in reply to: ↑ 55 Changed 11 years ago by leena

Replying to livings124:

leena: Give r10473 or later a try.

Just tried r10473 on 7 torrents. Set the ratio to 0.00 for my test. 1 of the torrents encountered the problem, so it doesn't appear that was the solution.

comment:57 follow-up: Changed 11 years ago by livings124

So you set them all to a ratio of 0.00 when they are active? Do they all instantly pause, or all but one. Do all their ratio settings stay the same (0.00)?

comment:58 in reply to: ↑ 57 Changed 11 years ago by leena

Replying to livings124:

So you set them all to a ratio of 0.00 when they are active?

Yes.

Do they all instantly pause, or all but one.

All but the one.

Do all their ratio settings stay the same (0.00)?

All but the one- it's ratio get's changed to 'seed forever'.

comment:59 follow-up: Changed 11 years ago by livings124

leena: This sounds an awful lot like the queue. Are you sure that you have both queues disabled in Preferences -> Transfers -> Management?

comment:60 in reply to: ↑ 59 Changed 11 years ago by leena

Replying to livings124:

leena: This sounds an awful lot like the queue. Are you sure that you have both queues disabled in Preferences -> Transfers -> Management?

YES. The only item set on that preference panel is the default seed ratio- which I have set to 1.0.

comment:61 Changed 11 years ago by charles

Does this problem persist in >= r10478?

comment:62 Changed 11 years ago by leena

Yes, the first torrent I tried after upgrading to r10478 encountered the problem.

Last edited 11 years ago by leena (previous) (diff)

comment:63 follow-up: Changed 11 years ago by charles

I can't reproduce this in the GTK+ client. I tried setting the seed ratio of 200 seeding torrents to 0 and all of them stopped.

comment:64 in reply to: ↑ 63 Changed 11 years ago by leena

Replying to charles:

I can't reproduce this in the GTK+ client. I tried setting the seed ratio of 200 seeding torrents to 0 and all of them stopped.

That's not exactly an equivalent test. I'm setting the seed ratio right after they are added, so they are downloading and not just seeding as in your case. But I am using the Mac client, so it may be specific to the client.

comment:65 Changed 11 years ago by livings124

leena: The transfers that become unlimited are downloading (still blue bar), not just seeding (green bar)?

comment:66 Changed 11 years ago by leena

This is what I am currently doing: add torrent, it starts downloading, I then change the ratio to 0.0, it finishes downloading, the next I know the ratio has somehow changed to 'seed forever'. Whatever happens between the last two steps occurs too quickly to notice. If you add some debug statements, I'd be happy to provide the output.

comment:67 Changed 11 years ago by charles

r10480 /trunk/ (3 files in 2 dirs): (trunk) #1869 "new status for torrents that reached the seed ratio" -- add temporary debug messages to help track down the issue leena's reporting in that ticket

leena: there you go... :)

comment:68 Changed 11 years ago by leena

Thanks, but it (r10481) fails to build:

/Users/leena/build/transmission/macosx/Torrent.m:289:0 /Users/leena/build/transmission/macosx/Torrent.m:289: error: expected expression before 'tr_info'

/Users/leena/build/transmission/macosx/Torrent.m:289:0 /Users/leena/build/transmission/macosx/Torrent.m:289: error: too few arguments to function 'tr_msg'

/Users/leena/build/transmission/macosx/Torrent.m:321:0 /Users/leena/build/transmission/macosx/Torrent.m:321: error: expected expression before 'tr_info'

/Users/leena/build/transmission/macosx/Torrent.m:321:0 /Users/leena/build/transmission/macosx/Torrent.m:321: error: too few arguments to function 'tr_msg'
Last edited 11 years ago by leena (previous) (diff)

comment:69 Changed 11 years ago by leena

Attached the log for a random pdf. Something is confused as it is claiming 'Restarted manually -- disabling its seed ratio'.

comment:70 Changed 11 years ago by livings124

leena: Does this only occur in cases where the data is being moved from the incomplete folder?

comment:71 Changed 11 years ago by er13

  • Cc ernews@… added

comment:72 Changed 11 years ago by er13

  • Cc ernews@… removed

comment:73 Changed 11 years ago by leena

Yes, I have not seen the problem since disabling the incomplete folder.

comment:74 Changed 11 years ago by charles

I don't see the "Seed ratio reached; pausing torrent" message in that log. Did you edit that out?

comment:75 Changed 11 years ago by leena

I had about 30 other large torrents that had been paused, but were filling up the debug log with messages. Apparently a paused torrent is still very chatty. So I filtered by torrent name (process column). I just reran the experiment after deleting all the other torrents and didn't filter the results this time- the 'Seed ratio reached' message was not present in the log.

comment:76 Changed 11 years ago by charles

leena: r10483 may fix this issue. could you give it a try and report back?

comment:77 Changed 11 years ago by leena

I tried r10483, but still encountering the problem. And I just noticed that after r10478 I encounter the problem for every torrent, instead of the 1 out of 10 before that change.

comment:78 Changed 11 years ago by livings124

#3150 is a duplicate of this

comment:79 Changed 11 years ago by KyleK

I just noticed the same behavior as leena describes myself. The problem seems to occur only when:

  1. A torrent is finished before reaching its seed ratio A torrent has already reached its seed ratio when it finishes
  2. And when you have set an Incomplete folder.

When moving the completed data, Transmission calls torrentStop() in case the torrent is still running, then moves the data, and then calls torrentStart(). torrentStart() now thinks that someone "manually" restarted the torrent, and sets the seed ratio to TR_RATIOLIMIT_UNLIMITED.

Maybe this could be resolved by introducing two new (internal) methods torrentPause() and torrentResume() that behave similarily to the existing ones but ignore stuff like seed ratio.

Last edited 11 years ago by KyleK (previous) (diff)

comment:80 Changed 11 years ago by charles

01:35:47 < CIA-40> charles * r10551 libtransmission/torrent.c: (trunk libT) #1869 "New status for torrents that reached the seed ratio" -- maybe fix the bug where the incomplete-dir and seed-ratio features conflicted with each other as reported by leena in http://trac.transmissionbt.com/ticket/1869#comment:45

comment:81 Changed 11 years ago by charles

leena, KyleK: does this change fix the problem for you?

comment:82 Changed 11 years ago by kovalev

A compiler warning which might have been introduced in libtransmission since r10550: torrent.c:944: warning: no previous declaration for ‘tr_torrentGetActivity’

comment:83 Changed 11 years ago by charles

That's fixed again, but regardless, it shouldn't affect testing the change to see if it fixes the seed ratio bug...

comment:84 Changed 11 years ago by KyleK

I just tested with build r10551 and this issue seems to be fixed.

comment:85 Changed 11 years ago by charles

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

Yay! Thanks for the heads-up.

comment:86 Changed 11 years ago by charles

00:26:29 < CIA-40> charles * r10647 libtransmission/torrent.c: (trunk libT) #1869 "new status for torrents that reached the seed ratio" -- use suggestion from Longinus00 to ensure torrents don't get flagged as finished if the user hits "download none" in the "add torrent" dialog

xref: http://trac.transmissionbt.com/ticket/1796#comment:42

comment:87 Changed 10 years ago by Tantali

  • Resolution fixed deleted
  • Status changed from closed to reopened

The problem that 'seed forever' hops in when the seed ratio is met, is back. This occurs in r10990. See my log (I hope I uploaded it alright).

comment:88 Changed 10 years ago by livings124

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

It appears that you are downloading pirated content. We refuse to aid people using the application in such a way. If users using the application for legitimate purposes have issues, please reopen the ticket.

Note: See TracTickets for help on using tickets.