From ee45a2273b0b161ca0c619f7189702ec4845d63f Mon Sep 17 00:00:00 2001 From: Matyas Selmeci Date: Mon, 20 May 2024 13:48:55 -0500 Subject: [PATCH 1/4] Cache and measure the update of the Topology Git repo separately instead of having the vo data, projects data, mappings data, and resourcegroup data all pulling from the git repo when they get updated. While I'm at it, use time.monotonic() instead of time.time() for the cache time (which avoids issues if we run, say, ntpdate). --- src/webapp/common.py | 9 +++++++++ src/webapp/models.py | 36 +++++++++++++++++++++++++----------- 2 files changed, 34 insertions(+), 11 deletions(-) diff --git a/src/webapp/common.py b/src/webapp/common.py index d11c8e232..c64a40643 100644 --- a/src/webapp/common.py +++ b/src/webapp/common.py @@ -6,6 +6,7 @@ import re import subprocess import sys +import time from typing import Any, Dict, List, Union, AnyStr, NewType, TypeVar from functools import wraps @@ -394,6 +395,14 @@ def wrapped(): return wrapped +def get_timestamp(): + """Return a monotonic timestamp if available, otherwise a wall-clock timestamp.""" + if hasattr(time, "monotonic"): + return time.monotonic() + else: + return time.time() + + XROOTD_CACHE_SERVER = "XRootD cache server" XROOTD_ORIGIN_SERVER = "XRootD origin server" GRIDTYPE_1 = "OSG Production Resource" diff --git a/src/webapp/models.py b/src/webapp/models.py index 702cf7b24..5d3c0d38a 100644 --- a/src/webapp/models.py +++ b/src/webapp/models.py @@ -2,7 +2,6 @@ import datetime import logging import os -import time from typing import Dict, Set, List, Optional import yaml @@ -24,7 +23,7 @@ def time(self): from webapp import common, contacts_reader, ldap_data, mappings, project_reader, rg_reader, vo_reader -from webapp.common import readfile +from webapp.common import readfile, get_timestamp from webapp.contacts_reader import ContactsData from webapp.topology import Topology, Downtime from webapp.vos_data import VOsData @@ -33,6 +32,7 @@ def time(self): log = logging.getLogger(__name__) topology_update_summary = Summary('topology_update_seconds', 'Time spent updating the topology repo data') +topology_git_update_summary = Summary('topology_git_update_seconds', 'Time spent pulling/cloning the topology git repo') contact_update_summary = Summary('contact_update_seconds', 'Time spent updating the contact repo data') comanage_update_summary = Summary('comanage_update_seconds', 'Time spent updating the comanage LDAP data') ligo_update_summary = Summary('ligo_update_seconds', 'Time spent updating the LIGO LDAP data') @@ -52,16 +52,16 @@ def should_update(self): """Return True if we should update, either because we're past the next update time or because force_update is True. """ - return self.force_update or not self.data or time.time() > self.next_update + return self.force_update or not self.data or get_timestamp() > self.next_update def try_again(self): """Set the next update time to now + the retry delay.""" - self.next_update = time.time() + self.retry_delay + self.next_update = get_timestamp() + self.retry_delay def update(self, data): """Cache new data and set the next update time to now + the cache lifetime.""" self.data = data - self.timestamp = time.time() + self.timestamp = get_timestamp() self.next_update = self.timestamp + self.cache_lifetime self.force_update = False @@ -90,6 +90,7 @@ def __init__(self, config=None, strict=False): self.topology = CachedData(cache_lifetime=topology_cache_lifetime) self.vos_data = CachedData(cache_lifetime=topology_cache_lifetime) self.mappings = CachedData(cache_lifetime=topology_cache_lifetime) + self.topology_repo_stamp = CachedData(cache_lifetime=topology_cache_lifetime) self.topology_data_dir = config["TOPOLOGY_DATA_DIR"] self.topology_data_repo = config.get("TOPOLOGY_DATA_REPO", "") self.topology_data_branch = config.get("TOPOLOGY_DATA_BRANCH", "") @@ -153,6 +154,19 @@ def _update_topology_repo(self): return False return True + def maybe_update_topology_repo(self): + """Update the local git clone of the topology github repo if it hasn't + been updated recently (based on the cache time for self.topology_repo_stamp). + """ + if self.topology_repo_stamp.should_update(): + with topology_git_update_summary.time(): + ok = self._update_topology_repo() + if ok: + self.topology_repo_stamp.update(get_timestamp()) + else: + self.topology_repo_stamp.try_again() + return self.topology_repo_stamp.data + def _update_contacts_repo(self): if not self.config["NO_GIT"]: parent = os.path.dirname(self.config["CONTACT_DATA_DIR"]) @@ -297,11 +311,11 @@ def get_topology(self) -> Optional[Topology]: return self.topology.data - def update_topology(self): + def update_topology(self) -> None: """ - Update topology data + Update topology facility/site/ResourceGroup data """ - ok = self._update_topology_repo() + ok = self.maybe_update_topology_repo() if ok: try: self.topology.update(rg_reader.get_topology(self.topology_dir, self.get_contacts_data(), strict=self.strict)) @@ -320,7 +334,7 @@ def get_vos_data(self) -> Optional[VOsData]: """ if self.vos_data.should_update(): with topology_update_summary.time(): - ok = self._update_topology_repo() + ok = self.maybe_update_topology_repo() if ok: try: self.vos_data.update(vo_reader.get_vos_data(self.vos_dir, self.get_contacts_data(), strict=self.strict)) @@ -341,7 +355,7 @@ def get_projects(self) -> Optional[Dict]: """ if self.projects.should_update(): with topology_update_summary.time(): - ok = self._update_topology_repo() + ok = self.maybe_update_topology_repo() if ok: try: self.projects.update(project_reader.get_projects(self.projects_dir, strict=self.strict)) @@ -364,7 +378,7 @@ def get_mappings(self, strict=None) -> Optional[mappings.Mappings]: strict = self.strict if self.mappings.should_update(): with topology_update_summary.time(): - ok = self._update_topology_repo() + ok = self.maybe_update_topology_repo() if ok: try: self.mappings.update(mappings.get_mappings(indir=self.mappings_dir, strict=strict)) From fa54522346e73469b51c2cc59cb4a32cab69553a Mon Sep 17 00:00:00 2001 From: Matyas Selmeci Date: Mon, 20 May 2024 14:27:04 -0500 Subject: [PATCH 2/4] Fix return type of maybe_update_topology_repo() --- src/webapp/models.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/webapp/models.py b/src/webapp/models.py index 5d3c0d38a..8a5ebb7c7 100644 --- a/src/webapp/models.py +++ b/src/webapp/models.py @@ -154,7 +154,7 @@ def _update_topology_repo(self): return False return True - def maybe_update_topology_repo(self): + def maybe_update_topology_repo(self) -> bool: """Update the local git clone of the topology github repo if it hasn't been updated recently (based on the cache time for self.topology_repo_stamp). """ @@ -163,9 +163,11 @@ def maybe_update_topology_repo(self): ok = self._update_topology_repo() if ok: self.topology_repo_stamp.update(get_timestamp()) + return True else: self.topology_repo_stamp.try_again() - return self.topology_repo_stamp.data + return False + return bool(self.topology_repo_stamp.data) def _update_contacts_repo(self): if not self.config["NO_GIT"]: From e44ebba1ccc5c5381dc1088797c9586aac7424d7 Mon Sep 17 00:00:00 2001 From: Matyas Selmeci Date: Tue, 21 May 2024 15:59:49 -0500 Subject: [PATCH 3/4] Add some more debug logging to updates --- src/webapp/models.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/webapp/models.py b/src/webapp/models.py index 8a5ebb7c7..6eea8f6e0 100644 --- a/src/webapp/models.py +++ b/src/webapp/models.py @@ -320,11 +320,13 @@ def update_topology(self) -> None: ok = self.maybe_update_topology_repo() if ok: try: + log.debug("Updating topology RG data") self.topology.update(rg_reader.get_topology(self.topology_dir, self.get_contacts_data(), strict=self.strict)) + log.debug("Updated topology RG data successfully") except Exception: if self.strict: raise - log.exception("Failed to update topology") + log.exception("Failed to update topology RG data") self.topology.try_again() else: self.topology.try_again() @@ -339,7 +341,9 @@ def get_vos_data(self) -> Optional[VOsData]: ok = self.maybe_update_topology_repo() if ok: try: + log.debug("Updating VOs") self.vos_data.update(vo_reader.get_vos_data(self.vos_dir, self.get_contacts_data(), strict=self.strict)) + log.debug("Updated VOs successfully") except Exception: if self.strict: raise @@ -360,7 +364,9 @@ def get_projects(self) -> Optional[Dict]: ok = self.maybe_update_topology_repo() if ok: try: + log.debug("Updating projects") self.projects.update(project_reader.get_projects(self.projects_dir, strict=self.strict)) + log.debug("Updated projects successfully") except Exception: if self.strict: raise @@ -383,7 +389,9 @@ def get_mappings(self, strict=None) -> Optional[mappings.Mappings]: ok = self.maybe_update_topology_repo() if ok: try: + log.debug("Updating mappings") self.mappings.update(mappings.get_mappings(indir=self.mappings_dir, strict=strict)) + log.debug("Updated mappings successfully") except Exception: if self.strict: raise From f304c2440d4a4c64cfd0bea723645225a473a039 Mon Sep 17 00:00:00 2001 From: Matyas Selmeci Date: Wed, 29 May 2024 13:59:02 -0500 Subject: [PATCH 4/4] Drop Python 3.2 compatibility shim --- src/webapp/common.py | 9 --------- src/webapp/models.py | 11 ++++++----- 2 files changed, 6 insertions(+), 14 deletions(-) diff --git a/src/webapp/common.py b/src/webapp/common.py index c64a40643..d11c8e232 100644 --- a/src/webapp/common.py +++ b/src/webapp/common.py @@ -6,7 +6,6 @@ import re import subprocess import sys -import time from typing import Any, Dict, List, Union, AnyStr, NewType, TypeVar from functools import wraps @@ -395,14 +394,6 @@ def wrapped(): return wrapped -def get_timestamp(): - """Return a monotonic timestamp if available, otherwise a wall-clock timestamp.""" - if hasattr(time, "monotonic"): - return time.monotonic() - else: - return time.time() - - XROOTD_CACHE_SERVER = "XRootD cache server" XROOTD_ORIGIN_SERVER = "XRootD origin server" GRIDTYPE_1 = "OSG Production Resource" diff --git a/src/webapp/models.py b/src/webapp/models.py index 6eea8f6e0..4573d7fe9 100644 --- a/src/webapp/models.py +++ b/src/webapp/models.py @@ -2,6 +2,7 @@ import datetime import logging import os +import time from typing import Dict, Set, List, Optional import yaml @@ -23,7 +24,7 @@ def time(self): from webapp import common, contacts_reader, ldap_data, mappings, project_reader, rg_reader, vo_reader -from webapp.common import readfile, get_timestamp +from webapp.common import readfile from webapp.contacts_reader import ContactsData from webapp.topology import Topology, Downtime from webapp.vos_data import VOsData @@ -52,16 +53,16 @@ def should_update(self): """Return True if we should update, either because we're past the next update time or because force_update is True. """ - return self.force_update or not self.data or get_timestamp() > self.next_update + return self.force_update or not self.data or time.monotonic() > self.next_update def try_again(self): """Set the next update time to now + the retry delay.""" - self.next_update = get_timestamp() + self.retry_delay + self.next_update = time.monotonic() + self.retry_delay def update(self, data): """Cache new data and set the next update time to now + the cache lifetime.""" self.data = data - self.timestamp = get_timestamp() + self.timestamp = time.monotonic() self.next_update = self.timestamp + self.cache_lifetime self.force_update = False @@ -162,7 +163,7 @@ def maybe_update_topology_repo(self) -> bool: with topology_git_update_summary.time(): ok = self._update_topology_repo() if ok: - self.topology_repo_stamp.update(get_timestamp()) + self.topology_repo_stamp.update(time.monotonic()) return True else: self.topology_repo_stamp.try_again()