From f8b9e4d780afc1db0856462f42ca57b22cb51d37 Mon Sep 17 00:00:00 2001
From: Marie <51697796+ijreilly@users.noreply.github.com>
Date: Fri, 18 Apr 2025 18:16:34 +0200
Subject: [PATCH] ignoreLock when recomputing rolePermissions when creating
 datasource (#11657)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

[sentry](https://twenty-v7.sentry.io/issues/6545999328/?environment=prod&project=4507072499810304&query=is%3Aunresolved%20issue.priority%3A%5Bhigh%2C%20medium%5D&referrer=issue-stream&sort=date&stream_index=2)

Our workers have failing jobs because they are concurrently creating
datasources on different pods
While the datasources are independent, they read from the same redis
cache, with the same `RolesPermissionsOngoingCachingLock` value in
redis.
<img width="852" alt="Capture d’écran 2025-04-18 à 18 00 20"
src="https://github.com/user-attachments/assets/42ce8479-acc1-462b-af4c-b547cc2bb0b8"
/>
As a consequence they can fail to create a datasource: the first
datasource computes the permissions and sets ongoingCachingLock to true,
then the second arrives, there are still no available permissions in the
cache, but because of the lock it early returns without permissions and
fails. This behavior goes unnoticed on the product and helps
performances, but is annoying for workers as jobs are failing.

Let's remove the cache lock when creating the datasource !
---
 .../workspace-permissions-cache.service.ts    | 77 ++++++++++---------
 .../factories/workspace-datasource.factory.ts |  9 ++-
 2 files changed, 45 insertions(+), 41 deletions(-)

diff --git a/packages/twenty-server/src/engine/metadata-modules/workspace-permissions-cache/workspace-permissions-cache.service.ts b/packages/twenty-server/src/engine/metadata-modules/workspace-permissions-cache/workspace-permissions-cache.service.ts
index d53581133..9171316aa 100644
--- a/packages/twenty-server/src/engine/metadata-modules/workspace-permissions-cache/workspace-permissions-cache.service.ts
+++ b/packages/twenty-server/src/engine/metadata-modules/workspace-permissions-cache/workspace-permissions-cache.service.ts
@@ -70,35 +70,36 @@ export class WorkspacePermissionsCacheService {
       workspaceId,
     );
 
-    let currentRolesPermissions: ObjectRecordsPermissionsByRoleId | undefined =
-      undefined;
+    try {
+      let currentRolesPermissions: ObjectRecordsPermissionsByRoleId | undefined;
 
-    if (roleIds) {
-      currentRolesPermissions =
-        await this.workspacePermissionsCacheStorageService.getRolesPermissions(
+      if (roleIds) {
+        currentRolesPermissions =
+          await this.workspacePermissionsCacheStorageService.getRolesPermissions(
+            workspaceId,
+          );
+      }
+
+      const recomputedRolesPermissions =
+        await this.getObjectRecordPermissionsForRoles({
           workspaceId,
-        );
-    }
+          isPermissionsV2Enabled,
+          roleIds,
+        });
 
-    const recomputedRolesPermissions =
-      await this.getObjectRecordPermissionsForRoles({
+      const freshObjectRecordsPermissionsByRoleId = roleIds
+        ? { ...currentRolesPermissions, ...recomputedRolesPermissions }
+        : recomputedRolesPermissions;
+
+      await this.workspacePermissionsCacheStorageService.setRolesPermissions(
         workspaceId,
-        isPermissionsV2Enabled,
-        roleIds,
-      });
-
-    const freshObjectRecordsPermissionsByRoleId = roleIds
-      ? { ...currentRolesPermissions, ...recomputedRolesPermissions }
-      : recomputedRolesPermissions;
-
-    await this.workspacePermissionsCacheStorageService.setRolesPermissions(
-      workspaceId,
-      freshObjectRecordsPermissionsByRoleId,
-    );
-
-    await this.workspacePermissionsCacheStorageService.removeRolesPermissionsOngoingCachingLock(
-      workspaceId,
-    );
+        freshObjectRecordsPermissionsByRoleId,
+      );
+    } finally {
+      await this.workspacePermissionsCacheStorageService.removeRolesPermissionsOngoingCachingLock(
+        workspaceId,
+      );
+    }
   }
 
   async recomputeUserWorkspaceRoleMapCache({
@@ -121,19 +122,21 @@ export class WorkspacePermissionsCacheService {
       workspaceId,
     );
 
-    const freshUserWorkspaceRoleMap =
-      await this.getUserWorkspaceRoleMapFromDatabase({
+    try {
+      const freshUserWorkspaceRoleMap =
+        await this.getUserWorkspaceRoleMapFromDatabase({
+          workspaceId,
+        });
+
+      await this.workspacePermissionsCacheStorageService.setUserWorkspaceRoleMap(
         workspaceId,
-      });
-
-    await this.workspacePermissionsCacheStorageService.setUserWorkspaceRoleMap(
-      workspaceId,
-      freshUserWorkspaceRoleMap,
-    );
-
-    await this.workspacePermissionsCacheStorageService.removeUserWorkspaceRoleMapOngoingCachingLock(
-      workspaceId,
-    );
+        freshUserWorkspaceRoleMap,
+      );
+    } finally {
+      await this.workspacePermissionsCacheStorageService.removeUserWorkspaceRoleMapOngoingCachingLock(
+        workspaceId,
+      );
+    }
   }
 
   async getRolesPermissionsFromCache({
diff --git a/packages/twenty-server/src/engine/twenty-orm/factories/workspace-datasource.factory.ts b/packages/twenty-server/src/engine/twenty-orm/factories/workspace-datasource.factory.ts
index 774187cff..876a02d86 100644
--- a/packages/twenty-server/src/engine/twenty-orm/factories/workspace-datasource.factory.ts
+++ b/packages/twenty-server/src/engine/twenty-orm/factories/workspace-datasource.factory.ts
@@ -223,10 +223,11 @@ export class WorkspaceDatasourceFactory {
         this.workspacePermissionsCacheStorageService.getRolesPermissionsVersion(
           workspaceId,
         ),
-      recomputeCache: (params) =>
-        this.workspacePermissionsCacheService.recomputeRolesPermissionsCache(
-          params,
-        ),
+      recomputeCache: () =>
+        this.workspacePermissionsCacheService.recomputeRolesPermissionsCache({
+          workspaceId,
+          ignoreLock: true,
+        }),
       cachedEntityName: 'Roles permissions',
       exceptionCode: TwentyORMExceptionCode.ROLES_PERMISSIONS_VERSION_NOT_FOUND,
     });