refactor: port udp to aquatic_udp_protocol#6
Merged
mickvandijke merged 1 commit intomainfrom Jan 29, 2022
Merged
Conversation
josecelano
added a commit
to josecelano/torrust-tracker
that referenced
this pull request
Apr 30, 2026
…rdown The schema teardown for both the SQLite and MySQL drivers was missing a DROP TABLE for torrent_aggregate_metrics, leaving the table behind after rollback. Add the missing statement so test/setup teardown leaves a clean schema. Addresses Copilot review items torrust#5 and torrust#6 on PR torrust#1718.
josecelano
added a commit
that referenced
this pull request
Apr 30, 2026
…sqlx 0.8 66f1246 test(tracker-core): wait for downloads==1 instead of asserting on intermediate state (Jose Celano) fd5be6d fix(tracker-core): accept no-op SQLite upserts as success (Jose Celano) c1cce75 docs(bootstrap): restore #[must_use] and fix typo on setup() (Jose Celano) 78c63a4 test(tracker-core): bound persistence retry with timeout (Jose Celano) 53297d3 fix(tracker-core): build SQLite connection options from filesystem path (Jose Celano) 61e4427 fix(tracker-core): reject negative valid_until and propagate row-count overflow (Jose Celano) 2c19024 fix(tracker-core): drop torrent_aggregate_metrics table on schema teardown (Jose Celano) 5512ac3 fix(tracker-core): accept no-op MySQL upserts as success (Jose Celano) 0990113 docs(agents): clarify how Committer handles split commits with cspell changes (Jose Celano) 6510da5 docs(tracker-core): record 2026-04-30 persistence benchmark run (Jose Celano) 42e3e5d test(tracker-core): wait for mysql readiness in persistence benchmark (Jose Celano) a4dbc63 refactor(tracker-core): remove legacy r2d2 persistence surface and temporary sqlx staging tree (Jose Celano) 8c07450 docs(1525-05): track remaining cleanup and validation work (Jose Celano) 93e25f3 refactor: propagate async initialization end-to-end removing all run_on_runtime bridges (Jose Celano) 18b3b0f refactor(tracker-core): complete async sqlx switch and add task 5 cleanup (Jose Celano) aff5bd2 refactor(tracker-core): remove lazy schema checks from sqlx drivers (Jose Celano) ed0cef1 docs(issues): keep eager schema initialization in 1525-05 (Jose Celano) 76594b7 feat(tracker-core): add async sqlx mysql driver in parallel module (Jose Celano) 2fb25a1 docs(skills): require opening PRs in upstream repo (Jose Celano) 8cc5deb feat(tracker-core): add async sqlx sqlite driver in parallel module (Jose Celano) 91547d5 feat(tracker-core): add sqlx 0.8 infrastructure and async error conversions (Jose Celano) 333f6ef docs(issues): correct 1717 spec to match current codebase structure (Jose Celano) 8f10e39 docs(issues): rename 1525-05 spec to include GitHub issue number 1717 (Jose Celano) Pull request description: Closes #1717 Migrates the SQLite and MySQL persistence drivers from the sync r2d2 stack to async sqlx 0.8. ## Current status Structural migration and post-migration benchmark validation are complete. Completed on this branch: - SQLite and MySQL driver implementations use `sqlx` pools and async trait methods. - Schema initialization remains eager via `initialize_database()`. - Schema creation still uses raw `sqlx::query()` DDL; `sqlx::migrate!()` is not introduced here. - Sync-to-async runtime bridge helpers have been removed; async initialization is propagated through all current call paths. - The temporary `packages/tracker-core/src/databases/sqlx/` staging tree has been deleted; the canonical `databases/driver/` and `databases/traits/` directories are now the single persistence surface. - Legacy dependencies (`r2d2`, `r2d2_sqlite`, `r2d2_mysql`) have been removed from `packages/tracker-core/Cargo.toml`. - Legacy compatibility code has been removed: the `Error::ConnectionPool` variant and all `From` impls for `r2d2::Error`, `rusqlite::Error`, `mysql::Error`, and `mysql::UrlError` are gone from `databases/error.rs`. The `From<rusqlite::Error>` impl in `authentication/key/mod.rs` was replaced with `From<sqlx::Error>`. - Stale `r2d2_*` references in driver doc comments have been refreshed to describe the sqlx-based pools. - Validation passing: `cargo machete`, `linter all`, doc tests, and `cargo test --workspace --all-targets`. - Persistence benchmarks re-run and compared against the 2026-04-28 baseline. Report: [`packages/tracker-core/docs/benchmarking/runs/2026-04-30/REPORT.md`](packages/tracker-core/docs/benchmarking/runs/2026-04-30/REPORT.md). No regression detected: MySQL totals are ~13–16% faster (mysql 8.4: 7381 → 6231 ms; mysql 8.0: 7633 → 6678 ms); SQLite totals shift within expected per-run jitter on the 100-op suite. See `docs/issues/1717-1525-05-migrate-sqlite-and-mysql-to-sqlx.md` for the full task breakdown and progress review. ## 2026-04-30 update - **Task 7 (persistence benchmarking) is done.** See the report linked above. - **Bench harness change** (`42e3e5df`): the `persistence_benchmark` MySQL setup now waits for the second `ready for connections` line on the container's stderr (the TCP listener, not the local socket) and then pings `SELECT 1` in a 30 × 500 ms retry loop before initializing the schema. This is needed because `sqlx` does not retry the first `connect()` (unlike the old r2d2 pool), so the harness must guarantee MySQL is accepting TCP connections before the benchmark proceeds. - **Production-code follow-up** (`6510da53` / related): expanded the `# Panics` Rustdoc section of `bittorrent_tracker_core::databases::setup::initialize_database` to document that it will panic if the database is not yet accepting connections, or on any other underlying `sqlx::Error`. This makes the new `sqlx`-based connect-without-retry behavior explicit to callers. ## 2026-04-30 update — Copilot review addressed Copilot performed an automated review on this PR (15 inline comments). All actionable items have been resolved across 6 GPG-signed commits (`5512ac30..c1cce75`); 1 item was already obsolete. | Commit | Subject | Addresses | |---|---|---| | `5512ac30` | fix(tracker-core): accept no-op MySQL upserts as success | #3, #4 | | `2c190249` | fix(tracker-core): drop torrent_aggregate_metrics table on schema teardown | #5, #6 | | `61e44277` | fix(tracker-core): reject negative valid_until and propagate row-count overflow | #8, #11, #12, #13, #14, #15 | | `53297d3e` | fix(tracker-core): build SQLite connection options from filesystem path | #10 | | `78c63a4d` | test(tracker-core): bound persistence retry with timeout | #1 | | `c1cce750` | docs(bootstrap): restore #[must_use] and fix typo on setup() | #2, #9 | Highlights: - **MySQL upsert semantics**: removed the `rows_affected() == 0` check on `INSERT … ON DUPLICATE KEY UPDATE` paths — a no-op update legitimately reports 0 affected rows and was being misclassified as `InsertFailed`. - **Data integrity**: negative `valid_until` timestamps now return `Error::MalformedDatabaseRecord` instead of being silently flipped positive via `unsigned_abs()`. Row-count → `usize` conversions now propagate errors instead of `unwrap_or(0)`. - **SQLite path handling**: switched from `SqliteConnectOptions::from_str("sqlite://{path}")` to `SqliteConnectOptions::new().filename(path)` so relative paths like `./storage/...` aren't reinterpreted as URL authority. - **Schema teardown**: `drop_database_tables()` in both drivers now drops the `torrent_aggregate_metrics` table that was being created but never cleaned up. - **Test determinism**: replaced the fixed-iteration `yield_now()` retry loop in `it_should_persist_the_number_of_completed_peers_for_each_torrent_into_the_database` with `tokio::time::timeout(5s, ...)` + `sleep(50ms)`. Item #7 (sqlx subdir not wired in) was already resolved in commit `a4dbc63a` earlier in this PR. Validation: `linter all` exit 0, full workspace test suite passing, MySQL-gated tests passing. ACKs for top commit: josecelano: ACK 66f1246 Tree-SHA512: 3c13001f502ecc00579896fe7297981241b5632e263eeb913e542658ec581de28d3ccd2ecf056629e817fd39bb6aa5d36d453b9d0bb2998fabd5a608bb17c47b
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.