Skip to content

Refactor workspace cache service#16208

Merged
Weiko merged 11 commits intomainfrom
c--refactor-workspace-cache-service
Dec 1, 2025
Merged

Refactor workspace cache service#16208
Weiko merged 11 commits intomainfrom
c--refactor-workspace-cache-service

Conversation

@Weiko
Copy link
Copy Markdown
Member

@Weiko Weiko commented Dec 1, 2025

Context

We've recently introduced a new workspace cache service which now acts as a cache access and local storage for all workspace related data, deprecating the individual specific services.

  • Better performance through multiple caching/fetching strategies
  • Consistent data access patterns across the codebase
  • Reduced redis queries through MGET/MSET/PIPELINE with multiple cache keys

Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

74 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Dec 1, 2025

🚀 Preview Environment Ready!

Your preview environment is available at: http://bore.pub:53705

This environment will automatically shut down when the PR is closed or after 5 hours.

@twentyhq twentyhq deleted a comment from greptile-apps bot Dec 1, 2025
@Weiko
Copy link
Copy Markdown
Member Author

Weiko commented Dec 1, 2025

@greptileai trigger

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Dec 1, 2025

Greptile Overview

Greptile Summary

This PR successfully consolidates workspace-level caching into a unified WorkspaceCacheService, replacing individual cache services with a provider-based pattern. The refactor delivers significant architectural improvements through multiple caching layers (local 100ms, memoized 10s, Redis), batch operations (MGET/MSET/MDEL), and hash-based cache validation.

Key Improvements:

  • Reduced local cache TTL from 30s to 100ms for faster invalidation while maintaining performance through memoization
  • Added strongly-typed cache keys with proper TypeScript inference
  • Implemented batch Redis operations via pipelining for improved performance
  • Simplified individual cache services by 50-70% LOC through provider pattern
  • Fixed previously flagged bug in workspace-roles-permissions-cache.service.ts:138
  • Added comprehensive test coverage for new cache service

Migration Impact:

  • 77 files changed with net -557 lines (1307 added, 1864 deleted)
  • All workspace-related cache services migrated to new pattern
  • Removed deprecated cache storage service and outdated test files
  • Module imports updated consistently across codebase

Confidence Score: 5/5

  • This PR is safe to merge with no critical issues found
  • The refactoring follows solid architectural patterns with comprehensive testing, proper type safety, and systematic migration across all affected services. The previously flagged bug has been fixed, and the implementation demonstrates good understanding of caching strategies with multiple layers of optimization.
  • No files require special attention

Important Files Changed

File Analysis

Filename Score Overview
packages/twenty-server/src/engine/workspace-cache/services/workspace-cache.service.ts 5/5 Core cache service refactored with improved type safety, reduced TTL from 30s to 100ms, added hash-based validation, and MGET/MSET for batch operations
packages/twenty-server/src/engine/workspace-cache/types/workspace-cache-key.type.ts 5/5 New type definitions providing strongly-typed cache keys with proper type inference and result mapping
packages/twenty-server/src/engine/metadata-modules/workspace-permissions-cache/workspace-roles-permissions-cache.service.ts 5/5 Migrated to new cache provider pattern, fixed loop assignment bug (line 138 now correctly outside inner loop)
packages/twenty-server/src/engine/workspace-cache/services/tests/workspace-cache.service.spec.ts 5/5 Comprehensive test suite covering cache initialization, TTL staleness, Redis interactions, invalidation, and multi-key operations
packages/twenty-server/src/engine/core-modules/cache-storage/services/cache-storage.service.ts 5/5 Added mdel and mset methods for batch Redis operations using pipelining
packages/twenty-server/src/engine/metadata-modules/workspace-feature-flags-map-cache/workspace-feature-flags-map-cache.service.ts 5/5 Simplified from 76 lines to 25 lines by extending WorkspaceCacheProvider and removing manual cache management

Sequence Diagram

sequenceDiagram
    participant Client as Service/Controller
    participant WCS as WorkspaceCacheService
    participant Local as Local Cache (Map)
    participant Memoizer as Promise Memoizer
    participant Redis as Redis (CacheStorageService)
    participant Provider as Cache Provider

    Client->>WCS: getOrRecompute(workspaceId, keys)
    WCS->>Memoizer: Check memoized result
    
    alt Result in Memoizer (< 10s)
        Memoizer-->>WCS: Return cached result
        WCS-->>Client: Return data
    else Not in Memoizer
        WCS->>Local: Check local cache (TTL 100ms)
        
        alt Fresh in Local Cache
            Local-->>WCS: Return fresh data
            WCS-->>Client: Return data
        else Stale in Local Cache
            WCS->>Redis: MGET hash keys
            Redis-->>WCS: Return hashes
            
            alt Hash matches local
                WCS->>Local: Update timestamp
                WCS-->>Client: Return local data
            else Hash mismatch or missing
                WCS->>Redis: MGET data keys
                Redis-->>WCS: Return data
                
                alt Data exists in Redis
                    WCS->>Local: Store with new hash
                    WCS-->>Client: Return Redis data
                else Data missing
                    WCS->>Provider: computeForCache(workspaceId)
                    Provider->>Provider: Query database
                    Provider-->>WCS: Return computed data
                    WCS->>Redis: MSET data + hash via pipeline
                    WCS->>Local: Store with hash
                    WCS->>Memoizer: Cache result
                    WCS-->>Client: Return computed data
                end
            end
        end
    end
    
    Note over Client,Provider: Invalidation Flow
    Client->>WCS: invalidateAndRecompute(workspaceId, keys)
    WCS->>Memoizer: clearKeys(prefix)
    WCS->>Redis: MDEL data + hash keys
    WCS->>Local: Delete entries
    WCS->>Provider: computeForCache(workspaceId)
    Provider-->>WCS: Fresh data
    WCS->>Redis: MSET data + hash
    WCS->>Local: Store with hash
    WCS-->>Client: Done
Loading

Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

77 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

return;
}

if (this.isRedisCache()) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should move to the driver but OK given the current state of other method of cache-storage service

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm


import { ALL_FLAT_ENTITY_MAPS_PROPERTIES } from 'src/engine/metadata-modules/flat-entity/constant/all-flat-entity-maps-properties.constant';
import { AllFlatEntityMaps } from 'src/engine/metadata-modules/flat-entity/types/all-flat-entity-maps.type';
import { WorkspaceFlatMapCacheRegistryService } from 'src/engine/workspace-flat-map-cache/services/workspace-flat-map-cache-registry.service';
import { WorkspaceFlatMapCacheService } from 'src/engine/workspace-flat-map-cache/services/workspace-flat-map-cache.service';
import { WorkspaceCacheService } from 'src/engine/workspace-cache/services/workspace-cache.service';

@Injectable()
export class WorkspaceManyOrAllFlatEntityMapsCacheService {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess we can get rid of this service?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(nitpick)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I'm in favor of this as well but I'm leaving as it is for now to avoid conflict with prastoin work. @charlesBochet

'rolesPermissions',
]);

const cachedFeatureFlagMapVersion = crypto
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a bit of surprise we have to compute this here, shouldn't it be the hashVersion directly?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's for legacy only, the cache service does not expose those keys and they are needed for the legacy datasource so I'm recomputing them again here. (this won't be needed later though) @charlesBochet

@@ -96,6 +96,12 @@ export class PromiseMemoizer<T> {
await this.clearKey(cacheKey, onDelete);
}
}

for (const cacheKey of [...this.pending.keys()]) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good catch

import { type AllFlatEntityMaps } from 'src/engine/metadata-modules/flat-entity/types/all-flat-entity-maps.type';
import { type UserWorkspaceRoleMap } from 'src/engine/metadata-modules/workspace-permissions-cache/types/user-workspace-role-map.type';

export const WORKSPACE_CACHE_KEYS_V2 = {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

found it :p

},
),
],
this.workspaceCacheService.invalidateAndRecompute(workspaceId, [
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

love it!

Copy link
Copy Markdown
Member

@charlesBochet charlesBochet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bravo!

@Weiko Weiko merged commit 1eb2e44 into main Dec 1, 2025
62 of 63 checks passed
@Weiko Weiko deleted the c--refactor-workspace-cache-service branch December 1, 2025 16:08
@twenty-eng-sync
Copy link
Copy Markdown

Hey @Weiko! After you've done the QA of your Pull Request, you can mark it as done here. Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants