diff --git a/onadata/apps/logger/models/instance.py b/onadata/apps/logger/models/instance.py index 254de3629..8b7cb63ad 100644 --- a/onadata/apps/logger/models/instance.py +++ b/onadata/apps/logger/models/instance.py @@ -65,6 +65,9 @@ def submission_time(): def update_xform_submission_count(sender, instance, created, **kwargs): if not created: return + # `defer_counting` is a Python-only attribute + if getattr(instance, 'defer_counting', False): + return with transaction.atomic(): xform = XForm.objects.only('user_id').get(pk=instance.xform_id) # Update with `F` expression instead of `select_for_update` to avoid diff --git a/onadata/libs/utils/logger_tools.py b/onadata/libs/utils/logger_tools.py index 8db53f22f..b706cf981 100644 --- a/onadata/libs/utils/logger_tools.py +++ b/onadata/libs/utils/logger_tools.py @@ -38,7 +38,9 @@ from onadata.apps.logger.models.instance import ( FormInactiveError, InstanceHistory, - get_id_string_from_xml_str) + get_id_string_from_xml_str, + update_xform_submission_count, +) from onadata.apps.logger.models import XForm from onadata.apps.logger.models.xform import XLSFormError from onadata.apps.logger.xform_instance_parser import ( @@ -69,7 +71,14 @@ mongo_instances = settings.MONGO_DB.instances -def _get_instance(xml, new_uuid, submitted_by, status, xform): +def _get_instance(xml, new_uuid, submitted_by, status, xform, + defer_counting=False): + ''' + `defer_counting=False` will set a Python-only attribute of the same name on + the *new* `Instance` if one is created. This will prevent + `update_xform_submission_count()` from doing anything, which avoids locking + any rows in `logger_xform` or `main_userprofile`. + ''' # check if its an edit submission old_uuid = get_deprecated_uuid_from_xml(xml) instances = Instance.objects.filter(uuid=old_uuid) @@ -86,8 +95,19 @@ def _get_instance(xml, new_uuid, submitted_by, status, xform): instance.save() else: # new submission - instance = Instance.objects.create( - xml=xml, user=submitted_by, status=status, xform=xform) + + # Avoid `Instance.objects.create()` so that we can set a Python-only + # attribute, `defer_counting`, before saving + instance = Instance() + instance.xml = xml + instance.user = submitted_by + instance.status = status + instance.xform = xform + if defer_counting: + # Only set the attribute if requested, i.e. don't bother ever + # setting it to `False` + instance.defer_counting = True + instance.save() return instance @@ -117,9 +137,11 @@ def get_xform_from_submission(xml, username, uuid=None): if uuid: # try find the form by its uuid which is the ideal condition - if XForm.objects.filter(uuid=uuid).count() > 0: + try: xform = XForm.objects.get(uuid=uuid) - + except XForm.DoesNotExist: + pass + else: return xform id_string = get_id_string_from_xml_str(xml) @@ -208,7 +230,22 @@ def save_submission(xform, xml, media_files, new_uuid, submitted_by, status, if not date_created_override: date_created_override = get_submission_date_from_xml(xml) - instance = _get_instance(xml, new_uuid, submitted_by, status, xform) + # We have to save the `Instance` to the database before we can associate + # any `Attachment`s with it, but we are inside a transaction and saving + # attachments is slow! Usually creating an `Instance` updates the + # submission count of the parent `XForm` automatically via a `post_save` + # signal, but that takes a lock on `logger_xform` that persists until the + # end of the transaction. We must avoid doing that until all attachments + # are saved, and we are as close as possible to the end of the transaction. + # See https://github.com/kobotoolbox/kobocat/issues/490. + # + # `_get_instance(..., defer_counting=True)` skips incrementing the + # submission counters and returns an `Instance` with a `defer_counting` + # attribute set to `True` *if* a new instance was created. We are + # responsible for calling `update_xform_submission_count()` if the returned + # `Instance` has `defer_counting = True`. + instance = _get_instance(xml, new_uuid, submitted_by, status, xform, + defer_counting=True) save_attachments(instance, media_files) @@ -228,6 +265,15 @@ def save_submission(xform, xml, media_files, new_uuid, submitted_by, status, if not created: pi.save(async=False) + # Now that the slow tasks are complete and we are (hopefully!) close to the + # end of the transaction, update the submission count if the `Instance` was + # newly created + if getattr(instance, 'defer_counting', False): + # Remove the Python-only attribute + del instance.defer_counting + update_xform_submission_count(sender=None, instance=instance, + created=True) + return instance