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

Health checks seem broken #8159

Closed
qstokkink opened this issue Sep 17, 2024 · 2 comments · Fixed by #8165
Closed

Health checks seem broken #8159

qstokkink opened this issue Sep 17, 2024 · 2 comments · Fixed by #8165
Assignees
Milestone

Comments

@qstokkink
Copy link
Contributor

After clicking the health of all the torrents in my search results I get a lot of seemingly dead torrents:

screenshot1

However, after waiting a few seconds and clicking them all again, the torrents come back alive:

screenshot2

@qstokkink
Copy link
Contributor Author

To my surprise, if I change the following line

health = aggregate_responses_for_infohash(infohash, cast(List[TrackerResponse], successful_responses))

to

health = aggregate_responses_for_infohash(infohash, cast(List[TrackerResponse], []))

everything starts working (I still have no idea why this could be).

@qstokkink qstokkink added this to the 8.0.0 milestone Sep 19, 2024
@qstokkink
Copy link
Contributor Author

qstokkink commented Sep 19, 2024

I think this logic is wrong. Consider

health = aggregate_responses_for_infohash(infohash, cast(List[TrackerResponse], successful_responses))
if health.last_check == 0: # if not zero, was already updated in get_tracker_response
health.last_check = int(time.time())
health.self_checked = True
self.update_torrent_health(health)
else:
# We don't need to store this in the db, but we still need to notify the GUI
self.notify(health)

def aggregate_responses_for_infohash(infohash: bytes, responses: List[TrackerResponse]) -> HealthInfo:
"""
Finds the "best" health info (with the max number of seeders) for a specified infohash.
"""
result = HealthInfo(infohash, last_check=0)
for response in responses:
for health in response.torrent_health_list:
if health.infohash == infohash and health > result:
result = health
return result

aggregate_responses_for_infohash takes the latest tracker responses and picks the most likely correct health. Then, this fresh valid tracker data is not stored in the database (???), only if there was no tracker data, with 0 seeders and 0 leechers, do we store the result (!?).

I think the check should be as follows:

if health.last_check == 0:
    self.notify(health)  # We don't need to store this in the db, but we still need to notify the GUI
else:
    self.update_torrent_health(health)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging a pull request may close this issue.

1 participant