From 275ef22d181ab5cba4ba26a8dea602d15f2bfa1f Mon Sep 17 00:00:00 2001 From: Jakub Kadlcik Date: Sun, 11 Aug 2024 13:09:35 +0200 Subject: [PATCH] distgit, backend, common: unify file locking code in copr-common Fix #3367 As a consequence, stop using `python3-oslo-concurrency` on Copr DistGit and start using `python3-filelock` as the rest of the stack. --- backend/copr-backend.spec | 4 +-- backend/run/copr-repo | 26 +++--------------- backend/tests/test_modifyrepo.py | 3 ++- common/copr_common/lock.py | 43 ++++++++++++++++++++++++++++++ common/python-copr-common.spec | 8 +++++- common/setup.py | 2 +- dist-git/copr-dist-git.spec | 4 +-- dist-git/copr_dist_git/helpers.py | 25 +++-------------- dist-git/copr_dist_git/importer.py | 4 +-- 9 files changed, 64 insertions(+), 55 deletions(-) create mode 100644 common/copr_common/lock.py diff --git a/backend/copr-backend.spec b/backend/copr-backend.spec index 658a622f9..fb27b0d91 100644 --- a/backend/copr-backend.spec +++ b/backend/copr-backend.spec @@ -6,7 +6,7 @@ %global tests_version 5 %global tests_tar test-data-copr-backend -%global copr_common_version 0.24.1~~dev0 +%global copr_common_version 0.25.1~~dev0 Name: copr-backend Version: 1.177 @@ -41,7 +41,6 @@ BuildRequires: python3-copr-common >= %copr_common_version BuildRequires: python3-daemon BuildRequires: python3-dateutil BuildRequires: python3-distro -BuildRequires: python3-filelock BuildRequires: python3-gobject BuildRequires: python3-httpretty BuildRequires: python3-humanize @@ -81,7 +80,6 @@ Recommends: python3-copr-messaging Requires: python3-daemon Requires: python3-dateutil Recommends: python3-fedmsg -Requires: python3-filelock Requires: python3-gobject Requires: python3-humanize Requires: python3-jinja2 diff --git a/backend/run/copr-repo b/backend/run/copr-repo index 01b2c0e01..d8df7c870 100755 --- a/backend/run/copr-repo +++ b/backend/run/copr-repo @@ -10,7 +10,6 @@ mess up everything around. """ import argparse -import contextlib import datetime import logging import os @@ -19,8 +18,7 @@ import shutil import subprocess import sys -import filelock - +from copr_common.lock import lock, LockTimeout from copr_backend.constants import CHROOTS_USING_SQLITE_REPODATA from copr_backend.createrepo import BatchedCreaterepo from copr_backend.helpers import ( @@ -313,24 +311,6 @@ def assert_new_createrepo(): assert b'--recycle-pkglist' in out -class LockTimeout(OSError): - """ Raised for lock() timeout, if timeout= option is set to value >= 0 """ - -@contextlib.contextmanager -def lock(opts, timeout=-1): - lock_path = os.environ.get('COPR_TESTSUITE_LOCKPATH', "/var/lock/copr-backend") - lock_basename = opts.directory.replace("/", "_@_") + '.lock' - lock_filename = os.path.join(lock_path, lock_basename) - opts.log.debug("acquiring lock") - try: - with filelock.FileLock(lock_filename, timeout=timeout): - opts.log.debug("acquired lock") - yield - except filelock.Timeout as err: - opts.log.debug("lock timeouted") - raise LockTimeout("Timeouted on lock file: {}".format(lock_path)) from err - - def main_locked(opts, batch, log): """ Main method, executed under lock. @@ -411,7 +391,9 @@ def main_try_lock(opts, batch): return try: - with lock(opts, timeout=5): + lockdir = os.environ.get( + "COPR_TESTSUITE_LOCKPATH", "/var/lock/copr-backend") + with lock(opts.directory, lockdir=lockdir, timeout=5, log=opts.log): main_locked(opts, batch, opts.log) # skip commit if main_locked() raises exception batch.commit() diff --git a/backend/tests/test_modifyrepo.py b/backend/tests/test_modifyrepo.py index b4f51707b..330ff68f7 100644 --- a/backend/tests/test_modifyrepo.py +++ b/backend/tests/test_modifyrepo.py @@ -47,7 +47,8 @@ def _lock(directory="non-existent"): opts.log = logging.getLogger() opts.directory = directory lock = filedict['lock'] - with lock(opts): + lockdir = os.environ.get("COPR_TESTSUITE_LOCKPATH") + with lock(opts.directory, lockdir=lockdir, timeout=-1, log=opts.log): yield opts class TestModifyRepo(object): diff --git a/common/copr_common/lock.py b/common/copr_common/lock.py new file mode 100644 index 000000000..cec3d281f --- /dev/null +++ b/common/copr_common/lock.py @@ -0,0 +1,43 @@ +""" +File locking for multithreading +""" + +import os +import contextlib +import filelock +from setproctitle import getproctitle, setproctitle + + +@contextlib.contextmanager +def lock(path, lockdir, timeout, log): + """ + Create a lock file that can be accessed only by one thread at the time. + A practical use-case for this is to lock a repository so multiple versions + of the same package cannot be imported in paralel. + + We want to pass a simple `path` parameter specifying what file or directory + should be locked. This however isn't where the lockfile is going to be + created. It will be created in the `lockdir`. + """ + filename = path.replace("/", "_@_") + ".lock" + lockfile = os.path.join(lockdir, filename) + + title = getproctitle() + setproctitle("{0} [locking]".format(title)) + log.debug("acquiring lock") + try: + with filelock.FileLock(lockfile, timeout=timeout): + setproctitle("{0} [locked]".format(title)) + log.debug("acquired lock") + yield + except filelock.Timeout as err: + log.debug("lock timeouted") + raise LockTimeout("Timeouted on lock for: {}".format(path)) from err + finally: + setproctitle("{0} [locking]".format(title)) + + +class LockTimeout(OSError): + """ + Raised for lock() timeout, if timeout= option is set to value >= 0 + """ diff --git a/common/python-copr-common.spec b/common/python-copr-common.spec index 281ae0974..549d77d0b 100644 --- a/common/python-copr-common.spec +++ b/common/python-copr-common.spec @@ -1,7 +1,7 @@ %global srcname copr-common Name: python-copr-common -Version: 0.25 +Version: 0.25.1.dev0 Release: 1%{?dist} Summary: Python code used by Copr @@ -19,6 +19,12 @@ BuildRequires: python3-devel BuildRequires: python3-setuptools BuildRequires: python3-pytest BuildRequires: python3-requests +BuildRequires: python3-filelock +BuildRequires: python3-setproctitle + +Requires: python3-filelock +Requires: python3-setproctitle + %global _description\ COPR is lightweight build system. It allows you to create new project in WebUI,\ diff --git a/common/setup.py b/common/setup.py index 1520da379..c9374a14b 100644 --- a/common/setup.py +++ b/common/setup.py @@ -20,7 +20,7 @@ setup( name='copr-common', - version="0.25", + version="0.25.1.dev0", description=__description__, long_description=long_description, author=__author__, diff --git a/dist-git/copr-dist-git.spec b/dist-git/copr-dist-git.spec index cb286367f..00b311358 100644 --- a/dist-git/copr-dist-git.spec +++ b/dist-git/copr-dist-git.spec @@ -1,4 +1,4 @@ -%global copr_common_version 0.20.1.dev1 +%global copr_common_version 0.25.1~~dev0 Name: copr-dist-git Version: 0.70 @@ -22,7 +22,6 @@ BuildRequires: python3-requests BuildRequires: python3-rpkg >= 1.67-1 BuildRequires: python3-pytest BuildRequires: python3-copr-common >= %copr_common_version -BuildRequires: python3-oslo-concurrency BuildRequires: python3-redis BuildRequires: python3-setproctitle @@ -37,7 +36,6 @@ Requires: python3-copr-common >= %copr_common_version Requires: python3-requests Requires: python3-rpkg >= 1.66-6 Requires: python3-munch -Requires: python3-oslo-concurrency Requires: python3-setproctitle Requires: python3-daemon Requires: python3-redis diff --git a/dist-git/copr_dist_git/helpers.py b/dist-git/copr_dist_git/helpers.py index 21e590afb..7967ec4b3 100644 --- a/dist-git/copr_dist_git/helpers.py +++ b/dist-git/copr_dist_git/helpers.py @@ -2,37 +2,18 @@ import sys import logging import subprocess -import munch +from configparser import ConfigParser -from oslo_concurrency import lockutils -from setproctitle import getproctitle, setproctitle -from copr_common.request import SafeRequest, RequestError +import munch from munch import Munch +from copr_common.request import SafeRequest, RequestError from .exceptions import FileDownloadException, RunCommandException -from contextlib import contextmanager -from configparser import ConfigParser log = logging.getLogger(__name__) LOCK_PATH = "/var/lock/copr-dist-git" -@contextmanager -def lock(name): - """ - Create a lock file that can be accessed only by one thread at the time. - A practical use-case for this is to lock a repository so multiple versions - of the same package cannot be imported in paralel. - """ - title = getproctitle() - setproctitle("{0} [locking]".format(title)) - with lockutils.lock(name=name, external=True, lock_path=LOCK_PATH, - fair=True, delay=0): - setproctitle("{0} [locked]".format(title)) - yield - setproctitle(title) - - def _get_conf(cp, section, option, default, mode=None): """ To make returning items from config parser less irritating diff --git a/dist-git/copr_dist_git/importer.py b/dist-git/copr_dist_git/importer.py index 701e2a767..06ae4ae56 100644 --- a/dist-git/copr_dist_git/importer.py +++ b/dist-git/copr_dist_git/importer.py @@ -8,6 +8,7 @@ from requests import get, post from copr_common.worker_manager import WorkerManager +from copr_common.lock import lock from .package_import import import_package from .process_pool import Worker, Pool, SingleThreadWorker @@ -82,8 +83,7 @@ def do_import(self, task): ) repo = os.path.join(self.opts.lookaside_location, task.reponame) - lockfile = os.path.join(repo, "import.lock") - with helpers.lock(lockfile): + with lock(repo, lockdir=helpers.LOCK_PATH, timeout=-1, log=log): result.update(import_package( self.opts, task.repo_namespace,