Skip to content

Commit f21957d

Browse files
committed
Merge #1531: Overhaul stats: Simplify how the AnnounceHandler persist number of downloads in tracker-core package
67d177b refactor: [#1524] command/query separation (Jose Celano) 8c31549 refactor: [#1524] rename methods (Jose Celano) 28603fe refactor: [#1524] extract TestEnv for integration tests in tracker-core (Jose Celano) ab2f52d fix: [#1524] test (move to integration test) (Jose Celano) b05bccd refactor: [#1524] integration tests in tracker-core (Jose Celano) f3d3495 refactor: [#1524] remove duplciate code for tracker core container (Jose Celano) e90585a refactor: [#1524] move total downloads udpate from announce command to event handler (Jose Celano) 8968757 refactor: extract method JobManger::push_opt (Jose Celano) 7e933f9 feat: [#1524] listens to torrent-repository events in the tracker-core pkg (Jose Celano) 3a23a38 fix: tracing message (Jose Celano) Pull request description: Simplify how the `AnnounceHandler` persist number of downloads in `tracker-core` package. It's now done asynchronously instead of synchronously. **NOTICE:** That `bittorrent_tracker_core::announce_handler::AnnounceHandler::announce` will not return a database error anymore, meaning the peer will receive the announce response even if the tracker can't update the downloads counter. ### Subtasks Main change: - [x] Scaffolding for listening to torrent-repository events in the `tracker-core` package. - [x] Move the SQL update from the `AnnounceHandler` to the event handler (`handle_event`). Cleanup: - [x] Fix test. https://github.com/torrust/torrust-tracker/actions/runs/15168471742/job/42652507081?pr=1531#step:7:250 - It's not the service's responsibility to increment the counter anymore. Add: - A unit test in the event handler to check that the counter is increased when the event is received. - An integration test to simulate a peer completing downloading. - Maybe we should also add a main app E2E test (to make sure the listener is initialised), but we are not testing it for other jobs directly. We have to review the strategy for E2E tests in the main app. - [x] Refactor: rename `upsert_peer` to `handle_announcement`. - [x] Refactor: rename `AnnounceHandler::announce` to `AnnounceHandler::handle_announcement` - [x] Refactor: Don't return the boolean `number of downloads increased` in `upsert_peer`. `upsert_peer` should only be a command decoupled from other secondary tasks. Side effects should be captured from events. NOTE: I cannot remove the `db_torrent_repository` dependency from the `AnnounceHandler` yet until we implement the new persistent metric for the number of downloads, because it's still used in: ```rust let opt_persistent_torrent = if self.config.tracker_policy.persistent_torrent_completed_stat { self.db_torrent_repository.load(info_hash)? } else { None }; ``` ACKs for top commit: josecelano: ACK 67d177b Tree-SHA512: 5d33fc5e383d280dda68879c165f736729c0659a1f69136ef2fbc48ed35e7a87e4cbf4ba30e5654f2192a3dbeabe4d474f8f5c54ae310320a2888b234ecf9614
2 parents 93f851d + 67d177b commit f21957d

File tree

32 files changed

+560
-405
lines changed

32 files changed

+560
-405
lines changed

Cargo.lock

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,12 +25,12 @@ pub struct Environment<S> {
2525

2626
impl<S> Environment<S> {
2727
/// Add a torrent to the tracker
28-
pub async fn add_torrent_peer(&self, info_hash: &InfoHash, peer: &peer::Peer) -> bool {
28+
pub async fn add_torrent_peer(&self, info_hash: &InfoHash, peer: &peer::Peer) {
2929
self.container
3030
.tracker_core_container
3131
.in_memory_torrent_repository
32-
.upsert_peer(info_hash, peer, None)
33-
.await
32+
.handle_announcement(info_hash, peer, None)
33+
.await;
3434
}
3535
}
3636

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -33,12 +33,12 @@ where
3333
S: std::fmt::Debug + std::fmt::Display,
3434
{
3535
/// Add a torrent to the tracker
36-
pub async fn add_torrent_peer(&self, info_hash: &InfoHash, peer: &peer::Peer) -> bool {
36+
pub async fn add_torrent_peer(&self, info_hash: &InfoHash, peer: &peer::Peer) {
3737
self.container
3838
.tracker_core_container
3939
.in_memory_torrent_repository
40-
.upsert_peer(info_hash, peer, None)
41-
.await
40+
.handle_announcement(info_hash, peer, None)
41+
.await;
4242
}
4343
}
4444

packages/http-tracker-core/src/services/announce.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ impl AnnounceService {
8787

8888
let announce_data = self
8989
.announce_handler
90-
.announce(
90+
.handle_announcement(
9191
&announce_request.info_hash,
9292
&mut peer,
9393
&remote_client_addr.ip(),

packages/http-tracker-core/src/services/scrape.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ impl ScrapeService {
7878
let scrape_data = if self.authentication_is_required() && !self.is_authenticated(maybe_key).await {
7979
ScrapeData::zeroed(&scrape_request.info_hashes)
8080
} else {
81-
self.scrape_handler.scrape(&scrape_request.info_hashes).await?
81+
self.scrape_handler.handle_scrape(&scrape_request.info_hashes).await?
8282
};
8383

8484
let remote_client_addr = resolve_remote_client_addr(&self.core_config.net.on_reverse_proxy.into(), client_ip_sources)?;
@@ -291,7 +291,7 @@ mod tests {
291291
let original_peer_ip = peer.ip();
292292
container
293293
.announce_handler
294-
.announce(&info_hash, &mut peer, &original_peer_ip, &PeersWanted::AsManyAsPossible)
294+
.handle_announcement(&info_hash, &mut peer, &original_peer_ip, &PeersWanted::AsManyAsPossible)
295295
.await
296296
.unwrap();
297297

@@ -482,7 +482,7 @@ mod tests {
482482
let original_peer_ip = peer.ip();
483483
container
484484
.announce_handler
485-
.announce(&info_hash, &mut peer, &original_peer_ip, &PeersWanted::AsManyAsPossible)
485+
.handle_announcement(&info_hash, &mut peer, &original_peer_ip, &PeersWanted::AsManyAsPossible)
486486
.await
487487
.unwrap();
488488

0 commit comments

Comments
 (0)