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

WIP: Move long-running (db) local torrents check to thread #7796

Closed
wants to merge 1 commit into from

Conversation

qstokkink
Copy link
Contributor

@qstokkink qstokkink commented Jan 4, 2024

Fixes #7784 as discussed on the issue.

The check_torrents_in_user_channel has almost the exact same code as check_local_torrents. So, I also gave that the same treatment.


Restarts on this PR:

  • PR / mac / build: build timed out.

@qstokkink qstokkink marked this pull request as ready for review January 4, 2024 12:50
@qstokkink qstokkink requested a review from a team as a code owner January 4, 2024 12:50
@qstokkink qstokkink requested review from egbertbouman and removed request for a team January 4, 2024 12:50
@qstokkink qstokkink changed the title WIP: Move long-running (db) local torrents check to thread READY: Move long-running (db) local torrents check to thread Jan 4, 2024
@qstokkink qstokkink changed the title READY: Move long-running (db) local torrents check to thread WIP: Move long-running (db) local torrents check to thread Jan 4, 2024
@qstokkink qstokkink force-pushed the upd_chklocaltrts branch 4 times, most recently from 8212cdc to 8dffb8e Compare January 4, 2024 13:55
@qstokkink qstokkink changed the title WIP: Move long-running (db) local torrents check to thread READY: Move long-running (db) local torrents check to thread Jan 4, 2024
:type torrents: list[TorrentState]
"""
for t in torrents:
loop.call_soon_threadsafe(self.register_anonymous_task, f"Check health {t.infohash}",
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure if this is intentional or not, but I'm guessing check_torrent_health is still doing database stuff on the main thread (since that's where the only running eventloop is).

If it is intentional, you might as well keep the check functions on the main thread, and schedule the torrents_to_check/torrents_to_check_in_user_channel using run_threaded

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This seems like it would necessitate a major refactoring of the torrent checker to solve sanely then. Secondly, I think that this refactoring would overlap too much with #7328.

Taking that into account, I'll close this PR in favor of another (future) more thorough refactoring PR.

"""
loop = loop or get_running_loop()
selected_torrents = self.torrents_to_check()
Copy link
Member

Choose a reason for hiding this comment

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

You may need to call db.disconnect after torrents_to_check/torrents_to_check_in_user_channel since they're not running on the main thread anymore.

@qstokkink qstokkink changed the title READY: Move long-running (db) local torrents check to thread WIP: Move long-running (db) local torrents check to thread Jan 5, 2024
@qstokkink qstokkink marked this pull request as draft January 5, 2024 07:30
@qstokkink qstokkink closed this Jan 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants