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

Use separate misc table for the KnowledgeDB #7413

Merged
merged 2 commits into from May 11, 2023

Conversation

drew2a
Copy link
Collaborator

@drew2a drew2a commented May 9, 2023

This PR fixes deadlocks that happen with mutual updates from:

  1. KnowledgeRulesProcessor (write attempt to KnowledgeDB and MetadataDB in the process_batch function)
  2. MDS (write attempt to KnowledgeDB and MetadataDB in the constructor of TorrentMetadata)
graph TD;
    P-->|write: process_batch|K(KnowledgeDB);
    P[KnowledgeRulesProcessor]-->|write: process_batch|M(MetadataDB);


    T[TorrentMetadata]-->|write: __init__|M;
    T-->|write: __init__|K;

This situation arises when two factors coincide:

  1. Access to DBs occur simultaneously (this is true because the Metadata Store uses a different thread to perform operations with the DB).
  2. These accesses lock the same entity (this is true because we use SQLite and it locks the entire database, as mentioned below).

3.0 Locking
From the point of view of a single process, a database file can be in one of five locking states:

  • UNLOCKED No locks are held on the database. The database may be neither read nor written. Any internally cached data is considered suspect and subject to verification against the database file before being used. Other processes can read or write the database as their own locking states permit. This is the default state.
  • SHARED The database may be read but not written. Any number of processes can hold SHARED locks at the same time, hence there can be many simultaneous readers. But no other thread or process is allowed to write to the database file while one or more SHARED locks are active.
  • RESERVED A RESERVED lock means that the process is planning on writing to the database file at some point in the future but that it is currently just reading from the file. Only a single RESERVED lock may be active at one time, though multiple SHARED locks can coexist with a single RESERVED lock. RESERVED differs from PENDING in that new SHARED locks can be acquired while there is a RESERVED lock.
  • PENDING A PENDING lock means that the process holding the lock wants to write to the database as soon as possible and is just waiting on all current SHARED locks to clear so that it can get an EXCLUSIVE lock. No new SHARED locks are permitted against the database if a PENDING lock is active, though existing SHARED locks are allowed to continue.
  • EXCLUSIVE An EXCLUSIVE lock is needed in order to write to the database file. Only one EXCLUSIVE lock is allowed on the file and no other locks of any kind are allowed to coexist with an EXCLUSIVE lock. In order to maximize concurrency, SQLite works to minimize the amount of time that EXCLUSIVE locks are held.

This PR eliminates the write attempts from KnowledgeRulesProcessor to MetadataDB by beginning to utilize a separate misc table.

graph TD;
    
    P[KnowledgeRulesProcessor]-->|write: process_batch|K(KnowledgeDB);

    T[TorrentMetadata]-->|write: __init__|M(MetadataDB);
    T-->|write: __init__|K;

Please note that this PR does not entirely solve the deadlock problem. A comprehensive solution will involve one of the following:

  1. Merging KnowledgeDB and MetadataDB into a single database.
  2. Creating a single namespace that houses two different databases.
  3. Eliminating the notifications.new_torrent_metadata_created event.

Refs:

@drew2a drew2a marked this pull request as ready for review May 10, 2023 08:12
@drew2a drew2a requested a review from a team as a code owner May 10, 2023 08:12
@drew2a drew2a requested review from xoriole and kozlovsky and removed request for a team May 10, 2023 08:12
@drew2a drew2a added this to the 7.13.0 milestone May 10, 2023
@drew2a drew2a self-assigned this May 10, 2023
Copy link
Collaborator

@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 is an important PR that should greatly improve Tribler stability.

The only thing that concerns me a bit is that old values of RULES_PROCESSOR_VERSION and LAST_PROCESSED_TORRENT_ID are not copied from the MetadataDB, and the processing of torrents is started again from scratch.

But I think the current logic of KnowledgeRulesProcessor should handle this well.

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.

None yet

2 participants