diff --git a/packages/twenty-server/src/engine/core-modules/feature-flag/services/feature-flag.service.ts b/packages/twenty-server/src/engine/core-modules/feature-flag/services/feature-flag.service.ts index 327e8a5af..bed48e12a 100644 --- a/packages/twenty-server/src/engine/core-modules/feature-flag/services/feature-flag.service.ts +++ b/packages/twenty-server/src/engine/core-modules/feature-flag/services/feature-flag.service.ts @@ -74,6 +74,7 @@ export class FeatureFlagService { await this.workspaceFeatureFlagsMapCacheService.recomputeFeatureFlagsMapCache( { workspaceId: workspaceId, + ignoreLock: true, }, ); } @@ -132,7 +133,8 @@ export class FeatureFlagService { await this.workspaceFeatureFlagsMapCacheService.recomputeFeatureFlagsMapCache( { - workspaceId: workspaceId, + workspaceId, + ignoreLock: true, }, ); diff --git a/packages/twenty-server/src/engine/metadata-modules/object-metadata/object-metadata.service.ts b/packages/twenty-server/src/engine/metadata-modules/object-metadata/object-metadata.service.ts index c8731d7ae..dcecc8db5 100644 --- a/packages/twenty-server/src/engine/metadata-modules/object-metadata/object-metadata.service.ts +++ b/packages/twenty-server/src/engine/metadata-modules/object-metadata/object-metadata.service.ts @@ -214,6 +214,7 @@ export class ObjectMetadataService extends TypeOrmQueryService this.recomputeFeatureFlagsMapCache(params), cachedEntityName: FEATURE_FLAG_MAP, exceptionCode: TwentyORMExceptionCode.FEATURE_FLAG_MAP_VERSION_NOT_FOUND, + logger: this.logger, }); } @@ -64,8 +65,18 @@ export class WorkspaceFeatureFlagsMapCacheService { workspaceId, ); - if (!ignoreLock && isAlreadyCaching) { - return; + if (isAlreadyCaching) { + if (ignoreLock) { + this.logger.warn( + `Feature flags map cache is already being cached (workspace ${workspaceId}), respecting lock and returning no data`, + ); + + return; + } else { + this.logger.warn( + `Feature flags map cache is already being cached (workspace ${workspaceId}), ignoring lock`, + ); + } } await this.workspaceCacheStorageService.addFeatureFlagMapOngoingCachingLock( diff --git a/packages/twenty-server/src/engine/metadata-modules/workspace-permissions-cache/workspace-permissions-cache-storage.service.ts b/packages/twenty-server/src/engine/metadata-modules/workspace-permissions-cache/workspace-permissions-cache-storage.service.ts index 1d05c15eb..a4b044b5c 100644 --- a/packages/twenty-server/src/engine/metadata-modules/workspace-permissions-cache/workspace-permissions-cache-storage.service.ts +++ b/packages/twenty-server/src/engine/metadata-modules/workspace-permissions-cache/workspace-permissions-cache-storage.service.ts @@ -89,10 +89,8 @@ export class WorkspacePermissionsCacheStorageService { async setUserWorkspaceRoleMap( workspaceId: string, userWorkspaceRoleMap: UserWorkspaceRoleMap, - ): Promise<{ - newUserWorkspaceRoleMapVersion: string; - }> { - const [, newUserWorkspaceRoleMapVersion] = await Promise.all([ + ): Promise { + await Promise.all([ this.cacheStorageService.set( `${WorkspaceCacheKeys.MetadataPermissionsUserWorkspaceRoleMap}:${workspaceId}`, userWorkspaceRoleMap, @@ -100,8 +98,6 @@ export class WorkspacePermissionsCacheStorageService { ), this.setUserWorkspaceRoleMapVersion(workspaceId), ]); - - return { newUserWorkspaceRoleMapVersion }; } async setUserWorkspaceRoleMapVersion(workspaceId: string) { @@ -146,6 +142,12 @@ export class WorkspacePermissionsCacheStorageService { ); } + removeUserWorkspaceRoleMap(workspaceId: string) { + return this.cacheStorageService.del( + `${WorkspaceCacheKeys.MetadataPermissionsUserWorkspaceRoleMap}:${workspaceId}`, + ); + } + getUserWorkspaceRoleMapOngoingCachingLock( workspaceId: string, ): Promise { 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 138d2326b..de41fde6a 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 @@ -52,13 +52,21 @@ export class WorkspacePermissionsCacheService { ignoreLock?: boolean; roleIds?: string[]; }): Promise { - if (!ignoreLock) { - const isAlreadyCaching = - await this.workspacePermissionsCacheStorageService.getRolesPermissionsOngoingCachingLock( - workspaceId, + const isAlreadyCaching = + await this.workspacePermissionsCacheStorageService.getRolesPermissionsOngoingCachingLock( + workspaceId, + ); + + if (isAlreadyCaching) { + if (ignoreLock) { + this.logger.warn( + `RolesPermissions data is already being cached (workspace ${workspaceId}), ignoring lock`, + ); + } else { + this.logger.warn( + `RolesPermissions data is already being cached (workspace ${workspaceId}), respecting lock and returning no data`, ); - if (isAlreadyCaching) { return; } } @@ -112,33 +120,48 @@ export class WorkspacePermissionsCacheService { workspaceId: string; ignoreLock?: boolean; }): Promise { - if (!ignoreLock) { + try { const isAlreadyCaching = await this.workspacePermissionsCacheStorageService.getUserWorkspaceRoleMapOngoingCachingLock( workspaceId, ); if (isAlreadyCaching) { - return; + if (ignoreLock) { + this.logger.warn( + `UserWorkspaceRoleMap data is already being cached (workspace ${workspaceId}), ignoring lock`, + ); + } else { + this.logger.warn( + `UserWorkspaceRoleMap data is already being cached (workspace ${workspaceId}), respecting lock and returning no data`, + ); + + return; + } } - } - await this.workspacePermissionsCacheStorageService.addUserWorkspaceRoleMapOngoingCachingLock( - workspaceId, - ); - - try { - const freshUserWorkspaceRoleMap = - await this.getUserWorkspaceRoleMapFromDatabase({ - workspaceId, - }); - - await this.workspacePermissionsCacheStorageService.setUserWorkspaceRoleMap( + await this.workspacePermissionsCacheStorageService.addUserWorkspaceRoleMapOngoingCachingLock( workspaceId, - freshUserWorkspaceRoleMap, ); - } finally { - await this.workspacePermissionsCacheStorageService.removeUserWorkspaceRoleMapOngoingCachingLock( + + try { + const freshUserWorkspaceRoleMap = + await this.getUserWorkspaceRoleMapFromDatabase({ + workspaceId, + }); + + await this.workspacePermissionsCacheStorageService.setUserWorkspaceRoleMap( + workspaceId, + freshUserWorkspaceRoleMap, + ); + } finally { + await this.workspacePermissionsCacheStorageService.removeUserWorkspaceRoleMapOngoingCachingLock( + workspaceId, + ); + } + } catch (error) { + // Flush stale userWorkspaceRoleMap + await this.workspacePermissionsCacheStorageService.removeUserWorkspaceRoleMap( workspaceId, ); } @@ -162,6 +185,7 @@ export class WorkspacePermissionsCacheService { recomputeCache: (params) => this.recomputeRolesPermissionsCache(params), cachedEntityName: ROLES_PERMISSIONS, exceptionCode: TwentyORMExceptionCode.ROLES_PERMISSIONS_VERSION_NOT_FOUND, + logger: this.logger, }); } @@ -169,22 +193,19 @@ export class WorkspacePermissionsCacheService { workspaceId, }: { workspaceId: string; - }): Promise> { - return getFromCacheWithRecompute({ + }): Promise> { + return getFromCacheWithRecompute({ workspaceId, getCacheData: () => this.workspacePermissionsCacheStorageService.getUserWorkspaceRoleMap( workspaceId, ), - getCacheVersion: () => - this.workspacePermissionsCacheStorageService.getUserWorkspaceRoleMapVersion( - workspaceId, - ), recomputeCache: (params) => this.recomputeUserWorkspaceRoleMapCache(params), cachedEntityName: USER_WORKSPACE_ROLE_MAP, exceptionCode: TwentyORMExceptionCode.USER_WORKSPACE_ROLE_MAP_VERSION_NOT_FOUND, + logger: this.logger, }); } 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 9a75a5f5b..254afad52 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 @@ -234,10 +234,10 @@ export class WorkspaceDatasourceFactory { recomputeCache: () => this.workspacePermissionsCacheService.recomputeRolesPermissionsCache({ workspaceId, - ignoreLock: true, }), cachedEntityName: ROLES_PERMISSIONS, exceptionCode: TwentyORMExceptionCode.ROLES_PERMISSIONS_VERSION_NOT_FOUND, + logger: this.logger, }); } diff --git a/packages/twenty-server/src/engine/utils/get-data-from-cache-with-recompute.util.ts b/packages/twenty-server/src/engine/utils/get-data-from-cache-with-recompute.util.ts index c46762b31..dfed3a206 100644 --- a/packages/twenty-server/src/engine/utils/get-data-from-cache-with-recompute.util.ts +++ b/packages/twenty-server/src/engine/utils/get-data-from-cache-with-recompute.util.ts @@ -1,3 +1,5 @@ +import { Logger } from '@nestjs/common'; + import { isDefined } from 'twenty-shared/utils'; import { @@ -17,36 +19,68 @@ const getFromCacheWithRecompute = async ({ recomputeCache, cachedEntityName, exceptionCode, + logger, }: { workspaceId: string; getCacheData: (workspaceId: string) => Promise; - getCacheVersion: (workspaceId: string) => Promise; - recomputeCache: (params: { workspaceId: string }) => Promise; + getCacheVersion?: (workspaceId: string) => Promise; + recomputeCache: (params: { + workspaceId: string; + ignoreLock?: boolean; + }) => Promise; cachedEntityName: string; exceptionCode: TwentyORMExceptionCode; + logger: Logger; }): Promise> => { let cachedVersion: T | undefined; let cachedData: U | undefined; - cachedVersion = await getCacheVersion(workspaceId); + const expectCacheVersion = isDefined(getCacheVersion); + + if (expectCacheVersion) { + cachedVersion = await getCacheVersion(workspaceId); + } + cachedData = await getCacheData(workspaceId); - if (!isDefined(cachedData) || !isDefined(cachedVersion)) { - await recomputeCache({ workspaceId }); + if ( + !isDefined(cachedData) || + (expectCacheVersion && !isDefined(cachedVersion)) + ) { + logger.warn( + `Triggering cache recompute for ${cachedEntityName} (workspace ${workspaceId})`, + { + cachedVersion, + cachedData, + }, + ); + await recomputeCache({ workspaceId, ignoreLock: true }); cachedData = await getCacheData(workspaceId); - cachedVersion = await getCacheVersion(workspaceId); + if (expectCacheVersion) { + cachedVersion = await getCacheVersion(workspaceId); + } - if (!isDefined(cachedData) || !isDefined(cachedVersion)) { + if ( + !isDefined(cachedData) || + (expectCacheVersion && !isDefined(cachedVersion)) + ) { + logger.warn( + `Data still missing after recompute for ${cachedEntityName} (workspace ${workspaceId})`, + { + cachedVersion, + cachedData, + }, + ); throw new TwentyORMException( - `${cachedEntityName} not found after recompute for workspace ${workspaceId}`, + `${cachedEntityName} not found after recompute for workspace ${workspaceId} (missingData: ${!isDefined(cachedData)}, missingVersion: ${expectCacheVersion && !isDefined(cachedVersion)})`, exceptionCode, ); } } return { - version: cachedVersion, + version: cachedVersion as T, data: cachedData, }; };