Skip to content

Overhaul stats: fix bug in downloads counter #1502

@josecelano

Description

@josecelano

Relates to: 34c159a

A couple of days ago, I made a change in this commit.

I changed the Swarm::meets_retaining_policy method from:

/// Returns true if the torrents meets the retention policy, meaning that
/// it should be kept in the tracker.
pub fn meets_retaining_policy(&self, policy: &TrackerPolicy) -> bool {
    if policy.persistent_torrent_completed_stat && self.metadata().downloaded > 0 {
        return true;
    }

    if policy.remove_peerless_torrents && self.is_empty() {
        return false;
    }

    true
}

To:

pub fn meets_retaining_policy(&self, policy: &TrackerPolicy) -> bool {
    !(policy.remove_peerless_torrents && self.is_empty())
}

I thought this code was not needed:

if policy.persistent_torrent_completed_stat && self.metadata().downloaded > 0 {
    return true;
}

However, it's needed.

One of the metrics returned by the tracker API is the total number of downloads for all torrents.

{
  "torrents": 320961,
  "seeders": 189885,
  "completed": 975119, <- this
  "leechers": 231044,
  ...
}

That metric is always stored in memory but can optionally persist into the database.

It's important to highlight that the metric represents:

  • The total number of downloads for ALL torrents ever, when the metric is persisted.
  • The total number of downloads for ALL torrents since the tracker started, when the metric is not persisted.

It could be mixed up with another internal metric (not exposed via the API), which is the same counter but only for ONE swarm (one torrent).

  • The total number of downloads for ONE concrete torrent ever, when the metric is persisted.
  • The total number of downloads for ONE concrete torrent since the tracker started, when the metric is not persisted.

The bug affects the first metric. The exposed via the API.

The problem is that this feature conflicts with removing the peerless torrents.

When removing the peerless torrents config option is enabled, the counter is lost unless it is persisted. Becuase the counter values are stored in the "Swarm" together with the list of peers.

If statistics persistence is enabled, that's not a problem. When the torrent is removed from the tracker (from the swarms or swarm collection), the counter is initialised again if the torrent is added. In other words, if a new peer starts the swarm again, the number of downloads is loaded from the database.

However, that works for the counter of each torrent (swarm) but not for the overall counter (the sum of downloads for all torrents). That metric is not stored anywhere. It's calculated on demand by iterating all the swarms and summing up the total for each torrent, giving the total amount of downloads for ALL torrents.

When the torrent is removed, the downloads for that torrent don't count in the total. That is the reason we have to keep the torrent (swarm) in memory, even if it does not have any peer (and it should be removed according to the other config flag).

The removed line:

if policy.persistent_torrent_completed_stat && self.metadata().downloaded > 0 {
    return true;
}

does that.

Solution

When the stats persistence is disabled, that's one way to store the value.

Alternatively, we could add another cache for the data and never remove that value.

The current solution has a problem: It can make the tracker consume a lot of memory because peerless torrents are not removed in practice (even if it's configured to be).

When the stats persistence is enabled, we can simply return the value from the database.

NOTICE: that the value is used in the scrape response, so it might be convenient to have a cache in memory anyway.

Subtasks

Metadata

Metadata

Assignees

Labels

- User -Enjoyable to Use our SoftwareBugIncorrect Behavior

Type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions