Opened 12 years ago

Closed 12 years ago

#2789 closed Enhancement (fixed)

Add a trackers tab to the inspector

Reported by: mjpieters Owned by: kjg
Priority: Normal Milestone: 1.90
Component: Web Client Version: 1.80
Severity: Normal Keywords: patch
Cc:

Description

I'd like to see a trackers tab in the Web UI inspector, listing at least the tiers, the host, and seeders, leechers and downloads stats.

I've created a quick mockup of what that would look like, as a patch to the existing trunk web ui. Attachments for this ticket include that patch and a screen shot.

  • The tracker button (info_trackers.png) is simply a copy of macosx/Images/InfoTracker.png
  • The actual contents of the new inspector container are hardcoded for now

Attachments (7)

trackers_tab.diff (2.4 KB) - added by mjpieters 12 years ago.
Mockup patch
webui.png (34.5 KB) - added by mjpieters 12 years ago.
Screenshot of the mockup with tooltip.
trackers_tab.2.diff (7.3 KB) - added by mjpieters 12 years ago.
Full implementation
trackers_tab.3.diff (7.1 KB) - added by mjpieters 12 years ago.
Cleaned up version of full implemtation patch.
tracker_tab.4.patch (6.9 KB) - added by KyleK 12 years ago.
tracker_tabs.5.patch (7.9 KB) - added by KyleK 12 years ago.
tracker_tab.patch (9.8 KB) - added by mjpieters 12 years ago.
Trackers inspector tab; load tracker stats only for selected tabs

Download all attachments as: .zip

Change History (31)

Changed 12 years ago by mjpieters

Mockup patch

Changed 12 years ago by mjpieters

Screenshot of the mockup with tooltip.

comment:1 Changed 12 years ago by mjpieters

  • Keywords patch added

I've implemented the full trackers inspector now, with more than one tracker per tier listed in rows similar to the files tab. See attached patch.

Changed 12 years ago by mjpieters

Full implementation

Changed 12 years ago by mjpieters

Cleaned up version of full implemtation patch.

comment:2 Changed 12 years ago by mjpieters

Not worth another revision of the patch, but 'Seeds' should of course read 'Seeders'.

comment:3 follow-up: Changed 12 years ago by KyleK

I've modified your patch a bit, so it only loads tracker stats for selected torrents. This should reduce the data overhead a bit, especially when you have many torrents loaded into Transmission.

Please have a thorough look at this, my JS skills are virtually non-existant :)

Changed 12 years ago by KyleK

comment:4 in reply to: ↑ 3 Changed 12 years ago by mjpieters

Replying to KyleK:

I've modified your patch a bit, so it only loads tracker stats for selected torrents. This should reduce the data overhead a bit, especially when you have many torrents loaded into Transmission.

Please have a thorough look at this, my JS skills are virtually non-existant :)

I'd say that if you were to load the tracker stats separately you'd incur more data overhead as it'd have to do an extra request for each and every torrent you look at. Tracker stats simply aren't that much data to go through that much effort.

You also changed when the tracker panel UI is rendered. I don't think the added complexity is necessary though. To me, it makes the code harder to read and maintain, and it only buys a few cycles when scrolling through the list of torrents. I've deliberately made the DOM generating code as fast as possible, and we are talking about a few trackers per torrent only here, not the potentially huge lists of filenames the files panel has to deal with. Is it really worth the complexity overhead?

Last but not least, your patch doesn't actually load any tracker stats; lines 1319 through to 1322 don't make sense to me. I don't see remote.loadTrackerStats defined, and the refresh_files_for data structure only applies to the files panel; it has nothing to do with the tracker panel.

comment:5 Changed 12 years ago by KyleK

For some reason my previous patch didn't include the changes to transmission.remote.js, which contains the function loadTrackerStats(). I basically copied the method loadTorrentFiles().

The function updateTorrentsData() in transmission.js, line 1304 determines which torrents are selected in the webUI, adds their IDs to a list (refresh_files_for), and then fires up tr.remote.loadTorrentFiles(). It doesn't make much sense to create a 2nd list just for retrieving tracker stats, so I pass the same list of IDs to tr.remote.loadTrackerStats(). With the updated patch this should make more sense :)

I still believe this is the more efficient way, albeit more complex. I know there are people out there which actually load 100 (!) torrents into Transmission. I just did a quick test: the trackerStats object containing a single tracker is ~600 byte in size. Many torrents have more than that, and the size scale is linear. Requesting 600*n bytes for each active torrent every 5 seconds is quite an overhead, especially if most of the time you don't even see the data.

I agree though there is something funky with the displaying code. After I selected a torrent, and then the tracker tab, it'll still take a while to load it with data. I'm not sure why that is. Then again, I'm no JS whizz :)

Changed 12 years ago by KyleK

comment:6 Changed 12 years ago by mjpieters

I'll take a look at your patch tomorrow; I've been travelling all day.

I'll then also look into how much data has been added to the JSON requests; I have a Transmission instance with 750+ torrents with my patch already applied, and I haven't noticed any slowdowns with web UI yet!

My initial thoughts: until Transmission 1.80, the Web UI was already including tracker stats for every torrent, albeit a smaller data set; I can't say that the absence of those bytes was noticeable on my 750+ torrent UI, nor is the addition of the more detailed tracker info with my patch applied.

And in HTTP performance, reducing the number of request to a server can be as important as reducing data sets; introducing an extra request every 5 seconds is a lot of overhead too!

I'd love to hear feedback from kjg on this issue, btw.

comment:7 Changed 12 years ago by KyleK

The additional data may not produce any slowdowns on a normal PC, but consider also that people use Transmissions web interface on their iPhones.

Also, with regard to mobile data plans, which are often limited to a couple of hundred MB per month (I'm on a 200MB/month plan), the increased use of data might be an issue.

comment:8 Changed 12 years ago by mjpieters

I've studied the code some more, and now understand the RPC protocol far better.

The web UI only asks for updates for recently changed torrents, not for all torrents tracked by the client. This means that the data package every 5 seconds is a lot smaller; especially when only seeding, the amount of data sent during these poll requests is really very small, only about 7 KB.

However, with my patch applied, there are more updates to report on and the poll package size goes up to around 25-30 KB; presumably because there is more activity to be reported on.

So, in that light, your direction makes sense; loading only the 0.6 KB of tracker data for currently selected torrents would save a lot of bandwidth. I'll rework your code though; the loadTorrentFiles can be renamed and updated to include the trackerData updates in one request, for example.

Changed 12 years ago by mjpieters

Trackers inspector tab; load tracker stats only for selected tabs

comment:9 Changed 12 years ago by mjpieters

I've now started to manage this patch in a mercurial patch queue; you can always see the latest version at:

http://bitbucket.org/mjpieters/transmission-mq/src/tip/tracker_tab.patch

This means that images are included as a GIT binary patch in the diff.

This version takes Kylek's changes into account; I've co-opted the file panel update mechanism to update tracker stats as well. To reflect this, I've renamed the methods involved from TrackerFiles? to TrackerDetails?. This all means that torrent stats are only updated for the currently selected torrent (meaning that switching to another torrent will show outdated data until the next poll, same as for files), and the update is done with the one torrent-get request.

comment:10 Changed 12 years ago by mjpieters

I tested this on my iPhone as well, to discover that the CSS for that needs to be updated too. That's a task for later this week.

comment:11 Changed 12 years ago by mjpieters

After discussing this issue with kjg in IRC today, I've decided to go back to the recently-active method of obtaining the tracker info. When using the only-update-the-selected torrent method, you easily end up with a tracker panel with stale data on it; any non-selected torrent where the tracker stats change will not receive the updated data.

I did change the panel rendering to only take place if the tracker panel is actually visible.

Also, I've added styling for the iPhone version, and verified that the styling still works in IE7 as well.

I've pushed a new version to bitbucket:

http://bitbucket.org/mjpieters/transmission-mq/src/tip/tracker_tab.patch

Unless further feedback brings something up, I consider this patch complete.

comment:12 follow-up: Changed 12 years ago by kjg

added in r10050. How do you feel about adding code to display the Last announce, next announce and last scape info, to bring it fully up to part with the other UIs? :-)

comment:13 Changed 12 years ago by kjg

  • Milestone changed from None Set to 1.90

comment:14 in reply to: ↑ 12 ; follow-up: Changed 12 years ago by mjpieters

Replying to kjg:

added in r10050. How do you feel about adding code to display the Last announce, next announce and last scape info, to bring it fully up to part with the other UIs? :-)

Done, and added the tooltip announce url feature as well.. ;-)

Get the patch at:

http://bitbucket.org/mjpieters/transmission-mq/src/tip/pending/tracker_announce_scrape_details.patch

I am not that happy with the table layout with the extra info in it though; it tends to jump around with different widths. Perhaps you can adjust that a little.

comment:15 follow-up: Changed 12 years ago by KyleK

I still consider your approach of receiving tracker data for all torrents at every refresh bad design, especially since the template for selective fetching is already there. I don't see a problem with waiting a couple of seconds for the tracker data to update, even more so since the data is purely informational (it doesn't matter of there are 5 seeders or 7 when you click the torrent. You can't change anything about it anyway).

comment:16 in reply to: ↑ 15 Changed 12 years ago by mjpieters

Replying to KyleK:

I still consider your approach of receiving tracker data for all torrents at every refresh bad design, especially since the template for selective fetching is already there.

It doesn't ever receive tracker data for all torrents! Only data changed since last time is received. That's what the 'recenly-active' method is all about.

Your proposal was to only load tracker data for the currently selected torrent, which lead to information skew: any updates to torrents not selected where quickly outdated, with no way to force a refresh of that display.

Note that you can alter the refresh interval (how often to ask the server about changes) if you are concerned about data transfer limits.

comment:17 in reply to: ↑ 14 ; follow-up: Changed 12 years ago by kjg

Replying to mjpieters:

Done, and added the tooltip announce url feature as well.. ;-)

I am not that happy with the table layout with the extra info in it though; it tends to jump around with different widths. Perhaps you can adjust that a little.

Committed in r10061.

So now just one more thing, and this should make KyleK happy.

The daemon only considers a torrent "Recently Updated" when it has received or set pieces (more or less). It does not consider a torrent "Recently Updated" if the torrent's only activity has been changes to trackerStats.

Therefor to ensure that the trackerStats get updated even when the torrent hasn't receive or sent data for awhile, it'll need to be queried for separately and thus -- as mentioned above -- you should only do this for the torrent when the tracker tab is selected.

comment:18 in reply to: ↑ 17 Changed 12 years ago by mjpieters

Replying to kjg:

Committed in r10061.

Thanks. Can I suggest a style change of my own? :-) All the break statements are redundant if the announce status string is simply returned immediately:

http://bitbucket.org/mjpieters/transmission-mq/src/tip/pending/announceState_codestyle.patch

So now just one more thing, and this should make KyleK happy.

The daemon only considers a torrent "Recently Updated" when it has received or set pieces (more or less). It does not consider a torrent "Recently Updated" if the torrent's only activity has been changes to trackerStats.

Therefor to ensure that the trackerStats get updated even when the torrent hasn't receive or sent data for awhile, it'll need to be queried for separately and thus -- as mentioned above -- you should only do this for the torrent when the tracker tab is selected.

Ah. Okay. And my code does not do that. I indeed see plenty of torrents with 'next announce in 0 seconds' states. How could be best go about that? Timestamp the info and update it if it hasn't changed for 30 minutes? Add a refresh button somewhere?

comment:19 Changed 12 years ago by KyleK

Consider me happy :)

comment:20 Changed 12 years ago by charles

kjg: I haven't been following this ticket, but I see it's still milestoned for 1.90 and that there have been several commits on this ticket. Is this going to be done in time for next week?

comment:21 Changed 12 years ago by kjg

All the patches on this ticket have been committed in some form. I would like if mjpieters or KyleK would split the tracker bit out so that it is a separate request and only called on one torrent and a time and only if the tracker tab is selected, but I don't see that as being critical.

comment:22 Changed 12 years ago by charles

if it's requesting all the trackers for all the torrents, especially whether it needs them or not, that's a major problem... that's a repeat of the file list bug that sent performance to hell before. If you've only got a few torrents it doesn't matter, but it's a huge issue when the torrent list grows.

If I'm understanding your summary right, that needs to get fixed or reverted before next week's release... :/

comment:23 Changed 12 years ago by KyleK

If I understand comment:17 and comment:18 correctly, the interface only fetches data for "recently updated" torrents, i.e. torrents that show activity. This in turn would mean that idle torrents won't get their tracker stats updated currently.

I haven't yet looked at the code to confirm this though.

comment:24 Changed 12 years ago by kjg

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

Marking this as fixed as a tracker tab is now implemented. Opening a new ticket for the modification to be made.

Note: See TracTickets for help on using tickets.