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.

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

Fix and speed up health checks #7313

Merged
merged 19 commits into from Mar 14, 2023
Merged

Conversation

kozlovsky
Copy link
Collaborator

@kozlovsky kozlovsky commented Mar 8, 2023

This PR fixes #7287. There were several reasons for the bug:

  1. When no trackers for a specific infohash were able to provide health info, the resulting health record with zero seeders and zero leechers had an incorrect last_check timestamp value of 0.
  2. The checks in HealthInfo.should_update() were also incorrect, and the logic sometimes decided to keep the previous health info and not replace it with the new health info.
  3. In such situations, due to the third bug, the notification was not sent to the GUI; as a result, the GUI status for the torrent was stuck in the "Checking..." state indefinitely.

Also, this PR fixes the long checking time by updating the torrent health immediately. Previously, TorrentChecker aggregated all results from all possible trackers before providing the new health info. Some trackers return result almost immediately, while other wait for up to 20 seconds. Because of this, all health checks previously took around 20 seconds, and now in many cases, they are updated almost immediately and then corrected with results from additional trackers if necessary.

@kozlovsky kozlovsky requested a review from a team as a code owner March 8, 2023 10:16
@kozlovsky kozlovsky requested review from drew2a and removed request for a team March 8, 2023 10:16
@kozlovsky kozlovsky marked this pull request as draft March 8, 2023 13:39
@kozlovsky kozlovsky force-pushed the fix/health_check branch 2 times, most recently from 8f76c31 to 4bb25e6 Compare March 8, 2023 15:24
Copy link
Collaborator

@drew2a drew2a left a comment

Choose a reason for hiding this comment

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

Overall, this PR offers a good fix for the health check, but let's try to make the should_update function simpler :)

@kozlovsky kozlovsky force-pushed the fix/health_check branch 2 times, most recently from 6963c1b to 1393a70 Compare March 9, 2023 16:03
@kozlovsky kozlovsky force-pushed the fix/health_check branch 3 times, most recently from fd9b6a3 to 9307a1d Compare March 10, 2023 08:31
@drew2a drew2a self-requested a review March 10, 2023 14:37
Copy link
Collaborator

@drew2a drew2a left a comment

Choose a reason for hiding this comment

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

The current version of PR looks good to me.

codecov/patch issues should be addressed (maybe not all of them, but as much as possible).

@kozlovsky kozlovsky force-pushed the fix/health_check branch 4 times, most recently from 99311dc to 5a359a5 Compare March 14, 2023 14:08
@kozlovsky kozlovsky changed the title [WIP] Fixing health checks Fix and speed up health checks Mar 14, 2023
@kozlovsky kozlovsky marked this pull request as ready for review March 14, 2023 15:03
Copy link
Collaborator

@drew2a drew2a left a comment

Choose a reason for hiding this comment

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

👍

@kozlovsky kozlovsky merged commit 1fa206f into Tribler:main Mar 14, 2023
28 checks passed
@kozlovsky kozlovsky deleted the fix/health_check branch March 14, 2023 15:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[main] Recheck torrent health is broken
2 participants