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

Add queue for multithread processing #7417

Merged
merged 2 commits into from
May 12, 2023
Merged

Conversation

drew2a
Copy link
Contributor

@drew2a drew2a commented May 11, 2023

This is a continuation of the previous PR #7413 and the final fix for the deadlock problem.

It adds a thread-safe queue that is used for processing incoming MDS entities, which are created in a different thread.
Additionally, it removes unnecessary logic from Endpoints that perform writes to both databases (KnowledgeDB and MetadataDB).

As a drawback, this PR reduces the speed of tag extraction, as they are now not performed immediately upon entity addition to the MetadataDB. Given that tags aren't directly used in the current release and we plan to remove MDS in upcoming releases, this drawback is acceptable.

@drew2a drew2a self-assigned this May 11, 2023
@drew2a drew2a added this to the 7.13.0 milestone May 11, 2023
@drew2a drew2a marked this pull request as ready for review May 11, 2023 09:48
@drew2a drew2a requested review from a team and kozlovsky and removed request for a team May 11, 2023 09:48
Copy link
Contributor

@kozlovsky kozlovsky left a comment

Choose a reason for hiding this comment

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

It seems that with this change KnowledgeRulesProcessor performs better.

But process_batch still updates two databases - MetadataDB by updating torrent.tag_processor_version and KnowledgeDB by processing the torrent's title. In some next PR, it may be beneficial to modify process_batch so it starts adding titles to the queue instead of processing them directly; after that, updating both databases will be completely separated.

@drew2a
Copy link
Contributor Author

drew2a commented May 12, 2023

But process_batch still updates two databases - MetadataDB by updating torrent.tag_processor_version and KnowledgeDB by processing the torrent's title.

Good catch, I overlooked that.
I've removed this line as it's not necessary. torrent.tag_processor_version should be set in all cases except process_batch (as it processes the MDB sequentially).

@drew2a drew2a merged commit 8e6e266 into Tribler:release/7.13 May 12, 2023
@drew2a drew2a deleted the fix/krp_queue branch May 12, 2023 10:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants