Merge branch 'concurrency-fixes-for-singleton' into 'develop'

Fix concurrency issue when update view indexes

See merge request baserow/baserow!1419

backend/src/baserow/config/settings/base.py

@@ -105,6 +105,7 @@ if BASEROW_CACHALOT_ONLY_CACHABLE_TABLES is None:
         "core_settings",
         "auth_user",
         "core_userprofile",
+        "core_workspace",
         "core_workspaceuser",
         "database_token",
         "database_tokenpermission",

backend/src/baserow/contrib/database/db/atomic.py

@@ -1,5 +1,6 @@
 from django.db.transaction import Atomic
 
+from cachalot.api import cachalot_disabled
 from psycopg2 import sql
 
 from baserow.core.db import IsolationLevel, transaction_atomic
@@ -43,13 +44,14 @@ def read_repeatable_single_database_atomic_transaction(
 """
     )
     first_statement_args = [sql.Literal(database_id)]
-    return transaction_atomic(
-        isolation_level=IsolationLevel.REPEATABLE_READ,
-        first_sql_to_run_in_transaction_with_args=(
-            first_statement,
-            first_statement_args,
-        ),
-    )
+    with cachalot_disabled():
+        return transaction_atomic(
+            isolation_level=IsolationLevel.REPEATABLE_READ,
+            first_sql_to_run_in_transaction_with_args=(
+                first_statement,
+                first_statement_args,
+            ),
+        )
 
 
 def read_committed_single_table_transaction(
@@ -134,10 +136,11 @@ def read_repeatable_read_single_table_transaction(
 """
     )
     first_statement_args = [sql.Literal(table_id)]
-    return transaction_atomic(
-        isolation_level=IsolationLevel.REPEATABLE_READ,
-        first_sql_to_run_in_transaction_with_args=(
-            first_statement,
-            first_statement_args,
-        ),
-    )
+    with cachalot_disabled():
+        return transaction_atomic(
+            isolation_level=IsolationLevel.REPEATABLE_READ,
+            first_sql_to_run_in_transaction_with_args=(
+                first_statement,
+                first_statement_args,
+            ),
+        )

backend/src/baserow/contrib/database/views/handler.py

@@ -1,4 +1,5 @@
 import re
+import traceback
 from collections import defaultdict, namedtuple
 from copy import deepcopy
 from dataclasses import dataclass
@@ -214,12 +215,18 @@ class ViewIndexingHandler(metaclass=baserow_trace_methods(tracer)):
         """
 
         view_type = view_type_registry.get_by_model(view)
-        if (
-            view_type.can_sort
-            and not view.db_index_name
-            and cls.get_index(view, model) is not None
-        ):
-            cls.schedule_index_update(view)
+        if not view_type.can_sort:
+            return
+
+        try:
+            db_index = cls.get_index(view, model)
+            if db_index is not None and db_index.name != view.db_index_name:
+                cls.schedule_index_update(view)
+        except Exception as exc:  # nosec
+            logger.error(
+                "Failed to check if view needs index because of {e}", e=str(exc)
+            )
+            traceback.print_exc()
 
     @classmethod
     def get_index(

backend/src/baserow/contrib/database/views/tasks.py

@@ -35,25 +35,19 @@ def update_view_index(view_id: int):
         )
         ViewIndexingHandler.update_index(view)
     finally:
-        # if the view_id is in the cache then it means something happened
-        # during the task and it should be re-scheduled.
-        if cache.delete(get_auto_index_cache_key(view_id)):
-            schedule_view_index_update(view_id)
+        # check for any pending view index updates and schedule them out of this
+        # singleton task to avoid concurrency issues
+        _check_for_pending_view_index_updates.delay(view_id)
 
 
-def schedule_view_index_update(view_id: int):
+@app.task(queue="export")
+def _check_for_pending_view_index_updates(view_id):
     """
-    Schedules a view index update for the provided view id. If the view index
-    update is already scheduled then just add the view_id in the cache so that
-    `update_view_index` will re-schedule itself at the end.
-
-    :param view_id: The id of the view for which the index should be updated.
+    Checks if there are any pending view index updates and schedules them.
     """
 
-    if not settings.AUTO_INDEX_VIEW_ENABLED:
-        return
-
-    transaction.on_commit(lambda: _schedule_view_index_update(view_id))
+    if cache.delete(get_auto_index_cache_key(view_id)):
+        _schedule_view_index_update(view_id)
 
 
 def _schedule_view_index_update(view_id: int):
@@ -75,3 +69,18 @@ def _schedule_view_index_update(view_id: int):
             True,
             timeout=settings.AUTO_INDEX_LOCK_EXPIRY * 2,
         )
+
+
+def schedule_view_index_update(view_id: int):
+    """
+    Schedules a view index update for the provided view id. If the view index
+    update is already scheduled then just add the view_id in the cache so that
+    `update_view_index` will re-schedule itself at the end.
+
+    :param view_id: The id of the view for which the index should be updated.
+    """
+
+    if not settings.AUTO_INDEX_VIEW_ENABLED:
+        return
+
+    transaction.on_commit(lambda: _schedule_view_index_update(view_id))

backend/tests/baserow/contrib/database/view/test_view_handler.py

@@ -3047,7 +3047,9 @@ def test_loading_a_view_checks_for_db_index_without_additional_queries(
 
     # actually create the index for the view
     ViewIndexingHandler.update_index(grid_view, model)
-    view.refresh_from_db()
+    view = view_handler.get_view(
+        grid_view.id, base_queryset=GridView.objects.prefetch_related("viewsort_set")
+    )
     assert view.db_index_name
 
     with override_settings(AUTO_INDEX_VIEW_ENABLED=True), django_assert_num_queries(0):