From 51392f73a85e23197cfd477601c5b5cae4e88ee5 Mon Sep 17 00:00:00 2001 From: Raphael Michel Date: Sun, 5 May 2019 16:08:41 +0200 Subject: [PATCH] Locking optimizations --- src/pretix/base/models/orders.py | 13 ++++++------ src/pretix/base/services/cart.py | 32 +++++++++++++++++++++++------ src/pretix/base/services/locking.py | 12 +++++++++++ src/pretix/base/services/orders.py | 19 ++++++++++++----- 4 files changed, 59 insertions(+), 17 deletions(-) diff --git a/src/pretix/base/models/orders.py b/src/pretix/base/models/orders.py index 375aa5cc26..7bc730d053 100644 --- a/src/pretix/base/models/orders.py +++ b/src/pretix/base/models/orders.py @@ -32,6 +32,7 @@ from pretix.base.decimal import round_decimal from pretix.base.i18n import language from pretix.base.models import User from pretix.base.reldate import RelativeDateWrapper +from pretix.base.services.locking import NoLockManager from pretix.base.settings import PERSON_NAME_SCHEMES from .base import LockModel, LoggedModel @@ -1222,13 +1223,13 @@ class OrderPayment(models.Model): if (self.order.status == Order.STATUS_PENDING and self.order.expires > now() + timedelta(hours=12)) or not lock: # Performance optimization. In this case, there's really no reason to lock everything and an atomic # database transaction is more than enough. - with transaction.atomic(): - self._mark_paid(force, count_waitinglist, user, auth, overpaid=payment_sum - refund_sum > self.order.total, - ignore_date=ignore_date) + lockfn = NoLockManager else: - with self.order.event.lock(): - self._mark_paid(force, count_waitinglist, user, auth, overpaid=payment_sum - refund_sum > self.order.total, - ignore_date=ignore_date) + lockfn = self.order.event.lock + + with lockfn(): + self._mark_paid(force, count_waitinglist, user, auth, overpaid=payment_sum - refund_sum > self.order.total, + ignore_date=ignore_date) invoice = None if invoice_qualified(self.order): diff --git a/src/pretix/base/services/cart.py b/src/pretix/base/services/cart.py index 60e85d1969..458bc5f8de 100644 --- a/src/pretix/base/services/cart.py +++ b/src/pretix/base/services/cart.py @@ -5,7 +5,7 @@ from typing import List, Optional from celery.exceptions import MaxRetriesExceededError from django.core.exceptions import ValidationError -from django.db import transaction +from django.db import DatabaseError, transaction from django.db.models import Q from django.dispatch import receiver from django.utils.timezone import make_aware, now @@ -21,7 +21,7 @@ from pretix.base.models.orders import OrderFee from pretix.base.models.tax import TAXED_ZERO, TaxedPrice, TaxRule from pretix.base.reldate import RelativeDateWrapper from pretix.base.services.checkin import _save_answers -from pretix.base.services.locking import LockTimeoutException +from pretix.base.services.locking import LockTimeoutException, NoLockManager from pretix.base.services.pricing import get_price from pretix.base.services.tasks import ProfiledTask from pretix.base.settings import PERSON_NAME_SCHEMES @@ -791,7 +791,11 @@ class CartManager: if available_count == 1: op.position.expires = self._expiry op.position.price = op.price.gross - op.position.save() + try: + op.position.save(force_update=True) + except DatabaseError: + # Best effort... The position might have been deleted in the meantime! + pass elif available_count == 0: op.position.addons.all().delete() op.position.delete() @@ -806,17 +810,33 @@ class CartManager: CartPosition.objects.bulk_create([p for p in new_cart_positions if not getattr(p, '_answers', None) and not p.pk]) return err + def _require_locking(self): + if self._voucher_use_diff: + # If any vouchers are used, we lock to make sure we don't redeem them to often + return True + + if self._quota_diff and any(q.size is not None for q in self._quota_diff): + # If any quotas are affected that are not unlimited, we lock + return True + + return False + def commit(self): self._check_presale_dates() self._check_max_cart_size() self._calculate_expiry() - with self.event.lock() as now_dt: + err = self._delete_out_of_timeframe() + err = self.extend_expired_positions() or err + + lockfn = NoLockManager + if self._require_locking(): + lockfn = self.event.lock + + with lockfn() as now_dt: with transaction.atomic(): self.now_dt = now_dt self._extend_expiry_of_valid_existing_positions() - err = self._delete_out_of_timeframe() - err = self.extend_expired_positions() or err err = self._perform_operations() or err if err: raise CartError(err) diff --git a/src/pretix/base/services/locking.py b/src/pretix/base/services/locking.py index a2ef2dd453..10e8c86058 100644 --- a/src/pretix/base/services/locking.py +++ b/src/pretix/base/services/locking.py @@ -13,6 +13,18 @@ logger = logging.getLogger('pretix.base.locking') LOCK_TIMEOUT = 120 +class NoLockManager: + def __init__(self): + pass + + def __enter__(self): + return now() + + def __exit__(self, exc_type, exc_val, exc_tb): + if exc_type is not None: + return False + + class LockManager: def __init__(self, event): self.event = event diff --git a/src/pretix/base/services/orders.py b/src/pretix/base/services/orders.py index 95a798b6c5..528717de07 100644 --- a/src/pretix/base/services/orders.py +++ b/src/pretix/base/services/orders.py @@ -39,7 +39,7 @@ from pretix.base.services import tickets from pretix.base.services.invoices import ( generate_cancellation, generate_invoice, invoice_qualified, ) -from pretix.base.services.locking import LockTimeoutException +from pretix.base.services.locking import LockTimeoutException, NoLockManager from pretix.base.services.mail import SendMailException from pretix.base.services.pricing import get_price from pretix.base.services.tasks import ProfiledTask @@ -665,9 +665,18 @@ def _perform_order(event: str, payment_provider: str, position_ids: List[str], except InvoiceAddress.DoesNotExist: pass - with event.lock() as now_dt: - positions = list(CartPosition.objects.filter( - id__in=position_ids).select_related('item', 'variation', 'subevent', 'addon_to').prefetch_related('addons')) + positions = CartPosition.objects.filter(id__in=position_ids, event=event) + + lockfn = NoLockManager + locked = False + if positions.filter(Q(voucher__isnull=False) | Q(expires__lt=now() + timedelta(minutes=2))).exists(): + # Performance optimization: If no voucher is used and no cart position is dangerously close to its expiry date, + # creating this order shouldn't be prone to any race conditions and we don't need to lock the event. + locked = True + lockfn = event.lock + + with lockfn() as now_dt: + positions = list(positions.select_related('item', 'variation', 'subevent', 'addon_to').prefetch_related('addons')) if len(positions) == 0: raise OrderError(error_messages['empty']) if len(position_ids) != len(positions): @@ -679,7 +688,7 @@ def _perform_order(event: str, payment_provider: str, position_ids: List[str], free_order_flow = payment and payment_provider == 'free' and order.total == Decimal('0.00') and not order.require_approval if free_order_flow: try: - payment.confirm(send_mail=False, lock=False) + payment.confirm(send_mail=False, lock=not locked) except Quota.QuotaExceededException: pass