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

Make class ConfigForm concurrency safe #4780

Open
8 tasks
nilmerg opened this issue May 10, 2022 · 0 comments
Open
8 tasks

Make class ConfigForm concurrency safe #4780

nilmerg opened this issue May 10, 2022 · 0 comments
Labels
area/configuration Affects the configuration

Comments

@nilmerg
Copy link
Member

nilmerg commented May 10, 2022

Preparations

  1. Checkout db: Add tables and models for config storage #4778 and base off a new branch from it.
  2. Apply the migration scripts for version 2.11 to the mysql and postgresql database.
  3. Checkout Select: Add support for locking clauses ipl-sql#55, you'll need it

Goal

If there are multiple users updating the same configuration at the same time, no change must be ignored or overridden without explicit user action.

Assuming there are two users:

User A User B
opens config
opens config
submits changes
submits changes
succeeds
fails

If the update fails, the user must be informed about it. In addition, the new configuration values should be shown in a message at the top of the form.

Tasks

  • Add a new validator to ConfigForm (You may need to extend method isValid() to achieve this as it should be a validator for the entire form)
  • Fetch the hash for the configuration's scope and compare it against the one sent in the form data (That's a task of Adjust ConfigForm to write configurations to database #4779)
    • Apply a locking clause to the query to ensure you'll get a result only if there's noone writing to it. (See FOR UPDATE)
  • If the hashes don't match
    • fetch the new config options from database
    • show a message at the top of the form ($this->warning) and list the new options there
    • forget the old hash and set the new hash so that the next submit includes it (Otherwise the validation fails again)
    • let the form validation fail

Hints

You can fake another user by running a transaction on the database in the console with a sleep in it:

begin;

update icingaweb_config_scope ... ;

do sleep(60);

commit;
@nilmerg nilmerg added the area/configuration Affects the configuration label May 10, 2022
@yhabteab yhabteab assigned yhabteab and unassigned yhabteab May 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/configuration Affects the configuration
Projects
None yet
Development

No branches or pull requests

2 participants