From c268da02a289a6379dddaf750ea023f218aeb9f5 Mon Sep 17 00:00:00 2001 From: Raphael Michel Date: Wed, 16 Sep 2015 11:59:07 +0200 Subject: [PATCH] Simplified the locking procedure --- .../migrations/0014_auto_20150916_1319.py | 25 ++++ src/pretix/base/models.py | 115 +++++++--------- src/pretix/base/services/locking.py | 125 +++++++++-------- src/pretix/base/services/orders.py | 86 +++++------- src/pretix/control/views/orders.py | 2 +- src/pretix/presale/views/cart.py | 129 +++++++++--------- src/pretix/presale/views/order.py | 2 +- src/requirements/production.txt | 4 +- 8 files changed, 240 insertions(+), 248 deletions(-) create mode 100644 src/pretix/base/migrations/0014_auto_20150916_1319.py diff --git a/src/pretix/base/migrations/0014_auto_20150916_1319.py b/src/pretix/base/migrations/0014_auto_20150916_1319.py new file mode 100644 index 0000000000..f717ce38e0 --- /dev/null +++ b/src/pretix/base/migrations/0014_auto_20150916_1319.py @@ -0,0 +1,25 @@ +# -*- coding: utf-8 -*- +from __future__ import unicode_literals + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('pretixbase', '0013_auto_20150916_0941'), + ] + + operations = [ + migrations.CreateModel( + name='EventLock', + fields=[ + ('event', models.CharField(primary_key=True, max_length=36, serialize=False)), + ('date', models.DateTimeField(auto_now=True)), + ], + ), + migrations.RemoveField( + model_name='quota', + name='locked', + ), + ] diff --git a/src/pretix/base/models.py b/src/pretix/base/models.py index 266d5bdaa6..cf2ded3853 100644 --- a/src/pretix/base/models.py +++ b/src/pretix/base/models.py @@ -1,8 +1,7 @@ import copy import random -import time import uuid -from datetime import datetime, timedelta +from datetime import datetime from itertools import product import six @@ -453,6 +452,7 @@ class Event(Versionable): null=True, blank=True, verbose_name=_("Plugins"), ) + locked_here = False class Meta: verbose_name = _("Event") @@ -531,6 +531,13 @@ class Event(Versionable): return False return True + def lock(self): + """ + Returns a contextmanager that can be used to lock an event for bookings + """ + from .services import locking + return locking.LockManager(self) + class EventPermission(Versionable): """ @@ -1372,10 +1379,6 @@ class Quota(Versionable): blank=True, verbose_name=_("Variations") ) - locked = models.DateTimeField( - null=True, blank=True - ) - locked_here = False class Meta: verbose_name = _("Quota") @@ -1443,34 +1446,9 @@ class Quota(Versionable): return Quota.AVAILABILITY_OK, self.size - paid_orders - pending_valid_orders - valid_cart_positions - class LockTimeoutException(Exception): - pass - class QuotaExceededException(Exception): pass - def lock(self): - """ - Issue a lock on this quota so nobody can take tickets from this quota until - you release the lock. Will retry 5 times on failure. - - :raises Quota.LockTimeoutException: if the quota is locked every time we try - to obtain the lock - """ - from .services import locking - - return locking.lock_quota(self) - - def release(self, force=False): - """ - Release a lock placed by :py:meth:`lock()`. If the parameter force is not set to ``True``, - the lock will only be released if it was issued in _this_ python - representation of the database object. - """ - from .services import locking - - return locking.release_quota(self, force) - class Order(Versionable): """ @@ -1637,22 +1615,22 @@ class Order(Versionable): order.save() return order - def _can_be_paid(self, keep_locked=False): + def _can_be_paid(self): error_messages = { 'late': _("The payment is too late to be accepted."), } if self.event.settings.get('payment_term_last') \ and now() > self.event.settings.get('payment_term_last'): - return error_messages['late'], None + return error_messages['late'] if now() < self.expires: - return True, None + return True if not self.event.settings.get('payment_term_accept_late'): - return error_messages['late'], None + return error_messages['late'] - return self._is_still_available(keep_locked) + return self._is_still_available() - def _is_still_available(self, keep_locked=False): + def _is_still_available(self): error_messages = { 'unavailable': _('Some of the ordered products were no longer available.'), 'busy': _('We were not able to process the request completely as the ' @@ -1664,43 +1642,34 @@ class Order(Versionable): 'variation__values', 'variation__values__prop', 'item__questions', 'answers' )) - quotas_locked = set() - release = True - + quota_cache = {} try: - for i, op in enumerate(positions): - quotas = list(op.item.quotas.all()) if op.variation is None else list(op.variation.quotas.all()) - if len(quotas) == 0: - raise Quota.QuotaExceededException(error_messages['unavailable']) - - for quota in quotas: - # Lock the quota, so no other thread is allowed to perform sales covered by this - # quota while we're doing so. - if quota.identity not in [q.identity for q in quotas_locked]: - quota.lock() - quotas_locked.add(quota) - quota.cached_availability = quota.availability()[1] - else: - # Use cached version - quota = [q for q in quotas_locked if q.identity == quota.identity][0] - quota.cached_availability -= 1 - if quota.cached_availability < 0: - # This quota is sold out/currently unavailable, so do not sell this at all + with self.event.lock(): + for i, op in enumerate(positions): + quotas = list(op.item.quotas.all()) if op.variation is None else list(op.variation.quotas.all()) + if len(quotas) == 0: raise Quota.QuotaExceededException(error_messages['unavailable']) + + for quota in quotas: + # Lock the quota, so no other thread is allowed to perform sales covered by this + # quota while we're doing so. + if quota.identity not in quota_cache: + quota_cache[quota.identity] = quota + quota.cached_availability = quota.availability()[1] + else: + # Use cached version + quota = quota_cache[quota.identity] + quota.cached_availability -= 1 + if quota.cached_availability < 0: + # This quota is sold out/currently unavailable, so do not sell this at all + raise Quota.QuotaExceededException(error_messages['unavailable']) except Quota.QuotaExceededException as e: - return str(e), None - except Quota.LockTimeoutException: + return str(e) + except EventLock.LockTimeoutException: # Is raised when there are too many threads asking for quota locks and we were # unaible to get one - return error_messages['busy'], None - else: - release = False - finally: - # Release the locks. This is important ;) - if release or not keep_locked: - for quota in quotas_locked: - quota.release() - return True, quotas_locked + return error_messages['busy'] + return True class CachedTicket(models.Model): @@ -1905,3 +1874,11 @@ class OrganizerSetting(Versionable): object = VersionedForeignKey(Organizer, related_name='setting_objects') key = models.CharField(max_length=255) value = models.TextField() + + +class EventLock(models.Model): + event = models.CharField(max_length=36, primary_key=True) + date = models.DateTimeField(auto_now=True) + + class LockTimeoutException(Exception): + pass diff --git a/src/pretix/base/services/locking.py b/src/pretix/base/services/locking.py index b860ae3a76..cb6c058be4 100644 --- a/src/pretix/base/services/locking.py +++ b/src/pretix/base/services/locking.py @@ -3,110 +3,117 @@ import time from datetime import timedelta from django.conf import settings -from django.db.models import Q +from django.db import transaction from django.utils.timezone import now -from pretix.base.models import Quota +from pretix.base.models import EventLock logger = logging.getLogger('pretix.base.locking') -def lock_quota(quota): +class LockManager: + def __init__(self, event): + self.event = event + + def __enter__(self): + lock_event(self.event) + + def __exit__(self, exc_type, exc_val, exc_tb): + release_event(self.event) + if exc_type is not None: + return False + + +def lock_event(event): """ - Issue a lock on this quota so nobody can take tickets from this quota until + Issue a lock on this event so nobody can book tickets for this event until you release the lock. Will retry 5 times on failure. - :raises Quota.LockTimeoutException: if the quota is locked every time we try - to obtain the lock + :raises EventLock.LockTimeoutException: if the event is locked every time we try + to obtain the lock """ + if event.locked_here: + return True if settings.HAS_REDIS: - return lock_quota_redis(quota) + return lock_event_redis(event) else: - return lock_quota_db(quota) + return lock_event_db(event) -def lock_quota_db(quota): - retries = 5 - for i in range(retries): - dt = now() - - updated = Quota.objects.current.filter( - Q(identity=quota.identity) - & Q(Q(locked__lt=dt - timedelta(seconds=120)) | Q(locked__isnull=True)) - & Q(version_end_date__isnull=True) - ).update( - locked=dt - ) - if updated: - quota.locked_here = dt - quota.locked = dt - return True - time.sleep(2 ** i / 100) - raise Quota.LockTimeoutException() - - -def release_quota(quota, force=False): +def release_event(event, force=False): """ Release a lock placed by :py:meth:`lock()`. If the parameter force is not set to ``True``, the lock will only be released if it was issued in _this_ python representation of the database object. """ - if not quota.locked_here and not force: + if not event.locked_here and not force: return False if settings.HAS_REDIS: - return release_quota_redis(quota) + return release_event_redis(event) else: - return release_quota_db(quota) + return release_event_db(event) -def release_quota_db(quota): - updated = Quota.objects.current.filter( - identity=quota.identity, - version_end_date__isnull=True - ).update( - locked=None - ) - quota.locked_here = None - quota.locked = None - return updated +def lock_event_db(event): + retries = 5 + for i in range(retries): + with transaction.atomic(): + dt = now() + l, created = EventLock.objects.get_or_create(event=event.identity) + if created: + event.locked_here = dt + return True + elif l.date < now() - timedelta(seconds=120): + updated = EventLock.objects.filter(event=event.identity, date=l.date).update(date=dt) + if updated: + event.locked_here = dt + return True + time.sleep(2 ** i / 100) + raise EventLock.LockTimeoutException() -def redis_lock_from_quota(quota): +def release_event_db(event): + deleted = EventLock.objects.filter(event=event.identity).delete() + event.locked_here = None + return deleted + + +def redis_lock_from_event(event): from django_redis import get_redis_connection from redis.lock import Lock - if not hasattr(quota, '_redis_lock'): + if not hasattr(event, '_redis_lock'): rc = get_redis_connection("redis") - quota._redis_lock = Lock(redis=rc, name='pretix_quota_%s' % quota.identity, timeout=120) - return quota._redis_lock + event._redis_lock = Lock(redis=rc, name='pretix_event_%s' % event.identity, timeout=120) + return event._redis_lock -def lock_quota_redis(quota): +def lock_event_redis(event): from redis.exceptions import RedisError - lock = redis_lock_from_quota(quota) + + lock = redis_lock_from_event(event) retries = 5 for i in range(retries): dt = now() try: if lock.acquire(False): - quota.locked_here = dt - quota.locked = dt + event.locked_here = dt return True except RedisError: - logger.exception('Error locking a quota') - raise Quota.LockTimeoutException() + logger.exception('Error locking an event') + raise EventLock.LockTimeoutException() time.sleep(2 ** i / 100) - raise Quota.LockTimeoutException() + raise EventLock.LockTimeoutException() -def release_quota_redis(quota): +def release_event_redis(event): from redis import RedisError - lock = redis_lock_from_quota(quota) + + lock = redis_lock_from_event(event) try: lock.release() except RedisError: - logger.exception('Error releasing a quota lock') - raise Quota.LockTimeoutException() - quota.locked_here = None - quota.locked = None + logger.exception('Error releasing an event lock') + raise EventLock.LockTimeoutException() + event.locked_here = None return True diff --git a/src/pretix/base/services/orders.py b/src/pretix/base/services/orders.py index 22f5909ffe..a998adfa5f 100644 --- a/src/pretix/base/services/orders.py +++ b/src/pretix/base/services/orders.py @@ -4,7 +4,7 @@ from django.db import transaction from django.utils.timezone import now from django.utils.translation import ugettext_lazy as _ -from pretix.base.models import Order, OrderPosition, Quota +from pretix.base.models import EventLock, Order, OrderPosition, Quota from pretix.base.services.mail import mail from pretix.base.signals import order_paid, order_placed from pretix.helpers.urls import build_absolute_uri @@ -27,22 +27,19 @@ def mark_order_paid(order, provider=None, info=None, date=None, manual=None, for :type force: boolean :raises Quota.QuotaExceededException: if the quota is exceeded and ``force`` is ``False`` """ - can_be_paid, quotas_locked = order._can_be_paid(keep_locked=True) - if not force and can_be_paid is not True: - raise Quota.QuotaExceededException(can_be_paid) - order = order.clone() - order.payment_provider = provider or order.payment_provider - order.payment_info = info or order.payment_info - order.payment_date = date or now() - if manual is not None: - order.payment_manual = manual - order.status = Order.STATUS_PAID - order.save() - order_paid.send(order.event, order=order) - - if quotas_locked: - for quota in quotas_locked: - quota.release() + with order.event.lock(): + can_be_paid = order._can_be_paid() + if not force and can_be_paid is not True: + raise Quota.QuotaExceededException(can_be_paid) + order = order.clone() + order.payment_provider = provider or order.payment_provider + order.payment_info = info or order.payment_info + order.payment_date = date or now() + if manual is not None: + order.payment_manual = manual + order.status = Order.STATUS_PAID + order.save() + order_paid.send(order.event, order=order) from pretix.base.services.mail import mail @@ -69,7 +66,7 @@ class OrderError(Exception): pass -def check_positions(event, dt, positions, quotas_locked): +def check_positions(event, dt, positions): error_messages = { 'unavailable': _('Some of the products you selected were no longer available. ' 'Please see below for details.'), @@ -102,11 +99,6 @@ def check_positions(event, dt, positions, quotas_locked): continue quota_ok = True for quota in quotas: - # Lock the quota, so no other thread is allowed to perform sales covered by this - # quota while we're doing so. - if quota.identity not in [q.identity for q in quotas_locked]: - quota.lock() - quotas_locked.add(quota) avail = quota.availability() if avail[0] != Quota.AVAILABILITY_OK: # This quota is sold out/currently unavailable, so do not sell this at all @@ -131,35 +123,31 @@ def perform_order(event, user, payment_provider, positions): 'server was too busy. Please try again.'), } dt = now() - quotas_locked = set() try: - check_positions(event, dt, positions, quotas_locked) - order = place_order(event, user, positions, dt, payment_provider) - mail( - user, _('Your order: %(code)s') % {'code': order.code}, - 'pretixpresale/email/order_placed.txt', - { - 'user': user, 'order': order, - 'event': event, - 'url': build_absolute_uri('presale:event.order', kwargs={ - 'event': event.slug, - 'organizer': event.organizer.slug, - 'order': order.code, - }), - 'payment': payment_provider.order_pending_mail_render(order) - }, - event - ) - return order - except Quota.LockTimeoutException: - # Is raised when there are too many threads asking for quota locks and we were - # unaible to get one + with event.lock(): + check_positions(event, dt, positions) + order = place_order(event, user, positions, dt, payment_provider) + mail( + user, _('Your order: %(code)s') % {'code': order.code}, + 'pretixpresale/email/order_placed.txt', + { + 'user': user, 'order': order, + 'event': event, + 'url': build_absolute_uri('presale:event.order', kwargs={ + 'event': event.slug, + 'organizer': event.organizer.slug, + 'order': order.code, + }), + 'payment': payment_provider.order_pending_mail_render(order) + }, + event + ) + return order + except EventLock.LockTimeoutException: + # Is raised when there are too many threads asking for event locks and we were + # unable to get one raise OrderError(error_messages['busy']) - finally: - # Release the locks. This is important ;) - for quota in quotas_locked: - quota.release() @transaction.atomic() diff --git a/src/pretix/control/views/orders.py b/src/pretix/control/views/orders.py index 5ef64b94f5..c4936066d8 100644 --- a/src/pretix/control/views/orders.py +++ b/src/pretix/control/views/orders.py @@ -248,7 +248,7 @@ class OrderExtend(OrderView): if oldvalue > now(): self.form.save() else: - is_available, _quotas_locked = self.order._is_still_available(keep_locked=False) + is_available = self.order._is_still_available() if is_available is True: self.form.save() messages.success(self.request, _('The payment term has been changed.')) diff --git a/src/pretix/presale/views/cart.py b/src/pretix/presale/views/cart.py index 4c0b0977f8..eed8ca8bf8 100644 --- a/src/pretix/presale/views/cart.py +++ b/src/pretix/presale/views/cart.py @@ -10,7 +10,9 @@ from django.utils.timezone import now from django.utils.translation import ugettext_lazy as _ from django.views.generic import View -from pretix.base.models import CartPosition, Item, ItemVariation, Quota +from pretix.base.models import ( + CartPosition, EventLock, Item, ItemVariation, Quota, +) from pretix.presale.views import EventLoginRequiredMixin, EventViewMixin @@ -187,78 +189,71 @@ class CartAdd(EventViewMixin, CartActionMixin, View): identity__in=[i[1] for i in self.items if i[1] is not None] ).select_related("item", "item__event").prefetch_related("quotas", "values", "values__prop") } + try: + with self.request.event.lock(): + # Process the request itself + for i in self.items: + # Check whether the specified items are part of what we just fetched from the database + # If they are not, the user supplied item IDs which either do not exist or belong to + # a different event + if i[0] not in items_cache or (i[1] is not None and i[1] not in variations_cache): + self.error_message(self.error_messages['not_for_sale']) + return redirect(self.get_failure_url()) - # Process the request itself - for i in self.items: - # Check whether the specified items are part of what we just fetched from the database - # If they are not, the user supplied item IDs which either do not exist or belong to - # a different event - if i[0] not in items_cache or (i[1] is not None and i[1] not in variations_cache): - self.error_message(self.error_messages['not_for_sale']) - return redirect(self.get_failure_url()) + item = items_cache[i[0]] + variation = variations_cache[i[1]] if i[1] is not None else None - item = items_cache[i[0]] - variation = variations_cache[i[1]] if i[1] is not None else None + # Execute restriction plugins to check whether they (a) change the price or + # (b) make the item/variation unavailable. If neither is the case, check_restriction + # will correctly return the default price + price = item.check_restrictions() if variation is None else variation.check_restrictions() - # Execute restriction plugins to check whether they (a) change the price or - # (b) make the item/variation unavailable. If neither is the case, check_restriction - # will correctly return the default price - price = item.check_restrictions() if variation is None else variation.check_restrictions() + # Fetch all quotas. If there are no quotas, this item is not allowed to be sold. + quotas = list(item.quotas.all()) if variation is None else list(variation.quotas.all()) - # Fetch all quotas. If there are no quotas, this item is not allowed to be sold. - quotas = list(item.quotas.all()) if variation is None else list(variation.quotas.all()) + if price is False or len(quotas) == 0 or not item.active: + self.error_message(self.error_messages['unavailable']) + continue - if price is False or len(quotas) == 0 or not item.active: - self.error_message(self.error_messages['unavailable']) - continue + # Assume that all quotas allow us to buy i[2] instances of the object + quota_ok = i[2] + for quota in quotas: + avail = quota.availability() + if avail[1] < i[2]: + # This quota is not available or less than i[2] items are left, so we have to + # reduce the number of bought items + self.error_message( + self.error_messages['unavailable'] + if avail[0] != Quota.AVAILABILITY_OK + else self.error_messages['in_part'] + ) + quota_ok = min(quota_ok, avail[1]) - # Assume that all quotas allow us to buy i[2] instances of the object - quota_ok = i[2] - try: - for quota in quotas: - # Lock the quota, so no other thread is allowed to perform sales covered by this - # quota while we're doing so. - quota.lock() - avail = quota.availability() - if avail[1] < i[2]: - # This quota is not available or less than i[2] items are left, so we have to - # reduce the number of bought items - self.error_message( - self.error_messages['unavailable'] - if avail[0] != Quota.AVAILABILITY_OK - else self.error_messages['in_part'] - ) - quota_ok = min(quota_ok, avail[1]) + # Create a CartPosition for as much items as we can + for k in range(quota_ok): + if len(i) > 3 and i[2] == 1: + # Recreating + cp = i[3].clone() + cp.expires = expiry + cp.price = price + cp.save() + else: + CartPosition.objects.create( + event=self.request.event, + user=self.request.user, + item=item, + variation=variation, + price=price, + expires=expiry + ) - # Create a CartPosition for as much items as we can - for k in range(quota_ok): - if len(i) > 3 and i[2] == 1: - # Recreating - cp = i[3].clone() - cp.expires = expiry - cp.price = price - cp.save() - else: - CartPosition.objects.create( - event=self.request.event, - user=self.request.user, - item=item, - variation=variation, - price=price, - expires=expiry - ) - except Quota.LockTimeoutException: - # Is raised when there are too many threads asking for quota locks and we were - # unaible to get one - self.error_message(self.error_messages['busy'], important=True) - finally: - # Release the locks. This is important ;) - for quota in quotas: - quota.release() + self._delete_expired() - self._delete_expired() + if not self.msg_some_unavailable: + messages.success(self.request, _('The products have been successfully added to your cart.')) - if not self.msg_some_unavailable: - messages.success(self.request, _('The products have been successfully added to your cart.')) - - return redirect(self.get_success_url()) + return redirect(self.get_success_url()) + except EventLock.LockTimeoutException: + # Is raised when there are too many threads asking for quota locks and we were + # unaible to get one + self.error_message(self.error_messages['busy'], important=True) diff --git a/src/pretix/presale/views/order.py b/src/pretix/presale/views/order.py index 22a119d466..371857ff55 100644 --- a/src/pretix/presale/views/order.py +++ b/src/pretix/presale/views/order.py @@ -94,7 +94,7 @@ class OrderDetails(EventViewMixin, EventLoginRequiredMixin, OrderDetailMixin, ctx['can_retry'] = ( self.payment_provider.order_can_retry(self.order) and self.payment_provider.is_enabled - and self.order._can_be_paid(keep_locked=False) + and self.order._can_be_paid() ) elif self.order.status == Order.STATUS_PAID: ctx['payment'] = self.payment_provider.order_paid_render(self.request, self.order) diff --git a/src/requirements/production.txt b/src/requirements/production.txt index fcc59d7734..0771b1c102 100644 --- a/src/requirements/production.txt +++ b/src/requirements/production.txt @@ -4,8 +4,8 @@ python-dateutil>=2.4,<2.5 pytz django-bootstrap3>=6.1,<6.2 -e git+https://github.com/pretix/django-formset-js.git@master#egg=django-formset-js -#cleanerversion>=1.5,<1.6 --e git+https://github.com/pretix/cleanerversion.git@pretix#egg=CleanerVersion +cleanerversion==1.5.3 +#-e git+https://github.com/pretix/cleanerversion.git@pretix#egg=CleanerVersion django-compressor>=1.5,<2.0 reportlab>=3.1.44,<3.2 -e git+https://github.com/pretix/PyPDF2.git@pretix#egg=PyPDF2