Skip to content

Commit

Permalink
distgit, backend, common: unify file locking code in copr-common
Browse files Browse the repository at this point in the history
Fix fedora-copr#3367

As a consequence, stop using `python3-oslo-concurrency` on Copr DistGit and
start using `python3-filelock` as the rest of the stack.
  • Loading branch information
FrostyX committed Aug 23, 2024
1 parent 69957fc commit b7f171d
Show file tree
Hide file tree
Showing 10 changed files with 72 additions and 57 deletions.
4 changes: 1 addition & 3 deletions backend/copr-backend.spec
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
26 changes: 4 additions & 22 deletions backend/run/copr-repo
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ mess up everything around.
"""

import argparse
import contextlib
import datetime
import logging
import os
Expand All @@ -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 (
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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()
Expand Down
3 changes: 2 additions & 1 deletion backend/tests/test_modifyrepo.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
43 changes: 43 additions & 0 deletions common/copr_common/lock.py
Original file line number Diff line number Diff line change
@@ -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
"""
4 changes: 3 additions & 1 deletion common/python-copr-common.spec
Original file line number Diff line number Diff line change
@@ -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

Expand All @@ -19,6 +19,8 @@ BuildRequires: python3-devel
BuildRequires: python3-setuptools
BuildRequires: python3-pytest
BuildRequires: python3-requests
BuildRequires: python3-filelock
BuildRequires: python3-setproctitle

%global _description\
COPR is lightweight build system. It allows you to create new project in WebUI,\
Expand Down
9 changes: 8 additions & 1 deletion common/setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,15 @@
__url__ = "https://github.com/fedora-copr/copr"


requires = [
"filelock",
"setproctitle",
]


setup(
name='copr-common',
version="0.25",
version="0.25.1.dev0",
description=__description__,
long_description=long_description,
author=__author__,
Expand All @@ -30,6 +36,7 @@
classifiers=[
"License :: OSI Approved :: GNU General Public License v2 or later (GPLv2+)",
],
install_requires=requires,
packages=['copr_common'],
include_package_data=True,
zip_safe=False
Expand Down
4 changes: 1 addition & 3 deletions dist-git/copr-dist-git.spec
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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

Expand All @@ -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
Expand Down
25 changes: 3 additions & 22 deletions dist-git/copr_dist_git/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions dist-git/copr_dist_git/importer.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down
7 changes: 5 additions & 2 deletions dist-git/tests/test_importer.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

import os
import copy
from tempfile import TemporaryDirectory
import pytest

from munch import Munch
Expand Down Expand Up @@ -111,8 +112,10 @@ def test_do_import(self, mc_import_package, mc_helpers):
reponame='foo',
branch_commits={self.BRANCH: '123', self.BRANCH2: '124'}
)
self.importer.post_back_safe = MagicMock()
self.importer.do_import(self.url_task)
with TemporaryDirectory(prefix="copr-dist-git-test-") as tmp:
mc_helpers.LOCK_PATH = tmp
self.importer.post_back_safe = MagicMock()
self.importer.do_import(self.url_task)

assert mc_import_package.call_args[0][0] == self.opts
assert mc_import_package.call_args[0][1] == self.url_task.repo_namespace
Expand Down

0 comments on commit b7f171d

Please sign in to comment.