Remove exclusive transactions #23
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…g functions simultaneously
I don't see a reason for the exclusive transaction. It should still work if someone modifies the task or schedule info. The task must be started outside the transaction anyway.
While this is not the same as SQLite (which prevents other processes from modifying the db during the transaction) it is still better than the current exclusive transaction which is not really doing anything.
Multiple transactions are used, so other processes can already affect the database, for example between chunks of NVTs or between inserting the NVTs and the preferences.
This was originally added for SQLite. It's safe to allow access during a VACUUM in Postgres.
These three just modify existing values. No need for exclusivity.
These rebuild options are used rarely, and there are no serious side effects of the data being accessed while it is being built, so no need for exclusivity.
I don't see harm in allowing other processes to read the user data while the delete transaction is busy. There's the potential danger that another process starts a task that has been deleted already in the transaction, but this was already not working with our Postgres exclusive transaction, so it needs an alternate solution.
The issue here is that exclusivity is needed to prevent other processes from resuming the report's task between the time we check that the report is not active and the time that we commit the deleted report. Locking the table, instead of trying to lock the entire database, seems like the best option at the moment.
Similarly to 44d7444, lock reports table when auto deleting all reports.
Like 44d7444, lock reports table when deleting tasks.
Lock tasks table, to prevent other processes from starting the task concurrently. Move the report and event work out of the locked transaction, to prevent deadlock and because there's a lot of work done there.
Because a config may only be modified when not in use, this does not affect tasks. With Postgres at worst a second concurrent modification will have to wait for the first to commit. Two processes modifying the same config is pretty rare anyway.
Use a lockfile for each type of process instead of using exclusive locks, to prevent certain types of processes from running simultaneously. The three types of processes are the main process, option (helper) processes and the --migrate process. Use a fourth lock, gvm-checking, to prevent races when checking the three lock types, and also to allow the main process exclusive access when checking the db initially.
Exclusivity is now provided via file locks in process startup.
timopollmeier
approved these changes
Mar 8, 2018
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This removes all exclusive transactions used by Postgres. The implementation of sql_begin_exclusive was not actually providing exclusivity for Postgres.
In most cases I figured a regular transaction would work OK. In some cases I've provided alternative locking, like a table lock, or file locks outside the db.
As a result on the SQLite side only 4 exclusive transactions remain. I didn't think these cases were safe, but didn't have time to go too deep into them.