admin/twenty
1
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

Fix cacheData not found after recomputes (#12032)

Browse Source 
Closes https://github.com/twentyhq/core-team-issues/issues/861

sentries: [User workspace role map not found after
recompute](https://twenty-v7.sentry.io/issues/6575092700/events/f9825338a30b470eb2345fe78c1e3479/?project=4507072499810304&query=is%3Aunresolved%20issue.priority%3A%5Bhigh%2C%20medium%5D%20not%20found%20after%20recompute&referrer=next-event&stream_index=0)
(64 events in 90d), [Feature flag map not found after
recompute](https://twenty-v7.sentry.io/issues/6547696076/?project=4507072499810304&query=is%3Aunresolved%20issue.priority%3A%5Bhigh%2C%20medium%5D%20not%20found%20after%20recompute&referrer=issue-stream&stream_index=1)
(23 events in 90d)

We have a structural issue with cached data and our locking mechanism:
if a data is missing in cache and queried at the same time by two
different entities, the first one will recompute the data and indicate a
lock during the operation, while the second one will seek to recompute
the data too but be stopped because of the ongoing lock, and be left
with no data to use, condemned to throw an error. In this PR I
considered that it was more important to avoid that error and chose to
ignoreLock instead when the data is being queried (through
getFromCacheWithRecompute), but this is maybe questionnable.
An important note is that I can't figure out how users that regularly
use twenty (as it has been the case since this error occured on our
internal workspace) can encounter that error as once computed, **the key
should always be present in the workspace**: the corresponding value it
is always updated, never deleted (until this PR) and recreated. I was
not able understand how this happened

For our data cached without a version to refer to in the database, I
also chose to ignore the lock when the recompute is triggered by a data
change (eg feature flag enabling or assigning user to a role or adding
an objectPermission on a role, etc.), as we never want to imped the
recompute in that case to avoid potential stale data.
This commit is contained in:
Marie
2025-05-14 15:41:24 +02:00
committed by GitHub
parent 83bdb1a515
commit 0c60fa9c23
10 changed files with 124 additions and 47 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
packages/twenty-server/src/engine/core-modules/feature-flag/services/feature-flag.service.ts
View File

@ -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,
},
);

2
packages/twenty-server/src/engine/metadata-modules/object-metadata/object-metadata.service.ts
View File

@ -214,6 +214,7 @@ export class ObjectMetadataService extends TypeOrmQueryService<ObjectMetadataEnt
await this.workspacePermissionsCacheService.recomputeRolesPermissionsCache({
workspaceId: objectMetadataInput.workspaceId,
ignoreLock: true,
});
return createdObjectMetadata;
@ -402,6 +403,7 @@ export class ObjectMetadataService extends TypeOrmQueryService<ObjectMetadataEnt
await this.workspacePermissionsCacheService.recomputeRolesPermissionsCache({
workspaceId,
ignoreLock: true,
});
return objectMetadata;

1
packages/twenty-server/src/engine/metadata-modules/object-permission/object-permission.service.ts
View File

@ -63,6 +63,7 @@ export class ObjectPermissionService {
{
workspaceId,
roleIds: [input.roleId],
ignoreLock: true,
},
);

3
packages/twenty-server/src/engine/metadata-modules/role/role.service.ts
View File

@ -85,6 +85,7 @@ export class RoleService {
await this.workspacePermissionsCacheService.recomputeRolesPermissionsCache({
workspaceId,
roleIds: [role.id],
ignoreLock: true,
});
return role;
@ -130,6 +131,7 @@ export class RoleService {
await this.workspacePermissionsCacheService.recomputeRolesPermissionsCache({
workspaceId,
roleIds: [input.id],
ignoreLock: true,
});
return { ...existingRole, ...updatedRole };
@ -196,6 +198,7 @@ export class RoleService {
await this.workspacePermissionsCacheService.recomputeRolesPermissionsCache({
workspaceId,
ignoreLock: true,
});
return roleId;

1
packages/twenty-server/src/engine/metadata-modules/user-role/user-role.service.ts
View File

@ -62,6 +62,7 @@ export class UserRoleService {
await this.workspacePermissionsCacheService.recomputeUserWorkspaceRoleMapCache(
{
workspaceId,
ignoreLock: true,
},
);
}

15
packages/twenty-server/src/engine/metadata-modules/workspace-feature-flags-map-cache/workspace-feature-flags-map-cache.service.ts
View File

@ -49,6 +49,7 @@ export class WorkspaceFeatureFlagsMapCacheService {
recomputeCache: (params) => 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(

14
packages/twenty-server/src/engine/metadata-modules/workspace-permissions-cache/workspace-permissions-cache-storage.service.ts
View File

@ -89,10 +89,8 @@ export class WorkspacePermissionsCacheStorageService {
async setUserWorkspaceRoleMap(
workspaceId: string,
userWorkspaceRoleMap: UserWorkspaceRoleMap,
): Promise<{
newUserWorkspaceRoleMapVersion: string;
}> {
const [, newUserWorkspaceRoleMapVersion] = await Promise.all([
): Promise<void> {
await Promise.all([
this.cacheStorageService.set<UserWorkspaceRoleMap>(
`${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<boolean | undefined> {

77
packages/twenty-server/src/engine/metadata-modules/workspace-permissions-cache/workspace-permissions-cache.service.ts
View File

@ -52,13 +52,21 @@ export class WorkspacePermissionsCacheService {
ignoreLock?: boolean;
roleIds?: string[];
}): Promise<void> {
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<void> {
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<CacheResult<string, UserWorkspaceRoleMap>> {
return getFromCacheWithRecompute<string, UserWorkspaceRoleMap>({
}): Promise<CacheResult<undefined, UserWorkspaceRoleMap>> {
return getFromCacheWithRecompute<undefined, UserWorkspaceRoleMap>({
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,
});
}

2
packages/twenty-server/src/engine/twenty-orm/factories/workspace-datasource.factory.ts
View File

@ -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,
});
}

52
packages/twenty-server/src/engine/utils/get-data-from-cache-with-recompute.util.ts
View File

@ -1,3 +1,5 @@
import { Logger } from '@nestjs/common';
import { isDefined } from 'twenty-shared/utils';
import {
@ -17,36 +19,68 @@ const getFromCacheWithRecompute = async <T, U>({
recomputeCache,
cachedEntityName,
exceptionCode,
logger,
}: {
workspaceId: string;
getCacheData: (workspaceId: string) => Promise<U | undefined>;
getCacheVersion: (workspaceId: string) => Promise<T | undefined>;
recomputeCache: (params: { workspaceId: string }) => Promise<void>;
getCacheVersion?: (workspaceId: string) => Promise<T | undefined>;
recomputeCache: (params: {
workspaceId: string;
ignoreLock?: boolean;
}) => Promise<void>;
cachedEntityName: string;
exceptionCode: TwentyORMExceptionCode;
logger: Logger;
}): Promise<CacheResult<T, U>> => {
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,
};
};