Skip to content

Commit

Permalink
Utils: Use single class for handling locks
Browse files Browse the repository at this point in the history
- The code is now shared for repo and component level locking
- It prefers Redis based locking as that usually performs better
- The Redis locks will also nicely work in a distributed environment
  (#2984)
  • Loading branch information
nijel committed Apr 14, 2021
1 parent 84ba90b commit 9a3bfa8
Show file tree
Hide file tree
Showing 9 changed files with 155 additions and 74 deletions.
2 changes: 2 additions & 0 deletions docs/conf.py
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,8 @@ def setup(app):
"weblate.trans.tasks",
"dateutil",
"filelock",
"redis_lock",
"django_redis",
"lxml",
"translate",
"siphashc",
Expand Down
4 changes: 2 additions & 2 deletions weblate/glossary/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,13 @@

from weblate.lang.models import Language
from weblate.trans.models import Component
from weblate.trans.models.component import ComponentLockTimeout
from weblate.utils.celery import app
from weblate.utils.lock import WeblateLockTimeout


@app.task(
trail=False,
autoretry_for=(Component.DoesNotExist, ComponentLockTimeout),
autoretry_for=(Component.DoesNotExist, WeblateLockTimeout),
retry_backoff=60,
)
def sync_terminology(pk: int, component: Optional[Component] = None):
Expand Down
2 changes: 1 addition & 1 deletion weblate/trans/bulk.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ def bulk_perform(
updated = 0
for component in components:
component.batch_checks = True
with transaction.atomic(), component.lock():
with transaction.atomic(), component.lock:
component.commit_pending("bulk edit", user)
component_units = matching.filter(translation__component=component)

Expand Down
4 changes: 0 additions & 4 deletions weblate/trans/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,3 @@ class FileParseError(Exception):

class PluralFormsMismatch(Exception):
"""Plural forms do not match the language."""


class ComponentLockTimeout(Exception):
"""Component lock timeout."""
64 changes: 13 additions & 51 deletions weblate/trans/models/component.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
import re
import time
from collections import Counter, defaultdict
from contextlib import contextmanager
from copy import copy
from datetime import datetime
from glob import glob
Expand All @@ -32,17 +31,14 @@
from celery import current_task
from celery.result import AsyncResult
from django.conf import settings
from django.core.cache import cache, caches
from django.core.cache import cache
from django.core.exceptions import ObjectDoesNotExist, ValidationError
from django.db import models, transaction
from django.db.models import Count, Q
from django.urls import reverse
from django.utils.functional import cached_property
from django.utils.translation import gettext as _
from django.utils.translation import gettext_lazy, ngettext, pgettext
from django_redis.cache import RedisCache
from filelock import FileLock, Timeout
from redis_lock import AlreadyAcquired, Lock, NotAcquired
from weblate_language_data.ambiguous import AMBIGUOUS

from weblate.checks.flags import Flags
Expand All @@ -57,7 +53,7 @@
PROJECT_NAME_LENGTH,
REPO_LENGTH,
)
from weblate.trans.exceptions import ComponentLockTimeout, FileParseError
from weblate.trans.exceptions import FileParseError
from weblate.trans.fields import RegexField
from weblate.trans.mixins import CacheKeyMixin, PathMixin, URLMixin
from weblate.trans.models.alert import ALERTS, ALERTS_IMPORT
Expand Down Expand Up @@ -92,6 +88,7 @@
from weblate.utils.errors import report_error
from weblate.utils.fields import JSONField
from weblate.utils.licenses import get_license_choices, get_license_url, is_libre
from weblate.utils.lock import WeblateLock, WeblateLockTimeout
from weblate.utils.render import (
render_template,
validate_render_addon,
Expand Down Expand Up @@ -843,50 +840,15 @@ def create_glossary(self):
project.glossaries.append(component)

@cached_property
def _lock(self):
default_cache = caches["default"]
if isinstance(default_cache, RedisCache):
# Prefer Redis locking as it works distributed
return Lock(
default_cache.client.get_client(),
name=f"component-update-lock-{self.pk}",
expire=60,
auto_renewal=True,
)
# Fall back to file based locking
return FileLock(
os.path.join(self.project.full_path, f"{self.slug}-update.lock"),
timeout=1,
)

@contextmanager
def lock(self):
default_cache = caches["default"]
if isinstance(default_cache, RedisCache):
# Prefer Redis locking as it works distributed
try:
if not self._lock.acquire(timeout=1):
raise ComponentLockTimeout()
except AlreadyAcquired:
pass
else:
# Fall back to file based locking
try:
self._lock.acquire()
except Timeout:
raise ComponentLockTimeout()

try:
# Execute context
yield
finally:
# Release lock (the API is same in both cases)
try:
self._lock.release()
except NotAcquired:
# This can happen in case of overloaded server fails to renew the
# lock before expiry
pass
return WeblateLock(
lock_path=self.project.full_path,
scope="component-update",
key=self.pk,
slug=self.slug,
cache_template="{scope}-lock-{key}",
file_template="{slug}-update.lock",
)

@cached_property
def update_key(self):
Expand Down Expand Up @@ -1857,11 +1819,11 @@ def create_translations(
):
"""Load translations from VCS."""
try:
with self.lock():
with self.lock:
return self._create_translations(
force, langs, request, changed_template, from_link
)
except ComponentLockTimeout:
except WeblateLockTimeout:
if settings.CELERY_TASK_ALWAYS_EAGER:
# Retry will not address anything
raise
Expand Down
37 changes: 25 additions & 12 deletions weblate/trans/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,13 +30,12 @@
from django.utils import timezone
from django.utils.translation import gettext as _
from django.utils.translation import ngettext, override
from filelock import Timeout

from weblate.addons.models import Addon
from weblate.auth.models import User, get_anonymous
from weblate.lang.models import Language
from weblate.trans.autotranslate import AutoTranslate
from weblate.trans.exceptions import ComponentLockTimeout, FileParseError
from weblate.trans.exceptions import FileParseError
from weblate.trans.models import (
Change,
Comment,
Expand All @@ -49,12 +48,16 @@
from weblate.utils.data import data_dir
from weblate.utils.errors import report_error
from weblate.utils.files import remove_tree
from weblate.utils.lock import WeblateLockTimeout
from weblate.utils.stats import prefetch_stats
from weblate.vcs.base import RepositoryException


@app.task(
trail=False, autoretry_for=(Timeout,), retry_backoff=600, retry_backoff_max=3600
trail=False,
autoretry_for=(WeblateLockTimeout,),
retry_backoff=600,
retry_backoff_max=3600,
)
def perform_update(cls, pk, auto=False, obj=None):
try:
Expand All @@ -73,7 +76,10 @@ def perform_update(cls, pk, auto=False, obj=None):


@app.task(
trail=False, autoretry_for=(Timeout,), retry_backoff=600, retry_backoff_max=3600
trail=False,
autoretry_for=(WeblateLockTimeout,),
retry_backoff=600,
retry_backoff_max=3600,
)
def perform_load(
pk: int,
Expand All @@ -89,15 +95,21 @@ def perform_load(


@app.task(
trail=False, autoretry_for=(Timeout,), retry_backoff=600, retry_backoff_max=3600
trail=False,
autoretry_for=(WeblateLockTimeout,),
retry_backoff=600,
retry_backoff_max=3600,
)
def perform_commit(pk, *args):
component = Component.objects.get(pk=pk)
component.commit_pending(*args)


@app.task(
trail=False, autoretry_for=(Timeout,), retry_backoff=600, retry_backoff_max=3600
trail=False,
autoretry_for=(WeblateLockTimeout,),
retry_backoff=600,
retry_backoff_max=3600,
)
def perform_push(pk, *args, **kwargs):
component = Component.objects.get(pk=pk)
Expand All @@ -111,7 +123,10 @@ def update_component_stats(pk):


@app.task(
trail=False, autoretry_for=(Timeout,), retry_backoff=600, retry_backoff_max=3600
trail=False,
autoretry_for=(WeblateLockTimeout,),
retry_backoff=600,
retry_backoff_max=3600,
)
def commit_pending(hours=None, pks=None, logger=None):
if pks is None:
Expand Down Expand Up @@ -327,7 +342,7 @@ def project_removal(pk, uid):

@app.task(
trail=False,
autoretry_for=(ComponentLockTimeout,),
autoretry_for=(WeblateLockTimeout,),
retry_backoff=600,
retry_backoff_max=3600,
)
Expand All @@ -349,9 +364,7 @@ def auto_translate(
user = User.objects.get(pk=user_id)
else:
user = None
with translation.component.lock(), override(
user.profile.language if user else "en"
):
with translation.component.lock, override(user.profile.language if user else "en"):
translation.log_info(
"starting automatic translation %s: %s: %s",
current_task.request.id,
Expand Down Expand Up @@ -383,7 +396,7 @@ def auto_translate(

@app.task(
trail=False,
autoretry_for=(ComponentLockTimeout,),
autoretry_for=(WeblateLockTimeout,),
retry_backoff=600,
retry_backoff_max=3600,
)
Expand Down
4 changes: 2 additions & 2 deletions weblate/trans/views/git.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,11 @@
from django.core.exceptions import PermissionDenied
from django.utils.translation import gettext as _
from django.views.decorators.http import require_POST
from filelock import Timeout

from weblate.trans.util import redirect_param
from weblate.utils import messages
from weblate.utils.errors import report_error
from weblate.utils.lock import WeblateLockTimeout
from weblate.utils.views import get_component, get_project, get_translation


Expand All @@ -36,7 +36,7 @@ def execute_locked(request, obj, message, call, *args, **kwargs):
# With False the call is supposed to show errors on its own
if result is None or result:
messages.success(request, message)
except Timeout:
except WeblateLockTimeout:
messages.error(
request,
_("Failed to lock the repository, another operation is in progress."),
Expand Down
100 changes: 100 additions & 0 deletions weblate/utils/lock.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
#
# Copyright © 2012 - 2021 Michal Čihař <michal@cihar.com>
#
# This file is part of Weblate <https://weblate.org/>
#
# This program is free software: you can redistribute it and/or modify
# it under the terms of the GNU General Public License as published by
# the Free Software Foundation, either version 3 of the License, or
# (at your option) any later version.
#
# This program is distributed in the hope that it will be useful,
# but WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
# GNU General Public License for more details.
#
# You should have received a copy of the GNU General Public License
# along with this program. If not, see <https://www.gnu.org/licenses/>.
#
import os
from typing import Optional

from django.core.cache import caches
from django_redis.cache import RedisCache
from filelock import FileLock, Timeout
from redis_lock import AlreadyAcquired, Lock, NotAcquired


class WeblateLockTimeout(Exception):
"""Weblate lock timeout."""


class WeblateLock:
"""Wrapper around Redis or file based lock."""

def __init__(
self,
lock_path: str,
scope: str,
key: int,
slug: str,
cache_template: str = "lock:{scope}:{key}",
file_template: Optional[str] = "{slug}-{scope}.lock",
timeout: int = 1,
):
self._timeout = timeout
self._lock_path = lock_path
self._scope = scope
self._key = key
self._slug = slug
default_cache = caches["default"]
self.use_redis = isinstance(default_cache, RedisCache)
if self.use_redis:
# Prefer Redis locking as it works distributed
self._lock = Lock(
default_cache.client.get_client(),
name=self._format_template(cache_template),
expire=60,
auto_renewal=True,
)
else:
# Fall back to file based locking
self._lock = FileLock(
os.path.join(lock_path, self._format_template(file_template)),
timeout=self._timeout,
)

def _format_template(self, template: str):
return template.format(
scope=self._scope,
key=self._key,
slug=self._slug,
)

def __enter__(self):
if self.use_redis:
try:
if not self._lock.acquire(timeout=self._timeout):
raise WeblateLockTimeout()
except AlreadyAcquired:
pass
else:
# Fall back to file based locking
try:
self._lock.acquire()
except Timeout:
raise WeblateLockTimeout()

def __exit__(self, exc_type, exc_value, traceback):
try:
self._lock.release()
except NotAcquired:
# This can happen in case of overloaded server fails to renew the
# lock before expiry
pass

@property
def is_locked(self):
if self.use_redis:
return self._lock.locked()
return self._lock.is_locked

0 comments on commit 9a3bfa8

Please sign in to comment.