Do not lock any rows until attachments are saved

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

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