Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add Timeouts to Tracker Client #765

Open
wants to merge 265 commits into
base: develop
Choose a base branch
from
Open

Add Timeouts to Tracker Client #765

wants to merge 265 commits into from

Conversation

da2ce7
Copy link
Contributor

@da2ce7 da2ce7 commented Mar 28, 2024 •
edited by josecelano
Loading

closes: #677 closes: #682 closes: #681 closes: #679 closes: #680 closes: #678 closes: #683 closes: #676 closes: #767

part of epic:

todo:

  • Write More Tests
  • Do Code Review
  • Make Timeouts Configurable for the UDP client, like the Http Client.

done:

@da2ce7 da2ce7 force-pushed the 20240318_tracker_client_work branch from 2933092 to 2e1893a Compare March 28, 2024 05:05
@da2ce7 da2ce7 changed the title Add timeouts to Torrent Client Add Timeouts to Tracker Client Mar 28, 2024
@codecov Codecov
Copy link

codecov bot commented Mar 28, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 53.52798% with 191 lines in your changes are missing coverage. Please review.

Project coverage is 80.08%. Comparing base (bf71687) to head (2a72445).
Report is 2 commits behind head on develop.

Files Patch % Lines
src/shared/bit_torrent/tracker/udp/client.rs 65.00% 42 Missing ⚠️
...bit_torrent/tracker/http/client/requests/scrape.rs 44.82% 16 Missing ⚠️
src/shared/bit_torrent/tracker/udp/mod.rs 27.77% 13 Missing ⚠️
src/console/clients/udp/mod.rs 0.00% 11 Missing ⚠️
...t_torrent/tracker/http/client/requests/announce.rs 74.41% 11 Missing ⚠️
src/console/clients/checker/checks/udp.rs 0.00% 9 Missing ⚠️
src/console/clients/http/mod.rs 0.00% 9 Missing ⚠️
...d/bit_torrent/tracker/http/client/responses/mod.rs 18.18% 9 Missing ⚠️
src/console/clients/checker/app.rs 0.00% 8 Missing ⚠️
src/console/clients/checker/checks/health.rs 0.00% 8 Missing ⚠️
... and 14 more
Additional details and impacted files 
@@             Coverage Diff             @@
##           develop     #765      +/-   ##
===========================================
+ Coverage    77.71%   80.08%   +2.36%     
===========================================
  Files          158      162       +4     
  Lines         8711     8894     +183     
===========================================
+ Hits          6770     7123     +353     
+ Misses        1941     1771     -170

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

@da2ce7 da2ce7 marked this pull request as ready for review March 30, 2024 03:55
@da2ce7 da2ce7 requested a review from josecelano March 30, 2024 03:55
@da2ce7
Copy link
Contributor Author

da2ce7 commented Mar 30, 2024

@josecelano This Pull Request is Ready for Review, however it needs some polish before merging. :)

da2ce7
da2ce7 commented Mar 30, 2024
View reviewed changes
src/console/clients/checker/checks/udp.rs Outdated

check_results.push(match res {
Ok(_) => {
console.println(&format!("{} - Announce of {} is OK", "✓".green(), colored_addr));
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: Should Be "Scrape" in Console Message Here

@josecelano josecelano self-assigned this Apr 1, 2024
josecelano
josecelano previously approved these changes Apr 1, 2024
View reviewed changes
Copy link
Member

@josecelano josecelano left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @da2ce7 overall I think this is very good refactor. I'm going to merge it. However, we should discuss future related work.

See my comments here.

I think:

  • We should move the mod src/shared/bit_torrent to a new package (packages/bit_torrent).
  • That bit_torrent package could be splitted into several ones. See Refactor packages #753.
  • Maybe the server and client of the same server should go in the same package. I did it on the Index APIhttps://github.com/torrust/torrust-index/tree/develop/src/web/api. Changes in the server API imply changes in the client. Besides we are versioning the API, I think it would be easier to maintain the client if it's closer to the server code. Maybe the server and client could be features of the same package otherwise you would need three packages per service: primitives, server and client. If we use only one package it could by default expose primitives and the client and the server could be features disabled by default. What do you think? Having an independent package for each service would allow other users to use them and even run a single service if they want. FOr example, some users might want to run only the UDP tracker.
  • For the HTTP client requests and responses are still duplicated. If we move the server and client to the same package we can remove that duplication. They are duplicated because I did not want to couple the client to the server implementation, but when both the server and the client depend on the HTTP tracker primitives that would not be a problem. Althougth it could be the case we want them to be duplicated because there are some aspects of the types in the server we don't want to expose in the API.

packages/configuration/src/lib.rs Outdated

/// A magic 64-bit integer constant defined in the protocol that is used to
/// identify the protocol.
pub const PROTOCOL_ID: i64 = 0x0417_2710_1980;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hi @da2ce7 in this context I would rename the const to UDP_PROTOCOL_ID or move the const to a UDP mod.

packages/configuration/src/lib.rs
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @da2ce7 I think this mod is too big. I would create a mod for each service and only put things in the root mod that are common to all services.

src/console/clients/http/app.rs
@@ -14,22 +14,21 @@
//! cargo run --bin http_tracker_client scrape http://127.0.0.1:7070 9c38422213e30bff212b30c360d26f9a02136422 | jq
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@da2ce7 update this to:

cargo run --bin http_tracker_client 10 scrape http://127.0.0.1:7070 9c38422213e30bff212b30c360d26f9a02136422 | jq

Where 10 is the timeout.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we can use this as an example:

cargo run --bin http_tracker_client -- --help

That never changes.

@josecelano josecelano added the Needs Rebase Base Branch has Incompatibilities label Apr 1, 2024
@josecelano
chore(deps): update dependencies
5373493 
```output
cargo update
    Updating crates.io index
    Updating async-compression v0.4.6 -> v0.4.8
    Updating async-executor v1.8.0 -> v1.10.0
    Updating autocfg v1.1.0 -> v1.2.0
      Adding base64 v0.22.0
    Updating borsh v1.3.1 -> v1.4.0
    Updating borsh-derive v1.3.1 -> v1.4.0
    Updating brotli v3.5.0 -> v4.0.0
    Updating brotli-decompressor v2.5.1 -> v3.0.0
    Updating bumpalo v3.15.4 -> v3.16.0
    Updating cc v1.0.90 -> v1.0.91
    Updating chrono v0.4.35 -> v0.4.37
    Updating event-listener v5.2.0 -> v5.3.0
    Updating event-listener-strategy v0.5.0 -> v0.5.1
    Updating getrandom v0.2.12 -> v0.2.14
    Updating h2 v0.4.3 -> v0.4.4
    Updating half v2.4.0 -> v2.4.1
    Updating itoa v1.0.10 -> v1.0.11
    Updating memchr v2.7.1 -> v2.7.2
    Updating openssl-sys v0.9.101 -> v0.9.102
    Updating pest v2.7.8 -> v2.7.9
    Updating pest_derive v2.7.8 -> v2.7.9
    Updating pest_generator v2.7.8 -> v2.7.9
    Updating pest_meta v2.7.8 -> v2.7.9
    Updating pin-project-lite v0.2.13 -> v0.2.14
    Updating regex-syntax v0.8.2 -> v0.8.3
    Updating reqwest v0.12.2 -> v0.12.3
    Updating rust_decimal v1.34.3 -> v1.35.0
    Removing rustls-pemfile v1.0.4
    Removing rustls-pemfile v2.1.1
      Adding rustls-pemfile v2.1.2
    Updating rustls-pki-types v1.4.0 -> v1.4.1
    Updating rustversion v1.0.14 -> v1.0.15
    Updating security-framework v2.9.2 -> v2.10.0
    Updating security-framework-sys v2.9.1 -> v2.10.0
    Updating serde_html_form v0.2.5 -> v0.2.6
    Updating serde_json v1.0.114 -> v1.0.115
    Updating strsim v0.11.0 -> v0.11.1
    Updating syn v2.0.55 -> v2.0.58
    Updating sync_wrapper v1.0.0 -> v1.0.1
    Updating tokio v1.36.0 -> v1.37.0
    Updating winreg v0.50.0 -> v0.52.0
    Updating zstd v0.13.0 -> v0.13.1
    Updating zstd-safe v7.0.0 -> v7.1.0
    Updating zstd-sys v2.0.9+zstd.1.5.5 -> v2.0.10+zstd.1.5.6
```
@josecelano
Merge torrust#780: Update dependencies
af52045 
5373493 chore(deps): update dependencies (Jose Celano)

Pull request description:

  ```output
  cargo update
      Updating crates.io index
      Updating async-compression v0.4.6 -> v0.4.8
      Updating async-executor v1.8.0 -> v1.10.0
      Updating autocfg v1.1.0 -> v1.2.0
        Adding base64 v0.22.0
      Updating borsh v1.3.1 -> v1.4.0
      Updating borsh-derive v1.3.1 -> v1.4.0
      Updating brotli v3.5.0 -> v4.0.0
      Updating brotli-decompressor v2.5.1 -> v3.0.0
      Updating bumpalo v3.15.4 -> v3.16.0
      Updating cc v1.0.90 -> v1.0.91
      Updating chrono v0.4.35 -> v0.4.37
      Updating event-listener v5.2.0 -> v5.3.0
      Updating event-listener-strategy v0.5.0 -> v0.5.1
      Updating getrandom v0.2.12 -> v0.2.14
      Updating h2 v0.4.3 -> v0.4.4
      Updating half v2.4.0 -> v2.4.1
      Updating itoa v1.0.10 -> v1.0.11
      Updating memchr v2.7.1 -> v2.7.2
      Updating openssl-sys v0.9.101 -> v0.9.102
      Updating pest v2.7.8 -> v2.7.9
      Updating pest_derive v2.7.8 -> v2.7.9
      Updating pest_generator v2.7.8 -> v2.7.9
      Updating pest_meta v2.7.8 -> v2.7.9
      Updating pin-project-lite v0.2.13 -> v0.2.14
      Updating regex-syntax v0.8.2 -> v0.8.3
      Updating reqwest v0.12.2 -> v0.12.3
      Updating rust_decimal v1.34.3 -> v1.35.0
      Removing rustls-pemfile v1.0.4
      Removing rustls-pemfile v2.1.1
        Adding rustls-pemfile v2.1.2
      Updating rustls-pki-types v1.4.0 -> v1.4.1
      Updating rustversion v1.0.14 -> v1.0.15
      Updating security-framework v2.9.2 -> v2.10.0
      Updating security-framework-sys v2.9.1 -> v2.10.0
      Updating serde_html_form v0.2.5 -> v0.2.6
      Updating serde_json v1.0.114 -> v1.0.115
      Updating strsim v0.11.0 -> v0.11.1
      Updating syn v2.0.55 -> v2.0.58
      Updating sync_wrapper v1.0.0 -> v1.0.1
      Updating tokio v1.36.0 -> v1.37.0
      Updating winreg v0.50.0 -> v0.52.0
      Updating zstd v0.13.0 -> v0.13.1
      Updating zstd-safe v7.0.0 -> v7.1.0
      Updating zstd-sys v2.0.9+zstd.1.5.5 -> v2.0.10+zstd.1.5.6
  ```

ACKs for top commit:
  josecelano:
    ACK 5373493

Tree-SHA512: f010b77d0723f2cacf0b6aaf35f0bafce68e3622e6c5157a7b41e5b277a71ca22da922ff2a8541fee35f7933f1686351cfb5dcf46b3ae9ae390ccd39f03a5806
@josecelano
chore(deps): add cargo dependency: dashmap
78b46c4 
It will be used to create a new torrent repository implementation,
using a DashMap for the torrent list.

DashMap crate: https://crates.io/crates/dashmap
@josecelano
feat: [torrust#565] new torrent repository implementation usind DashMap
00ee9db 
It's not enabled as the deafult repository becuase DashMap does not
return the items in order. Some tests fail:

``` output
failures:

---- core::services::torrent::tests::searching_for_torrents::should_return_torrents_ordered_by_info_hash stdout ----
thread 'core::services::torrent::tests::searching_for_torrents::should_return_torrents_ordered_by_info_hash' panicked at src/core/services/torrent.rs:303:13:
assertion `left == right` failed
  left: [BasicInfo { info_hash: InfoHash([158, 2, 23, 208, 250, 113, 200, 115, 50, 205, 139, 249, 219, 234, 188, 178, 194, 207, 60, 77]), seeders: 1, completed: 0, leechers: 0 }, BasicInfo { info_hash: InfoHash([3, 132, 5, 72, 100, 58, 242, 167, 182, 58, 159, 92, 188, 163, 72, 188, 113, 80, 202, 58]), seeders: 1, completed: 0, leechers: 0 }]
 right: [BasicInfo { info_hash: InfoHash([3, 132, 5, 72, 100, 58, 242, 167, 182, 58, 159, 92, 188, 163, 72, 188, 113, 80, 202, 58]), seeders: 1, completed: 0, leechers: 0 }, BasicInfo { info_hash: InfoHash([158, 2, 23, 208, 250, 113, 200, 115, 50, 205, 139, 249, 219, 234, 188, 178, 194, 207, 60, 77]), seeders: 1, completed: 0, leechers: 0 }]
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

failures:
    core::services::torrent::tests::searching_for_torrents::should_return_torrents_ordered_by_info_hash

test result: FAILED. 212 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out; finished in 1.18s

error: test failed, to rerun pass `--lib`
```

On the other hand, to use it, a new data strcuture should be added to the repo:

An Index with sorted torrents by Infohash

The API uses pagination returning torrents in alphabetically order by InfoHash. Adding such an Index would probably
decrease the performace of this repository implementation. And it's
performace looks similar to the current SkipMap implementation.

SkipMap performace with Aquatic UDP load test:

```
Requests out: 396788.68/second
Responses in: 357105.27/second
  - Connect responses:  176662.91
  - Announce responses: 176863.44
  - Scrape responses:   3578.91
  - Error responses:    0.00
Peers per announce response: 0.00
Announce responses per info hash:
  - p10: 1
  - p25: 1
  - p50: 1
  - p75: 1
  - p90: 2
  - p95: 3
  - p99: 105
  - p99.9: 287
  - p100: 351
```

DashMap performace with Aquatic UDP load test:

```
Requests out: 410658.38/second
Responses in: 365892.86/second
  - Connect responses:  181258.91
  - Announce responses: 181005.95
  - Scrape responses:   3628.00
  - Error responses:    0.00
Peers per announce response: 0.00
Announce responses per info hash:
  - p10: 1
  - p25: 1
  - p50: 1
  - p75: 1
  - p90: 2
  - p95: 3
  - p99: 104
  - p99.9: 295
  - p100: 363
```
@josecelano
chore: add dashmap cargo dep to cargp machete
1e76c17 
It's only used for benchmarking.
@josecelano
fix: torrent repository tests. DashMap is not ordered
4030fd1 
DashMap implementation does not support returning torrent ordered, so we
have to skip those tests for this reporitoy implementation.
@josecelano
Merge torrust#784: Performance optimization: create a new torrent rep…
b5fb03b 
…ository using `DashMap`

4030fd1 fix: torrent repository tests. DashMap is not ordered (Jose Celano)
1e76c17 chore: add dashmap cargo dep to cargp machete (Jose Celano)
00ee9db feat: [torrust#565] new torrent repository implementation usind DashMap (Jose Celano)
78b46c4 chore(deps): add cargo dependency: dashmap (Jose Celano)

Pull request description:

  Relates to:

  - torrust#778
  - torrust#567 (comment)

  This PR adds a new torrent repository implementation where the outer collection for torrents uses a [DashMap](https://docs.rs/dashmap/latest/dashmap/).

  This is something @mickvandijke was working on this [PR](torrust#645).

  There have been many changes. @da2ce7 has extracted a [package for the repositories](https://github.com/torrust/torrust-tracker/tree/develop/packages/torrent-repository).

  This PR adds a new repo using DashMap to the new package. However, it does not implement some of the extra features @mickvandijke added to the other [PR](torrust#645). For example, it does not limit memory consumption. None of the other repos have that feature, so I suggest merging this PR and implementing that feature in the future for all repos.

  ### Why

  The current repository used in production is the one using a [SkipMap](https://docs.rs/crossbeam-skiplist/latest/crossbeam_skiplist/struct.SkipMap.html) data structure. DashMap was the first type we considered to allow adding new torrents in parallels.

  The implementation with DashMap has not been merged because it does not guarantee the order when you iterate over the torrents. The tracker API returns torrents ordered by InfoHash. To avoid breaking the API that PR would need to add a new data structure (kind of Index) to keep the torrent list ordered.

  This implementation helps us run performance tests with this option without spending much time fixing its limitations. It looks like the performance is similar to the SkiMap, so we don't need to use it in production now. We only need to keep it for benchmarking in the future if other versions are better.

  ### Benchmarking

  Running the Aquatic UDP load test, the DashMap looks slightly better.

  SkipMap:

  ```output
  Requests out: 396788.68/second
  Responses in: 357105.27/second
    - Connect responses:  176662.91
    - Announce responses: 176863.44
    - Scrape responses:   3578.91
    - Error responses:    0.00
  Peers per announce response: 0.00
  Announce responses per info hash:
    - p10: 1
    - p25: 1
    - p50: 1
    - p75: 1
    - p90: 2
    - p95: 3
    - p99: 105
    - p99.9: 287
    - p100: 351
  ```

  DashMap with initial capacity 0 (best result. On average is lower, similar to SkipMap):

  ```output
  Requests out: 410658.38/second
  Responses in: 365892.86/second
    - Connect responses:  181258.91
    - Announce responses: 181005.95
    - Scrape responses:   3628.00
    - Error responses:    0.00
  Peers per announce response: 0.00
  Announce responses per info hash:
    - p10: 1
    - p25: 1
    - p50: 1
    - p75: 1
    - p90: 2
    - p95: 3
    - p99: 104
    - p99.9: 295
    - p100: 363
  ```

  With Criterion:

  ![image](https://github.com/torrust/torrust-tracker/assets/58816/1e1fca2d-821d-4e2c-adf8-49e055758bd0)

  ![image](https://github.com/torrust/torrust-tracker/assets/58816/fcb9a32f-c4f1-4ffd-9797-4a74fc000336)

  ![image](https://github.com/torrust/torrust-tracker/assets/58816/efe2d788-31ac-4997-82f6-d022aa8f79a0)

  ![image](https://github.com/torrust/torrust-tracker/assets/58816/f64a4e5a-a363-48cd-87d9-78162cda11d4)

  ### Conclusion

  From my point of view, other [performance optimisations](torrust#774) have more potential than this, considering this implementation is not finished. The changes needed to finish this implementation will probably decrease the performance obtained in this benchmarking. In the future, we can review this option if we change the [tracker API behaviour to getting all torrents](torrust#775).

ACKs for top commit:
  josecelano:
    ACK 4030fd1

Tree-SHA512: 1a6c56f3aecb34fc40c401597efe1663c3cc66903f2d27bf8f2bbc6b058080d487a89b705c224657aaa6059f1c2a8597583e636e30727f9293bb43f460441415
@josecelano
refactor: segregate command and query for announce request
aa4bfba 
This changes the API of the torrent repository. The method:

```
fn update_torrent_with_peer_and_get_stats(&self, info_hash: &InfoHash, peer: &peer::Peer) -> (bool, SwarmMetadata);
```

is replaced with:

```
fn upsert_peer(&self, info_hash: &InfoHash, peer: &peer::Peer);
fn get_swarm_metadata(&self, info_hash: &InfoHash) -> Option<SwarmMetadata>;
```

The performance is not affected. Benchmaring is still using both methods
in order to simulate `announce` requests.

1. The interface is simpler (command/query segregation.
2. In the long-term:
    - Returning swarm metadata in the announce request could be
      optional. The announce request process would be faster if the
tracker does not have to mantain the swarm data. This is not likely to
happen becuase the scrape request needs this metadata.
    - New repository performance improvements could be implemented. This allow
      decoupling peer lists from swarm metadata. The repository
internally can have two data strcutures one for the peer list and
another for the swarm metatada. Both using different locks.
@josecelano
Merge torrust#792: Refactor: segregate command and query for announce…
24b82f3 
… request

aa4bfba refactor: segregate command and query for announce request (Jose Celano)

Pull request description:

  This changes the API of the torrent repository.

  The method:

  ```rust
  fn update_torrent_with_peer_and_get_stats(&self, info_hash: &InfoHash, peer: &peer::Peer) -> (bool, SwarmMetadata);
  ```

  is replaced with two methods (command and query):

  ```rust
  fn upsert_peer(&self, info_hash: &InfoHash, peer: &peer::Peer);
  fn get_swarm_metadata(&self, info_hash: &InfoHash) -> Option<SwarmMetadata>;
  ```

  The performance is not affected. Bechmarking is still using both methods in order to simulate `announce` requests.

  The **interface is simpler** (command/query segregation).

ACKs for top commit:
  josecelano:
    ACK aa4bfba

Tree-SHA512: 64229572b260f1f898f48786d27e476259d19a2202cd3b05328b137e84bad32a72ffcf2d7255a1a1daaa28f57bed879d113fe11779d96ecb191a83fe94245393
@josecelano
chore(deps): update dependencies
5fa01e7 
```output
    Updating crates.io index
    Updating allocator-api2 v0.2.16 -> v0.2.18
    Updating anyhow v1.0.81 -> v1.0.82
    Updating async-channel v2.2.0 -> v2.2.1
    Updating async-executor v1.10.0 -> v1.11.0
    Updating async-trait v0.1.79 -> v0.1.80
    Updating axum-client-ip v0.5.1 -> v0.6.0
    Updating cc v1.0.91 -> v1.0.94
    Updating either v1.10.0 -> v1.11.0
    Updating encoding_rs v0.8.33 -> v0.8.34
    Updating jobserver v0.1.28 -> v0.1.30
    Updating proc-macro2 v1.0.79 -> v1.0.80
    Updating quote v1.0.35 -> v1.0.36
    Updating rstest v0.18.2 -> v0.19.0
    Updating rstest_macros v0.18.2 -> v0.19.0
    Updating serde_repr v0.1.18 -> v0.1.19
    Updating syn v2.0.58 -> v2.0.59
    Updating time v0.3.34 -> v0.3.36
    Updating time-macros v0.2.17 -> v0.2.18
    Updating windows-targets v0.52.4 -> v0.52.5
    Updating windows_aarch64_gnullvm v0.52.4 -> v0.52.5
    Updating windows_aarch64_msvc v0.52.4 -> v0.52.5
    Updating windows_i686_gnu v0.52.4 -> v0.52.5
      Adding windows_i686_gnullvm v0.52.5
    Updating windows_i686_msvc v0.52.4 -> v0.52.5
    Updating windows_x86_64_gnu v0.52.4 -> v0.52.5
    Updating windows_x86_64_gnullvm v0.52.4 -> v0.52.5
    Updating windows_x86_64_msvc v0.52.4 -> v0.52.5
    Updating winnow v0.6.5 -> v0.6.6
```
@josecelano
Merge torrust#793: Update dependencies
52b7e3a 
5fa01e7 chore(deps): update dependencies (Jose Celano)

Pull request description:

  ```console
      Updating crates.io index
      Updating allocator-api2 v0.2.16 -> v0.2.18
      Updating anyhow v1.0.81 -> v1.0.82
      Updating async-channel v2.2.0 -> v2.2.1
      Updating async-executor v1.10.0 -> v1.11.0
      Updating async-trait v0.1.79 -> v0.1.80
      Updating axum-client-ip v0.5.1 -> v0.6.0
      Updating cc v1.0.91 -> v1.0.94
      Updating either v1.10.0 -> v1.11.0
      Updating encoding_rs v0.8.33 -> v0.8.34
      Updating jobserver v0.1.28 -> v0.1.30
      Updating proc-macro2 v1.0.79 -> v1.0.80
      Updating quote v1.0.35 -> v1.0.36
      Updating rstest v0.18.2 -> v0.19.0
      Updating rstest_macros v0.18.2 -> v0.19.0
      Updating serde_repr v0.1.18 -> v0.1.19
      Updating syn v2.0.58 -> v2.0.59
      Updating time v0.3.34 -> v0.3.36
      Updating time-macros v0.2.17 -> v0.2.18
      Updating windows-targets v0.52.4 -> v0.52.5
      Updating windows_aarch64_gnullvm v0.52.4 -> v0.52.5
      Updating windows_aarch64_msvc v0.52.4 -> v0.52.5
      Updating windows_i686_gnu v0.52.4 -> v0.52.5
        Adding windows_i686_gnullvm v0.52.5
      Updating windows_i686_msvc v0.52.4 -> v0.52.5
      Updating windows_x86_64_gnu v0.52.4 -> v0.52.5
      Updating windows_x86_64_gnullvm v0.52.4 -> v0.52.5
      Updating windows_x86_64_msvc v0.52.4 -> v0.52.5
      Updating winnow v0.6.5 -> v0.6.6
  ```

ACKs for top commit:
  josecelano:
    ACK 5fa01e7

Tree-SHA512: 363ebd95fe796c0ca0b1ec778a8f70b7eb3ca5bda3fb55883f525a910c9f4425b6a35c605b121b944a80e808189064d963b5490eb2883b7279f8193be92302ff
@josecelano
refactor: extract PeerList
4a567cd 
Extract a type for a collection of peers.

The performance adter the exatract is similar:

```output
Requests out: 415067.21/second
Responses in: 369397.08/second
  - Connect responses:  183049.81
  - Announce responses: 182717.15
  - Scrape responses:   3630.12
  - Error responses:    0.00
Peers per announce response: 0.00
Announce responses per info hash:
  - p10: 1
  - p25: 1
  - p50: 1
  - p75: 1
  - p90: 2
  - p95: 3
  - p99: 104
  - p99.9: 297
  - p100: 375
```
@josecelano
refactor: rename field from peers to swarm
922afda
@josecelano
refactor: extract mod peer_list
42f1b30
mario-nt and others added 25 commits June 20, 2024 15:03
@mario-nt @da2ce7
refactor: [torrust#670] new mod for responses logic and refactors to …
19a3c5e 
…json serialization trait
@mario-nt @da2ce7
refactor: [torrust#670] changed DTOs and variable names
5299b35
@mario-nt @da2ce7
refactor:[torrust#670] fix clippy errors
4b1bcd5
@josecelano @da2ce7
docs: add benchmarking to torrent repo README
c171a4a
@josecelano @da2ce7
chore(deps): add cargo dependency tracing
68d8344 
We will move from `log` to `tracing` crate.
@josecelano @da2ce7
refactor: [torrust#884] move from log to tracing crate
307801f
@josecelano @da2ce7
refactor: [torrust#884] move from log to tracing crate
4e7b773
@josecelano @da2ce7
test: [torrust#884] add test for parsing array of services from app logs
08233b0 
The tracker can run multiple UDP or HTTP trackers. We parse the services
from app output but there was not test for multiple services of the same
type (UDP or HTTP tracker).
@josecelano @da2ce7
chore(deps): remove unused dependencies log and fern
95e0d13 
We have moved from crate log to tracing.
@josecelano @da2ce7
refactor: [torrust#659] use clap and anyhow in E2E test runner
1816a38 
You can execute the E2E runner with:

```bash
cargo run --bin e2e_tests_runner -- --config-toml-path "./share/default/config/tracker.e2e.container.sqlite3.toml"
```

Or:

```bash
TORRUST_TRACKER_CONFIG_TOML_PATH="./share/default/config/tracker.e2e.container.sqlite3.toml" cargo run --bin e2e_tests_runner
```

Or:

```bash
TORRUST_TRACKER_CONFIG_TOML=$(cat "./share/default/config/tracker.e2e.container.sqlite3.toml") cargo run --bin e2e_tests_runner
```
@josecelano @da2ce7
docs: [torrust#770] update benchmarking docs
b29efb1 
We are now using criterion to benchmark the torrent repository
implementations.
@josecelano @da2ce7
fix: [torrust#893] enable color in logs
3f2c696
@josecelano @da2ce7
chore(deps): add cargo dependency regex
555de77 
To parse cargo logs.
@josecelano @da2ce7
fix: [torrust#893] enable color for logs
dd10c9a 
It was disabled becuase parsing lgos to extract the services URLs was
not working due to hidden color chars.

This changes the parser to ignore color chars.
@josecelano @da2ce7
fix: [torrust#898] docker build error: failed to load bitcode of modu…
4f2e23b 
…le criterion

Command:

```
docker build --target release --tag torrust-tracker:release --file Containerfile .
```

Error:

```s
 => ERROR [build 3/3] RUN cargo nextest archive --tests --benches --examples --workspace --all-targets --all-features --archive-  56.8s
------
 > [build 3/3] RUN cargo nextest archive --tests --benches --examples --workspace --all-targets --all-features --archive-file /build/torrust-tracker.tar.zst --release:
0.674    Compiling torrust-tracker-located-error v3.0.0-alpha.12-develop (/build/src/packages/located-error)
0.675    Compiling torrust-tracker-primitives v3.0.0-alpha.12-develop (/build/src/packages/primitives)
0.679    Compiling torrust-tracker-contrib-bencode v3.0.0-alpha.12-develop (/build/src/contrib/bencode)
0.763    Compiling torrust-tracker-configuration v3.0.0-alpha.12-develop (/build/src/packages/configuration)
0.763    Compiling torrust-tracker-clock v3.0.0-alpha.12-develop (/build/src/packages/clock)
0.936    Compiling torrust-tracker-torrent-repository v3.0.0-alpha.12-develop (/build/src/packages/torrent-repository)
0.936    Compiling torrust-tracker-test-helpers v3.0.0-alpha.12-develop (/build/src/packages/test-helpers)
1.181    Compiling torrust-tracker v3.0.0-alpha.12-develop (/build/src)
1.891 warning: Invalid value (Producer: 'LLVM18.1.7-rust-1.79.0-stable' Reader: 'LLVM 18.1.7-rust-1.79.0-stable')
1.891
1.891 error: failed to load bitcode of module "criterion-af9a3f7183f1573d.criterion.b69900c842eb33fa-cgu.08.rcgu.o":
1.891
1.991 warning: `torrust-tracker-contrib-bencode` (bench "bencode_benchmark") generated 1 warning
1.991 error: could not compile `torrust-tracker-contrib-bencode` (bench "bencode_benchmark") due to 1 previous error; 1 warning emitted
1.991 warning: build failed, waiting for other jobs to finish...
3.936 warning: `torrust-tracker-torrent-repository` (bench "repository_benchmark") generated 1 warning (1 duplicate)
3.936 error: could not compile `torrust-tracker-torrent-repository` (bench "repository_benchmark") due to 1 previous error; 1 warning emitted
56.80 error: command `/usr/local/rustup/toolchains/1.79.0-x86_64-unknown-linux-gnu/bin/cargo test --no-run --message-format json-render-diagnostics --workspace --examples --tests --benches --all-targets --all-features --release` exited with code 101
------
Containerfile:61
--------------------
  59 |     WORKDIR /build/src
  60 |     COPY . /build/src
  61 | >>> RUN cargo nextest archive --tests --benches --examples --workspace --all-targets --all-features --archive-file /build/torrust-tracker.tar.zst --release
  62 |
  63 |
--------------------
ERROR: failed to solve: process "/bin/sh -c cargo nextest archive --tests --benches --examples --workspace --all-targets --all-features --archive-file /build/torrust-tracker.tar.zst --release" did not complete successfully: exit code: 101
```

- Docker: version 25.0.2, build 29cf629
- Rust: nightly-x86_64-unknown-linux-gnu (default). rustc 1.81.0-nightly (d7f6ebace 2024-06-16)
@josecelano @da2ce7
feat!: [torrust#878] extract logging and core section in toml config …
73dec71 
…files
@josecelano @da2ce7
feat!: [torrust#878] make log_level config value mandatory
37a09aa 
Althought, it has a default value `info` so you can omit it in the TOML
config file.
@josecelano @da2ce7
feat!: [torrust#878] extract database section in core config section
4a39d65
@josecelano @da2ce7
feat!: [torrust#878] extract net section in core config section
36c7ed1
@josecelano @da2ce7
feat!: [torrust#878] extract announce_policy section in core config s…
063caf3 
…ection
@josecelano @da2ce7
feat!: [torrust#878] extract tracker_policy section in core config se…
2708f5d 
…ction
@josecelano @da2ce7
feat: [torrust#878] extract tsl_config in toml config
6d07cf4 
TSL configuration for HTTP trackers and the Tracker API is still
optional. However, when it's provided is enabled. The `ssl_enabled`
field was removed. You can remove the whole `tsl_config` to disable TSL.

If you want to kee a copy in the TOML file you can just comment the
lines.

```toml
[[http_trackers]]
...

[http_trackers.tsl_config]
ssl_cert_path = "./storage/tracker/lib/tls/localhost.crt"
ssl_key_path = "./storage/tracker/lib/tls/localhost.key"

[http_api]
...

[http_api.tsl_config]
ssl_cert_path = "./storage/tracker/lib/tls/localhost.crt"
ssl_key_path = "./storage/tracker/lib/tls/localhost.key"
```
@josecelano @da2ce7
feat!: [torrust#878] remove enabled fields in config
855bd4f 
By default all services are disabled. If the service section is missing
in the TOML config file it means the service is disabled.

From:

```toml
[[udp_trackers]]
enabled = false
bind_address = "0.0.0.0:6969"
```

To:

```toml
```

The `http_api` section has been disabled by default becuase there is no
way to override it to disable it, if it's enabled by default. You nned
to explicitly enabled the API now.
@da2ce7
Merge branch 'dev_1' into develop
27ee9aa
@da2ce7
dev: clients checker cleanup
7e8f05b
@da2ce7 da2ce7 requested a review from a team as a code owner June 26, 2024 14:12
@da2ce7 da2ce7 mentioned this pull request Jun 28, 2024
Merged
josecelano added a commit that referenced this pull request Jul 1, 2024
@josecelano
Merge #923: Client Work: Improve Error Enums and Some Basic Cleanup
e339c38 
c202db7 dev: tracker client error enums (Cameron Garnham)
988f1c7 dev: add vscode 'code-workspace' to git ignore file (Cameron Garnham)

Pull request description:

  most work extracted from #765

Top commit has no ACKs.

Tree-SHA512: bfa36bb3849b4694adc812c9ec2f401a14c0f4fabfd201abe88c43cfc215493ebbe7a6847fb075456ad85d48bf5d2b7a91b2af65ef5b05805a5f7fc5758ccc75
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@josecelano josecelano josecelano left review comments

Assignees

@josecelano josecelano

Labels
Needs Rebase Base Branch has Incompatibilities
Projects
None yet
Milestone
No milestone
5 participants
@da2ce7 @josecelano @ngthhu @GGLinnk @mario-nt