[checkmk-commits] Make WATO's exclusive lock a context manager.

Module: check_mk Branch: master Commit: 2cc032a4cf04f97ab65b3e2df692a8103f249b9c URL: http://git.mathias-kettner.de/git/?p=check_mk.git;a=commit;h=2cc032a4cf04f9… Author: Sven Panne <sp(a)mathias-kettner.de> Date: Thu Jan 10 08:48:25 2019 +0100 Make WATO's exclusive lock a context manager. There are a few very obscure unbalanced lock/unlock call sites left, we should *really* fix these: Locking must be done in a balanced way locally, otherwise hard-to-find bugs are basically guaranteed.... Change-Id: Ia6a90eee5ba329e0b49512d2fd5ab2d1c710357b --- cmk/gui/wato/__init__.py | 22 ++++----- cmk/gui/wato/pages/bulk_discovery.py | 12 ++--- cmk/gui/wato/pages/parentscan.py | 9 ++-- cmk/gui/watolib.py | 86 ++++++++++++++++++++---------------- 4 files changed, 62 insertions(+), 67 deletions(-) diff --git a/cmk/gui/wato/__init__.py b/cmk/gui/wato/__init__.py index 7bd898a..77ef646 100644 --- a/cmk/gui/wato/__init__.py +++ b/cmk/gui/wato/__init__.py @@ -868,23 +868,19 @@ def add_scanned_hosts_to_folder(folder, found): if not watolib.Host.host_exists(host_name): entries.append((host_name, attrs, None)) - watolib.lock_exclusive() - folder.create_hosts(entries) - folder.save() - watolib.unlock_exclusive() + with watolib.exclusive_lock(): + folder.create_hosts(entries) + folder.save() def save_network_scan_result(folder, result): # Reload the folder, lock WATO before to protect against concurrency problems. - watolib.lock_exclusive() - - # A user might have changed the folder somehow since starting the scan. Load the - # folder again to get the current state. - write_folder = watolib.Folder.folder(folder.path()) - write_folder.set_attribute("network_scan_result", result) - write_folder.save() - - watolib.unlock_exclusive() + with watolib.exclusive_lock(): + # A user might have changed the folder somehow since starting the scan. Load the + # folder again to get the current state. + write_folder = watolib.Folder.folder(folder.path()) + write_folder.set_attribute("network_scan_result", result) + write_folder.save() #. diff --git a/cmk/gui/wato/pages/bulk_discovery.py b/cmk/gui/wato/pages/bulk_discovery.py index edf971f..8064f01 100644 --- a/cmk/gui/wato/pages/bulk_discovery.py +++ b/cmk/gui/wato/pages/bulk_discovery.py @@ -132,23 +132,17 @@ class BulkDiscoveryBackgroundJob(WatoBackgroundJob): return counts, failed_hosts def _process_discovery_results(self, task, job_interface, counts, failed_hosts): - try: - # The following code updates the host config. The progress from loading the WATO folder - # until it has been saved needs to be locked. - watolib.lock_exclusive() - + # The following code updates the host config. The progress from loading the WATO folder + # until it has been saved needs to be locked. + with watolib.exclusive_lock(): watolib.Folder.invalidate_caches() folder = watolib.Folder.folder(task.folder_path) - for hostname in task.host_names: self._process_service_counts_for_host(counts[hostname]) msg = self._process_discovery_result_for_host( folder.host(hostname), failed_hosts.get(hostname, False), counts[hostname]) job_interface.send_progress_update("%s: %s" % (hostname, msg)) - finally: - watolib.unlock_exclusive() - def _process_service_counts_for_host(self, host_counts): self._num_services_added += host_counts[0] self._num_services_removed += host_counts[1] diff --git a/cmk/gui/wato/pages/parentscan.py b/cmk/gui/wato/pages/parentscan.py index 93940bf..e7d5466 100644 --- a/cmk/gui/wato/pages/parentscan.py +++ b/cmk/gui/wato/pages/parentscan.py @@ -127,13 +127,10 @@ class ParentScanBackgroundJob(WatoBackgroundJob): state, skipped_gateways, error = gateways[0][1:] if state in ["direct", "root", "gateway"]: - try: - # The following code updates the host config. The progress from loading the WATO folder - # until it has been saved needs to be locked. - watolib.lock_exclusive() + # The following code updates the host config. The progress from loading the WATO folder + # until it has been saved needs to be locked. + with watolib.exclusive_lock(): self._configure_host_and_gateway(task, settings, state, gateway) - finally: - watolib.unlock_exclusive() else: self._logger.error(error) diff --git a/cmk/gui/watolib.py b/cmk/gui/watolib.py index fa67d43..455f03c 100644 --- a/cmk/gui/watolib.py +++ b/cmk/gui/watolib.py @@ -46,6 +46,7 @@ import abc import ast import base64 import cStringIO +from contextlib import contextmanager import copy import glob from hashlib import sha256 @@ -236,16 +237,16 @@ def init_wato_datastructures(with_wato_lock=False): not _need_to_create_sample_config(): return - if with_wato_lock: - lock_exclusive() - - if not os.path.exists(ConfigDomainCACertificates.trusted_cas_file): - ConfigDomainCACertificates().activate() - - _create_sample_config() + def init(): + if not os.path.exists(ConfigDomainCACertificates.trusted_cas_file): + ConfigDomainCACertificates().activate() + _create_sample_config() if with_wato_lock: - unlock_exclusive() + with exclusive_lock(): + init() + else: + init() # TODO: Create a hook here and move CEE and other specific things away @@ -5459,35 +5460,33 @@ class ActivateChangesManager(ActivateChanges): # Lock WATO modifications during snapshot creation def _create_snapshots(self): - lock_exclusive() - - if not self._changes: - raise MKUserError(None, _("Currently there are no changes to activate.")) + with exclusive_lock(): + if not self._changes: + raise MKUserError(None, _("Currently there are no changes to activate.")) - if self._get_last_change_id() != self._activate_until: - raise MKUserError( - None, - _("Another change has been made in the meantime. Please review it " - "to ensure you also want to activate it now and start the " - "activation again.")) - - # Create (legacy) WATO config snapshot - start = time.time() - logger.debug("Snapshot creation started") - # TODO: Remove/Refactor once new changes mechanism has been implemented - # This single function is responsible for the slow activate changes (python tar packaging..) - create_snapshot(self._comment) - - work_dir = os.path.join(self.activation_tmp_base_dir, self._activation_id) - if cmk.is_managed_edition(): - import cmk.gui.cme.managed_snapshots as managed_snapshots - managed_snapshots.CMESnapshotManager( - work_dir, self._get_site_configurations()).generate_snapshots() - else: - self._generate_snapshots(work_dir) + if self._get_last_change_id() != self._activate_until: + raise MKUserError( + None, + _("Another change has been made in the meantime. Please review it " + "to ensure you also want to activate it now and start the " + "activation again.")) + + # Create (legacy) WATO config snapshot + start = time.time() + logger.debug("Snapshot creation started") + # TODO: Remove/Refactor once new changes mechanism has been implemented + # This single function is responsible for the slow activate changes (python tar packaging..) + create_snapshot(self._comment) + + work_dir = os.path.join(self.activation_tmp_base_dir, self._activation_id) + if cmk.is_managed_edition(): + import cmk.gui.cme.managed_snapshots as managed_snapshots + managed_snapshots.CMESnapshotManager( + work_dir, self._get_site_configurations()).generate_snapshots() + else: + self._generate_snapshots(work_dir) - logger.debug("Snapshot creation took %.4f" % (time.time() - start)) - unlock_exclusive() + logger.debug("Snapshot creation took %.4f" % (time.time() - start)) def _get_site_configurations(self): site_configurations = {} @@ -5664,8 +5663,7 @@ class ActivateChangesManager(ActivateChanges): # Cleanup stale activations? def _do_housekeeping(self): - lock_exclusive() - try: + with exclusive_lock(): for activation_id in self._existing_activation_ids(): # skip the current activation_id if self._activation_id == activation_id: @@ -5690,8 +5688,6 @@ class ActivateChangesManager(ActivateChanges): if delete: shutil.rmtree("%s/%s" % (ActivateChangesManager.activation_tmp_base_dir, activation_id)) - finally: - unlock_exclusive() def _existing_activation_ids(self): ids = [] @@ -10536,10 +10532,22 @@ def make_action_link(vars_): return folder_preserving_link(vars_ + [("_transid", html.transaction_manager.get())]) +@contextmanager +def exclusive_lock(): + path = cmk.utils.paths.default_config_dir + "/multisite.mk" + store.aquire_lock(path) + try: + yield + finally: + store.release_lock(path) + + +# TODO: Use exclusive_lock() and nuke this! def lock_exclusive(): store.aquire_lock(cmk.utils.paths.default_config_dir + "/multisite.mk") +# TODO: Use exclusive_lock() and nuke this! def unlock_exclusive(): store.release_lock(cmk.utils.paths.default_config_dir + "/multisite.mk")