Skip to content

Conversation

@josecelano
Copy link
Member

@josecelano josecelano commented May 21, 2025

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:

  • Scaffolding for listening to torrent-repository events in the tracker-core package.
  • Move the SQL update from the AnnounceHandler to the event handler (handle_event).

Cleanup:

  • 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.
  • Refactor: rename upsert_peer to handle_announcement.
  • Refactor: rename AnnounceHandler::announce to AnnounceHandler::handle_announcement
  • 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:

let opt_persistent_torrent = if self.config.tracker_policy.persistent_torrent_completed_stat {
    self.db_torrent_repository.load(info_hash)?
} else {
    None
};

…ker-core pkg

This will enable udpating stats (number of torrent downloads per
torrent) from the event handler (persisting in the DB).

And after that, I will enable adding lebeled metrics.
@josecelano josecelano requested a review from da2ce7 May 21, 2025 16:17
@josecelano josecelano self-assigned this May 21, 2025
@josecelano josecelano added the Code Cleanup / Refactoring Tidying and Making Neat label May 21, 2025
@codecov
Copy link

codecov bot commented May 21, 2025

Codecov Report

Attention: Patch coverage is 84.23645% with 32 lines in your changes missing coverage. Please review.

Project coverage is 83.88%. Comparing base (93f851d) to head (67d177b).
Report is 11 commits behind head on develop.

Files with missing lines Patch % Lines
...ages/tracker-core/src/statistics/event/listener.rs 57.14% 11 Missing and 1 partial ⚠️
...kages/tracker-core/src/statistics/event/handler.rs 56.00% 6 Missing and 5 partials ⚠️
src/bootstrap/jobs/tracker_core.rs 44.44% 4 Missing and 1 partial ⚠️
packages/udp-tracker-server/src/environment.rs 0.00% 2 Missing ⚠️
packages/http-tracker-core/src/services/scrape.rs 66.66% 0 Missing and 1 partial ⚠️
src/bootstrap/jobs/torrent_repository.rs 0.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #1531      +/-   ##
===========================================
- Coverage    83.92%   83.88%   -0.04%     
===========================================
  Files          267      270       +3     
  Lines        19207    19181      -26     
  Branches     19207    19181      -26     
===========================================
- Hits         16119    16090      -29     
- Misses        2811     2817       +6     
+ Partials       277      274       -3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@josecelano josecelano added the - Developer - Torrust Improvement Experience label May 21, 2025
@josecelano josecelano changed the title Overhaul stats: simplify how the AnnounceHandler persist number of downloads in tracker-core package Overhaul stats: Simplify how the AnnounceHandler persist number of downloads in tracker-core package May 22, 2025
The returned value is not needed anymore. Secondary action (increase
metrics) is done in the event listeners.
@josecelano
Copy link
Member Author

ACK 67d177b

@josecelano josecelano marked this pull request as ready for review May 26, 2025 15:18
@josecelano josecelano merged commit f21957d into torrust:develop May 26, 2025
21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

- Developer - Torrust Improvement Experience Code Cleanup / Refactoring Tidying and Making Neat

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Overhaul stats: simplify how the AnnounceHandler persist number of downloads in tracker-core package

1 participant