🧵(backend) remove lock from db table

To avoid race conditions when creating a new
document, we used to create a lock on the database
table the time of the creation, to prevent
concurrent creations and duplicate path keys.
This approach was not ideal as it could lead to
deadlocks and performance issues.
We now catch the integrity error raised by the
database when a duplicate path key is created,
and we retry the creation with a new path key.
This approach is more efficient as it is retrying
only when there is a conflict, and it does not
require locking the entire table.

CHANGELOG.md

@@ -10,6 +10,10 @@ and this project adheres to
 
 - ⚡️(frontend) add skeleton on content loading #2254
 
+### Changed
+
+- 🧵(backend) remove lock from db table #2272
+
 ### Fixed
 
 - 💬(frontend) add missing link in onboarding description #2233

src/backend/core/api/serializers.py

@@ -4,10 +4,11 @@
 import binascii
 import mimetypes
 from base64 import b64decode
+from logging import getLogger
 from os.path import splitext
 
 from django.conf import settings
-from django.db import connection, transaction
+from django.db import IntegrityError, transaction
 from django.db.models import Q
 from django.utils.functional import lazy
 from django.utils.text import slugify
@@ -25,6 +26,8 @@ from core.services.converter_services import (
     Converter,
 )
 
+logger = getLogger(__name__)
+
 
 class UserSerializer(serializers.ModelSerializer):
     """Serialize users."""
@@ -467,18 +470,18 @@ class ServerCreateDocumentSerializer(serializers.Serializer):
                 {"content": ["Could not convert content"]}
             ) from err
 
-        with transaction.atomic():
-            # locks the table to ensure safe concurrent access
-            with connection.cursor() as cursor:
-                cursor.execute(
-                    f'LOCK TABLE "{models.Document._meta.db_table}" '  # noqa: SLF001
-                    "IN SHARE ROW EXCLUSIVE MODE;"
-                )
-
-            document = models.Document.add_root(
-                title=validated_data["title"],
-                creator=user,
-            )
+        while True:
+            try:
+                with transaction.atomic():
+                    document = models.Document.add_root(
+                        title=validated_data["title"],
+                        creator=user,
+                    )
+                break
+            except IntegrityError as e:
+                if "impress_document_path_key" not in str(e):
+                    raise
+                logger.warning("Path key conflict when creating document, retrying...")
 
         if user:
             # Associate the document with the pre-existing user

src/backend/core/api/viewsets.py

@@ -19,7 +19,7 @@ from django.core.cache import cache
 from django.core.exceptions import ValidationError
 from django.core.files.storage import default_storage
 from django.core.validators import URLValidator
-from django.db import connection, transaction
+from django.db import IntegrityError, connection, transaction
 from django.db import models as db
 from django.db.models.expressions import RawSQL
 from django.db.models.functions import Greatest, Left, Length
@@ -708,18 +708,18 @@ class DocumentViewSet(
                     {"file": ["Could not convert file content"]}
                 ) from err
 
-        with transaction.atomic():
-            # locks the table to ensure safe concurrent access
-            with connection.cursor() as cursor:
-                cursor.execute(
-                    f'LOCK TABLE "{models.Document._meta.db_table}" '  # noqa: SLF001
-                    "IN SHARE ROW EXCLUSIVE MODE;"
-                )
-
-            obj = models.Document.add_root(
-                creator=self.request.user,
-                **serializer.validated_data,
-            )
+        while True:
+            try:
+                with transaction.atomic():
+                    obj = models.Document.add_root(
+                        creator=self.request.user,
+                        **serializer.validated_data,
+                    )
+                break
+            except IntegrityError as e:
+                if "impress_document_path_key" not in str(e):
+                    raise
+                logger.warning("Path key conflict when creating document, retrying...")
         serializer.instance = obj
         models.DocumentAccess.objects.create(
             document=obj,

src/backend/core/models.py

@@ -19,7 +19,7 @@ from django.core.cache import cache
 from django.core.files.base import ContentFile
 from django.core.files.storage import default_storage
 from django.core.mail import send_mail
-from django.db import connection, models, transaction
+from django.db import IntegrityError, models, transaction
 from django.db.models.functions import Left, Length
 from django.template.loader import render_to_string
 from django.utils import timezone
@@ -277,24 +277,26 @@ class User(AbstractBaseUser, BaseModel, auth_models.PermissionsMixin):
                 )
                 return
 
-            with transaction.atomic():
-                # locks the table to ensure safe concurrent access
-                with connection.cursor() as cursor:
-                    cursor.execute(
-                        f'LOCK TABLE "{Document._meta.db_table}" '  # noqa: SLF001
-                        "IN SHARE ROW EXCLUSIVE MODE;"
+            while True:
+                try:
+                    with transaction.atomic():
+                        sandbox_document = Document.add_root(
+                            title=template_document.title,
+                            content=template_document.content,
+                            attachments=template_document.attachments,
+                            duplicated_from=template_document,
+                            creator=self,
+                        )
+                        DocumentAccess.objects.create(
+                            user=self, document=sandbox_document, role=RoleChoices.OWNER
+                        )
+                    break
+                except IntegrityError as e:
+                    if "impress_document_path_key" not in str(e):
+                        raise
+                    logger.warning(
+                        "Path key conflict when creating sandbox document, retrying..."
                     )
-                sandbox_document = Document.add_root(
-                    title=template_document.title,
-                    content=template_document.content,
-                    attachments=template_document.attachments,
-                    duplicated_from=template_document,
-                    creator=self,
-                )
-
-                DocumentAccess.objects.create(
-                    user=self, document=sandbox_document, role=RoleChoices.OWNER
-                )
 
     def _convert_valid_invitations(self):
         """