Tribler / tribler

Privacy enhanced BitTorrent client with P2P content discovery
https://www.tribler.org
GNU General Public License v3.0
4.86k stars 450 forks source link

Health checks seem broken #8159

Closed qstokkink closed 2 months ago

qstokkink commented 2 months ago

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 commented 2 months ago

To my surprise, if I change the following line

https://github.com/Tribler/tribler/blob/6a6c8e052e54dc8e987a548a990d284f591d47de/src/tribler/core/torrent_checker/torrent_checker.py#L356

to

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

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

qstokkink commented 2 months ago

I think this logic is wrong. Consider

https://github.com/Tribler/tribler/blob/6a6c8e052e54dc8e987a548a990d284f591d47de/src/tribler/core/torrent_checker/torrent_checker.py#L356-L363

https://github.com/Tribler/tribler/blob/6a6c8e052e54dc8e987a548a990d284f591d47de/src/tribler/core/torrent_checker/torrent_checker.py#L43-L52

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)