Skip to content

Commit 7b473e9

Browse files
committed
Merge torrust#1356: Fix bug: number of downloads for a torrent is reset
06ef1d5 fix: [torrust#1264] number of downloads preset when torrent is persisted (Jose Celano) 8f67f12 fix: [torrust#1264] partially. The correct number of downloads is persited (Jose Celano) 6beec3a refactor: [torrust#1264] add a database method to increase the number of downloads for a torrent (Jose Celano) 8a4dba3 refactor: [torrust#1264] rename variables (Jose Celano) 2fb1c6f reafactor: [torrust#1264] add a DB methog to get the number of completed downloads for one torrent (Jose Celano) c5db71a refactor: [torrust#1264] remove unnecessary fn call (Jose Celano) 820329b refactor: [torrust#1264] return wether swarm stats have change or not after upserting a peer (Jose Celano) Pull request description: ### Problem **The number of downloads for a torrent is not persisted correctly**. Even when persistence is enabled, it does not work. There are two cases when the counter is reset: - When you restart the tracker. - When a torrent entry is removed from the torrents repository because the last peer was removed (peerless torrents). This only happens if "removing peerless torrents" is enabled in the configuration. Check the [issue](torrust#1264) to know how to reproduce the bug. ### Solution Every time a new torrent entry is added in the torrent repository we need to load stats (also called persistent torrent, which only includes number of downloads currently) from the database. If persistence for stats is enabled. ### Other considerations **1. Unused code** There is a method to load the counters for all torrents but it's not used in production code. ```rust fn import_persistent(&self, persistent_torrents: &PersistentTorrents) { for (info_hash, completed) in persistent_torrents { if self.torrents.contains_key(info_hash) { continue; } let entry = EntryMutexStd::new( EntrySingle { swarm: PeerList::default(), downloaded: *completed, } .into(), ); self.torrents.insert(*info_hash, entry); } } ``` I guess it was added to load the information from the database when the tracker is restarted. However that's not enough because torrents are removed from the repository when they do not have peers (peerless torrents). That would work only if removing peerless torrents is disabled. **2. Better solution** I think there is a better solution than the one this PR applies. This PR is a temporary patch to fix the bug asap. I would differentiate to different counters: 1. number of downloads per torrent while it was active in the swarm (since the tracker was restarted) 2. historical number of downloads per torrent per web service (since the service was put into production) The counter 1 is what we have before merging this PR. The counter is reset every time the torrent is removed from the swarm. We can keep like it's now before merging this PR. This counter is handled at the in-memory torrent repository. The new counter (counter 2) would be handled by the announce handler. After processing the announce request if the number of downloads has increased the handler also increases the historical counter. This new value should be the one exposed in the API. ```json { "torrents": 379508, "seeders": 150440, "completed": 58992, <- this should be the historical "leechers": 264865, "tcp4_connections_handled": 15, "tcp4_announces_handled": 15, "tcp4_scrapes_handled": 0, "tcp6_connections_handled": 0, "tcp6_announces_handled": 0, "tcp6_scrapes_handled": 0, "udp_requests_aborted": 556458, "udp_requests_banned": 24773536, "udp_banned_ips_total": 29868, "udp_avg_connect_processing_time_ns": 478212, "udp_avg_announce_processing_time_ns": 8724637, "udp_avg_scrape_processing_time_ns": 296950, "udp4_requests": 298728340, "udp4_connections_handled": 104369821, "udp4_announces_handled": 158354656, "udp4_scrapes_handled": 3664492, "udp4_responses": 273954123, "udp4_errors_handled": 7540716, "udp6_requests": 0, "udp6_connections_handled": 0, "udp6_announces_handled": 0, "udp6_scrapes_handled": 0, "udp6_responses": 0, "udp6_errors_handled": 0 } ``` This patches solves the problem but pre-loading the historical data every time the torrent is added to the repository. ACKs for top commit: josecelano: ACK 06ef1d5 Tree-SHA512: 2d1ad14d26efa981d24b1264640b5e2deaca621092ec61638e85e21dc83f92ad53a4041cb97e4e798611f3bfed610c7794b68027e43a847751c8ec022abc6b19
2 parents bf883da + 06ef1d5 commit 7b473e9

File tree

30 files changed

+441
-179
lines changed

30 files changed

+441
-179
lines changed

packages/axum-http-tracker-server/src/environment.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -22,11 +22,11 @@ pub struct Environment<S> {
2222
impl<S> Environment<S> {
2323
/// Add a torrent to the tracker
2424
pub fn add_torrent_peer(&self, info_hash: &InfoHash, peer: &peer::Peer) {
25-
let () = self
25+
let _number_of_downloads_increased = self
2626
.container
2727
.tracker_core_container
2828
.in_memory_torrent_repository
29-
.upsert_peer(info_hash, peer);
29+
.upsert_peer(info_hash, peer, None);
3030
}
3131
}
3232

packages/axum-rest-tracker-api-server/src/environment.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -33,11 +33,11 @@ where
3333
{
3434
/// Add a torrent to the tracker
3535
pub fn add_torrent_peer(&self, info_hash: &InfoHash, peer: &peer::Peer) {
36-
let () = self
36+
let _number_of_downloads_increased = self
3737
.container
3838
.tracker_core_container
3939
.in_memory_torrent_repository
40-
.upsert_peer(info_hash, peer);
40+
.upsert_peer(info_hash, peer, None);
4141
}
4242
}
4343

packages/torrent-repository/benches/helpers/asyn.rs

+6-6
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ where
1818

1919
let info_hash = InfoHash::default();
2020

21-
torrent_repository.upsert_peer(&info_hash, &DEFAULT_PEER).await;
21+
torrent_repository.upsert_peer(&info_hash, &DEFAULT_PEER, None).await;
2222

2323
torrent_repository.get_swarm_metadata(&info_hash).await;
2424
}
@@ -37,7 +37,7 @@ where
3737
let handles = FuturesUnordered::new();
3838

3939
// Add the torrent/peer to the torrent repository
40-
torrent_repository.upsert_peer(&info_hash, &DEFAULT_PEER).await;
40+
torrent_repository.upsert_peer(&info_hash, &DEFAULT_PEER, None).await;
4141

4242
torrent_repository.get_swarm_metadata(&info_hash).await;
4343

@@ -47,7 +47,7 @@ where
4747
let torrent_repository_clone = torrent_repository.clone();
4848

4949
let handle = runtime.spawn(async move {
50-
torrent_repository_clone.upsert_peer(&info_hash, &DEFAULT_PEER).await;
50+
torrent_repository_clone.upsert_peer(&info_hash, &DEFAULT_PEER, None).await;
5151

5252
torrent_repository_clone.get_swarm_metadata(&info_hash).await;
5353

@@ -87,7 +87,7 @@ where
8787
let torrent_repository_clone = torrent_repository.clone();
8888

8989
let handle = runtime.spawn(async move {
90-
torrent_repository_clone.upsert_peer(&info_hash, &DEFAULT_PEER).await;
90+
torrent_repository_clone.upsert_peer(&info_hash, &DEFAULT_PEER, None).await;
9191

9292
torrent_repository_clone.get_swarm_metadata(&info_hash).await;
9393

@@ -123,7 +123,7 @@ where
123123

124124
// Add the torrents/peers to the torrent repository
125125
for info_hash in &info_hashes {
126-
torrent_repository.upsert_peer(info_hash, &DEFAULT_PEER).await;
126+
torrent_repository.upsert_peer(info_hash, &DEFAULT_PEER, None).await;
127127
torrent_repository.get_swarm_metadata(info_hash).await;
128128
}
129129

@@ -133,7 +133,7 @@ where
133133
let torrent_repository_clone = torrent_repository.clone();
134134

135135
let handle = runtime.spawn(async move {
136-
torrent_repository_clone.upsert_peer(&info_hash, &DEFAULT_PEER).await;
136+
torrent_repository_clone.upsert_peer(&info_hash, &DEFAULT_PEER, None).await;
137137
torrent_repository_clone.get_swarm_metadata(&info_hash).await;
138138

139139
if let Some(sleep_time) = sleep {

packages/torrent-repository/benches/helpers/sync.rs

+6-6
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ where
2020

2121
let info_hash = InfoHash::default();
2222

23-
torrent_repository.upsert_peer(&info_hash, &DEFAULT_PEER);
23+
torrent_repository.upsert_peer(&info_hash, &DEFAULT_PEER, None);
2424

2525
torrent_repository.get_swarm_metadata(&info_hash);
2626
}
@@ -39,7 +39,7 @@ where
3939
let handles = FuturesUnordered::new();
4040

4141
// Add the torrent/peer to the torrent repository
42-
torrent_repository.upsert_peer(&info_hash, &DEFAULT_PEER);
42+
torrent_repository.upsert_peer(&info_hash, &DEFAULT_PEER, None);
4343

4444
torrent_repository.get_swarm_metadata(&info_hash);
4545

@@ -49,7 +49,7 @@ where
4949
let torrent_repository_clone = torrent_repository.clone();
5050

5151
let handle = runtime.spawn(async move {
52-
torrent_repository_clone.upsert_peer(&info_hash, &DEFAULT_PEER);
52+
torrent_repository_clone.upsert_peer(&info_hash, &DEFAULT_PEER, None);
5353

5454
torrent_repository_clone.get_swarm_metadata(&info_hash);
5555

@@ -89,7 +89,7 @@ where
8989
let torrent_repository_clone = torrent_repository.clone();
9090

9191
let handle = runtime.spawn(async move {
92-
torrent_repository_clone.upsert_peer(&info_hash, &DEFAULT_PEER);
92+
torrent_repository_clone.upsert_peer(&info_hash, &DEFAULT_PEER, None);
9393

9494
torrent_repository_clone.get_swarm_metadata(&info_hash);
9595

@@ -125,7 +125,7 @@ where
125125

126126
// Add the torrents/peers to the torrent repository
127127
for info_hash in &info_hashes {
128-
torrent_repository.upsert_peer(info_hash, &DEFAULT_PEER);
128+
torrent_repository.upsert_peer(info_hash, &DEFAULT_PEER, None);
129129
torrent_repository.get_swarm_metadata(info_hash);
130130
}
131131

@@ -135,7 +135,7 @@ where
135135
let torrent_repository_clone = torrent_repository.clone();
136136

137137
let handle = runtime.spawn(async move {
138-
torrent_repository_clone.upsert_peer(&info_hash, &DEFAULT_PEER);
138+
torrent_repository_clone.upsert_peer(&info_hash, &DEFAULT_PEER, None);
139139
torrent_repository_clone.get_swarm_metadata(&info_hash);
140140

141141
if let Some(sleep_time) = sleep {

packages/torrent-repository/src/entry/single.rs

+5-3
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ impl Entry for EntrySingle {
5151
}
5252

5353
fn upsert_peer(&mut self, peer: &peer::Peer) -> bool {
54-
let mut downloaded_stats_updated: bool = false;
54+
let mut number_of_downloads_increased: bool = false;
5555

5656
match peer::ReadInfo::get_event(peer) {
5757
AnnounceEvent::Stopped => {
@@ -62,15 +62,17 @@ impl Entry for EntrySingle {
6262
// Don't count if peer was not previously known and not already completed.
6363
if previous.is_some_and(|p| p.event != AnnounceEvent::Completed) {
6464
self.downloaded += 1;
65-
downloaded_stats_updated = true;
65+
number_of_downloads_increased = true;
6666
}
6767
}
6868
_ => {
69+
// `Started` event (first announced event) or
70+
// `None` event (announcements done at regular intervals).
6971
drop(self.swarm.upsert(Arc::new(*peer)));
7072
}
7173
}
7274

73-
downloaded_stats_updated
75+
number_of_downloads_increased
7476
}
7577

7678
fn remove_inactive_peers(&mut self, current_cutoff: DurationSinceUnixEpoch) {

packages/torrent-repository/src/repository/dash_map_mutex_std.rs

+8-4
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ use torrust_tracker_configuration::TrackerPolicy;
66
use torrust_tracker_primitives::pagination::Pagination;
77
use torrust_tracker_primitives::swarm_metadata::SwarmMetadata;
88
use torrust_tracker_primitives::torrent_metrics::TorrentsMetrics;
9-
use torrust_tracker_primitives::{peer, DurationSinceUnixEpoch, PersistentTorrents};
9+
use torrust_tracker_primitives::{peer, DurationSinceUnixEpoch, PersistentTorrent, PersistentTorrents};
1010

1111
use super::Repository;
1212
use crate::entry::peer_list::PeerList;
@@ -23,13 +23,17 @@ where
2323
EntryMutexStd: EntrySync,
2424
EntrySingle: Entry,
2525
{
26-
fn upsert_peer(&self, info_hash: &InfoHash, peer: &peer::Peer) {
26+
fn upsert_peer(&self, info_hash: &InfoHash, peer: &peer::Peer, _opt_persistent_torrent: Option<PersistentTorrent>) -> bool {
27+
// todo: load persistent torrent data if provided
28+
2729
if let Some(entry) = self.torrents.get(info_hash) {
28-
entry.upsert_peer(peer);
30+
entry.upsert_peer(peer)
2931
} else {
3032
let _unused = self.torrents.insert(*info_hash, Arc::default());
3133
if let Some(entry) = self.torrents.get(info_hash) {
32-
entry.upsert_peer(peer);
34+
entry.upsert_peer(peer)
35+
} else {
36+
false
3337
}
3438
}
3539
}

packages/torrent-repository/src/repository/mod.rs

+8-3
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ use torrust_tracker_configuration::TrackerPolicy;
33
use torrust_tracker_primitives::pagination::Pagination;
44
use torrust_tracker_primitives::swarm_metadata::SwarmMetadata;
55
use torrust_tracker_primitives::torrent_metrics::TorrentsMetrics;
6-
use torrust_tracker_primitives::{peer, DurationSinceUnixEpoch, PersistentTorrents};
6+
use torrust_tracker_primitives::{peer, DurationSinceUnixEpoch, PersistentTorrent, PersistentTorrents};
77

88
pub mod dash_map_mutex_std;
99
pub mod rw_lock_std;
@@ -24,7 +24,7 @@ pub trait Repository<T>: Debug + Default + Sized + 'static {
2424
fn remove(&self, key: &InfoHash) -> Option<T>;
2525
fn remove_inactive_peers(&self, current_cutoff: DurationSinceUnixEpoch);
2626
fn remove_peerless_torrents(&self, policy: &TrackerPolicy);
27-
fn upsert_peer(&self, info_hash: &InfoHash, peer: &peer::Peer);
27+
fn upsert_peer(&self, info_hash: &InfoHash, peer: &peer::Peer, opt_persistent_torrent: Option<PersistentTorrent>) -> bool;
2828
fn get_swarm_metadata(&self, info_hash: &InfoHash) -> Option<SwarmMetadata>;
2929
}
3030

@@ -37,6 +37,11 @@ pub trait RepositoryAsync<T>: Debug + Default + Sized + 'static {
3737
fn remove(&self, key: &InfoHash) -> impl std::future::Future<Output = Option<T>> + Send;
3838
fn remove_inactive_peers(&self, current_cutoff: DurationSinceUnixEpoch) -> impl std::future::Future<Output = ()> + Send;
3939
fn remove_peerless_torrents(&self, policy: &TrackerPolicy) -> impl std::future::Future<Output = ()> + Send;
40-
fn upsert_peer(&self, info_hash: &InfoHash, peer: &peer::Peer) -> impl std::future::Future<Output = ()> + Send;
40+
fn upsert_peer(
41+
&self,
42+
info_hash: &InfoHash,
43+
peer: &peer::Peer,
44+
opt_persistent_torrent: Option<PersistentTorrent>,
45+
) -> impl std::future::Future<Output = bool> + Send;
4146
fn get_swarm_metadata(&self, info_hash: &InfoHash) -> impl std::future::Future<Output = Option<SwarmMetadata>> + Send;
4247
}

packages/torrent-repository/src/repository/rw_lock_std.rs

+5-3
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ use torrust_tracker_configuration::TrackerPolicy;
33
use torrust_tracker_primitives::pagination::Pagination;
44
use torrust_tracker_primitives::swarm_metadata::SwarmMetadata;
55
use torrust_tracker_primitives::torrent_metrics::TorrentsMetrics;
6-
use torrust_tracker_primitives::{peer, DurationSinceUnixEpoch, PersistentTorrents};
6+
use torrust_tracker_primitives::{peer, DurationSinceUnixEpoch, PersistentTorrent, PersistentTorrents};
77

88
use super::Repository;
99
use crate::entry::peer_list::PeerList;
@@ -46,12 +46,14 @@ impl Repository<EntrySingle> for TorrentsRwLockStd
4646
where
4747
EntrySingle: Entry,
4848
{
49-
fn upsert_peer(&self, info_hash: &InfoHash, peer: &peer::Peer) {
49+
fn upsert_peer(&self, info_hash: &InfoHash, peer: &peer::Peer, _opt_persistent_torrent: Option<PersistentTorrent>) -> bool {
50+
// todo: load persistent torrent data if provided
51+
5052
let mut db = self.get_torrents_mut();
5153

5254
let entry = db.entry(*info_hash).or_insert(EntrySingle::default());
5355

54-
entry.upsert_peer(peer);
56+
entry.upsert_peer(peer)
5557
}
5658

5759
fn get_swarm_metadata(&self, info_hash: &InfoHash) -> Option<SwarmMetadata> {

packages/torrent-repository/src/repository/rw_lock_std_mutex_std.rs

+5-3
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ use torrust_tracker_configuration::TrackerPolicy;
55
use torrust_tracker_primitives::pagination::Pagination;
66
use torrust_tracker_primitives::swarm_metadata::SwarmMetadata;
77
use torrust_tracker_primitives::torrent_metrics::TorrentsMetrics;
8-
use torrust_tracker_primitives::{peer, DurationSinceUnixEpoch, PersistentTorrents};
8+
use torrust_tracker_primitives::{peer, DurationSinceUnixEpoch, PersistentTorrent, PersistentTorrents};
99

1010
use super::Repository;
1111
use crate::entry::peer_list::PeerList;
@@ -33,7 +33,9 @@ where
3333
EntryMutexStd: EntrySync,
3434
EntrySingle: Entry,
3535
{
36-
fn upsert_peer(&self, info_hash: &InfoHash, peer: &peer::Peer) {
36+
fn upsert_peer(&self, info_hash: &InfoHash, peer: &peer::Peer, _opt_persistent_torrent: Option<PersistentTorrent>) -> bool {
37+
// todo: load persistent torrent data if provided
38+
3739
let maybe_entry = self.get_torrents().get(info_hash).cloned();
3840

3941
let entry = if let Some(entry) = maybe_entry {
@@ -44,7 +46,7 @@ where
4446
entry.clone()
4547
};
4648

47-
entry.upsert_peer(peer);
49+
entry.upsert_peer(peer)
4850
}
4951

5052
fn get_swarm_metadata(&self, info_hash: &InfoHash) -> Option<SwarmMetadata> {

packages/torrent-repository/src/repository/rw_lock_std_mutex_tokio.rs

+10-3
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ use torrust_tracker_configuration::TrackerPolicy;
99
use torrust_tracker_primitives::pagination::Pagination;
1010
use torrust_tracker_primitives::swarm_metadata::SwarmMetadata;
1111
use torrust_tracker_primitives::torrent_metrics::TorrentsMetrics;
12-
use torrust_tracker_primitives::{peer, DurationSinceUnixEpoch, PersistentTorrents};
12+
use torrust_tracker_primitives::{peer, DurationSinceUnixEpoch, PersistentTorrent, PersistentTorrents};
1313

1414
use super::RepositoryAsync;
1515
use crate::entry::peer_list::PeerList;
@@ -37,7 +37,14 @@ where
3737
EntryMutexTokio: EntryAsync,
3838
EntrySingle: Entry,
3939
{
40-
async fn upsert_peer(&self, info_hash: &InfoHash, peer: &peer::Peer) {
40+
async fn upsert_peer(
41+
&self,
42+
info_hash: &InfoHash,
43+
peer: &peer::Peer,
44+
_opt_persistent_torrent: Option<PersistentTorrent>,
45+
) -> bool {
46+
// todo: load persistent torrent data if provided
47+
4148
let maybe_entry = self.get_torrents().get(info_hash).cloned();
4249

4350
let entry = if let Some(entry) = maybe_entry {
@@ -48,7 +55,7 @@ where
4855
entry.clone()
4956
};
5057

51-
entry.upsert_peer(peer).await;
58+
entry.upsert_peer(peer).await
5259
}
5360

5461
async fn get_swarm_metadata(&self, info_hash: &InfoHash) -> Option<SwarmMetadata> {

packages/torrent-repository/src/repository/rw_lock_tokio.rs

+10-3
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ use torrust_tracker_configuration::TrackerPolicy;
33
use torrust_tracker_primitives::pagination::Pagination;
44
use torrust_tracker_primitives::swarm_metadata::SwarmMetadata;
55
use torrust_tracker_primitives::torrent_metrics::TorrentsMetrics;
6-
use torrust_tracker_primitives::{peer, DurationSinceUnixEpoch, PersistentTorrents};
6+
use torrust_tracker_primitives::{peer, DurationSinceUnixEpoch, PersistentTorrent, PersistentTorrents};
77

88
use super::RepositoryAsync;
99
use crate::entry::peer_list::PeerList;
@@ -47,12 +47,19 @@ impl RepositoryAsync<EntrySingle> for TorrentsRwLockTokio
4747
where
4848
EntrySingle: Entry,
4949
{
50-
async fn upsert_peer(&self, info_hash: &InfoHash, peer: &peer::Peer) {
50+
async fn upsert_peer(
51+
&self,
52+
info_hash: &InfoHash,
53+
peer: &peer::Peer,
54+
_opt_persistent_torrent: Option<PersistentTorrent>,
55+
) -> bool {
56+
// todo: load persistent torrent data if provided
57+
5158
let mut db = self.get_torrents_mut().await;
5259

5360
let entry = db.entry(*info_hash).or_insert(EntrySingle::default());
5461

55-
entry.upsert_peer(peer);
62+
entry.upsert_peer(peer)
5663
}
5764

5865
async fn get_swarm_metadata(&self, info_hash: &InfoHash) -> Option<SwarmMetadata> {

packages/torrent-repository/src/repository/rw_lock_tokio_mutex_std.rs

+10-3
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ use torrust_tracker_configuration::TrackerPolicy;
55
use torrust_tracker_primitives::pagination::Pagination;
66
use torrust_tracker_primitives::swarm_metadata::SwarmMetadata;
77
use torrust_tracker_primitives::torrent_metrics::TorrentsMetrics;
8-
use torrust_tracker_primitives::{peer, DurationSinceUnixEpoch, PersistentTorrents};
8+
use torrust_tracker_primitives::{peer, DurationSinceUnixEpoch, PersistentTorrent, PersistentTorrents};
99

1010
use super::RepositoryAsync;
1111
use crate::entry::peer_list::PeerList;
@@ -35,7 +35,14 @@ where
3535
EntryMutexStd: EntrySync,
3636
EntrySingle: Entry,
3737
{
38-
async fn upsert_peer(&self, info_hash: &InfoHash, peer: &peer::Peer) {
38+
async fn upsert_peer(
39+
&self,
40+
info_hash: &InfoHash,
41+
peer: &peer::Peer,
42+
_opt_persistent_torrent: Option<PersistentTorrent>,
43+
) -> bool {
44+
// todo: load persistent torrent data if provided
45+
3946
let maybe_entry = self.get_torrents().await.get(info_hash).cloned();
4047

4148
let entry = if let Some(entry) = maybe_entry {
@@ -46,7 +53,7 @@ where
4653
entry.clone()
4754
};
4855

49-
entry.upsert_peer(peer);
56+
entry.upsert_peer(peer)
5057
}
5158

5259
async fn get_swarm_metadata(&self, info_hash: &InfoHash) -> Option<SwarmMetadata> {

0 commit comments

Comments
 (0)