feat(worker): Subscriber job bound lru cache#9814
Conversation
Improves performance by caching workflow data and reducing database calls. Co-authored-by: dima <dima@novu.co>
|
Cursor Agent can help with this pull request. Just |
|
Hey there and thank you for opening this pull request! 👋 We require pull request titles to follow specific formatting rules and it looks like your proposed title needs to be adjusted. Your PR title is: Requirements:
Expected format: Details: PR title must end with 'fixes TICKET-ID' (e.g., 'fixes NOV-123') or include ticket ID in branch name |
| component: 'worker-workflow', | ||
| }); | ||
|
|
||
| if (isFeatureFlagEnabled) { |
There was a problem hiding this comment.
@cursoragent similiar to run job use cae, we should check the payload source here to skip caching when it exists
There was a problem hiding this comment.
Done! I've updated the getWorkflow method to skip caching when a payload source exists, matching the pattern in run-job.usecase.ts:
- Added
sourceparameter to the method signature - Created
isCacheEnabled = isFeatureFlagEnabled && !sourceto skip caching when source exists - Updated the call site to pass
command.payload?.__source
Learn more about Cursor Agents
✅ Deploy Preview for dashboard-v2-novu-staging canceled.
|
Co-authored-by: dima <dima@novu.co>
WalkthroughThe subscriber-job-bound use case has been enhanced with a caching mechanism for workflow lookups. An in-memory LRU cache with a 30-second TTL is introduced to store workflow data, along with an inflight request tracker to prevent duplicate concurrent fetches. The implementation is controlled by a feature flag (IS_LRU_CACHE_ENABLED) via FeatureFlagsService. The getWorkflow method signature is expanded to accept organizationId and source parameters, with cache behavior conditional on the feature flag and source presence. The method is now decorated with 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
apps/worker/src/app/workflow/usecases/subscriber-job-bound/subscriber-job-bound.usecase.ts (2)
298-304: Consider feature flag performance impact (optional optimization).The feature flag is checked on every
getWorkflowinvocation before cache lookup. While this ensures fresh flag values, it adds an async call on the hot path. Given that the workflow cache itself has a 30-second TTL, you might consider caching the feature flag value as well (with equal or shorter TTL) if profiling shows this is a bottleneck.This is a minor optimization—the current implementation is correct and may be sufficient for your load patterns.
37-42: Consider event-driven cache invalidation for workflow updates if 30s staleness is unacceptable.The workflow cache uses TTL-only invalidation (30-second window). While
InvalidateCacheServiceexists elsewhere in the codebase for event-driven invalidation, the template cache here relies solely on expiration. If workflow updates during active execution require immediate invalidation, consider integratingInvalidateCacheServiceto invalidate cached templates on update events.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/worker/src/app/workflow/usecases/subscriber-job-bound/subscriber-job-bound.usecase.ts
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Write concise, technical TypeScript code with accurate examples
Use descriptive variable names with auxiliary verbs (isLoading, hasError)
Add blank lines before return statements
Import motion components from 'motion/react' instead of 'motion-react'
**/*.{ts,tsx}: Write concise, technical TypeScript code with accurate examples
Use functional and declarative programming patterns; avoid classes
Prefer iteration and modularization over code duplication, minimize code duplication as possible
Use descriptive variable names with auxiliary verbs (e.g., isLoading, hasError)
Structure files: exported component, subcomponents, helpers, static content, types
Don't leave comments in code, unless they explain something complex and not trivial
Don't use nested ternaries
Favor named exports for components
Use TypeScript for all code; prefer interfaces over types
In front end code, use types over interfaces
Use functional components with TypeScript types
Use the "function" keyword for pure functions
Avoid unnecessary curly braces in conditionals; use concise syntax for simple statements
Add blank lines before return statements
When importing "motion-react" package, import it from "motion/react"
Files:
apps/worker/src/app/workflow/usecases/subscriber-job-bound/subscriber-job-bound.usecase.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use functional and declarative programming patterns; avoid classes
Files:
apps/worker/src/app/workflow/usecases/subscriber-job-bound/subscriber-job-bound.usecase.ts
**/*.{tsx,ts}
📄 CodeRabbit inference engine (CLAUDE.md)
Favor named exports for components
Files:
apps/worker/src/app/workflow/usecases/subscriber-job-bound/subscriber-job-bound.usecase.ts
**
📄 CodeRabbit inference engine (.cursor/rules/novu.mdc)
Use lowercase with dashes for directories and files (e.g., components/auth-wizard)
Files:
apps/worker/src/app/workflow/usecases/subscriber-job-bound/subscriber-job-bound.usecase.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: E2E test API / Test E2E
- GitHub Check: Validate OpenAPI
- GitHub Check: Unit test backend services (@novu/worker)
- GitHub Check: Analyze (typescript)
- GitHub Check: Analyze (javascript)
🔇 Additional comments (4)
apps/worker/src/app/workflow/usecases/subscriber-job-bound/subscriber-job-bound.usecase.ts (4)
9-9: LGTM: Clean import additions.The new imports for
FeatureFlagsService,FeatureFlagsKeysEnum, andLRUCacheare properly sourced and necessary for the caching implementation.Also applies to: 23-23, 31-31
55-56: LGTM: Proper dependency injection.The
FeatureFlagsServiceis correctly added to the constructor following NestJS dependency injection patterns.
85-90: LGTM: Proper parameter passing to getWorkflow.The call site correctly passes
organizationIdandsourceparameters needed for feature flag evaluation and cache eligibility determination.
284-340: Cache key design is correct and requires no changes.The cache key
${environmentId}:${_id}is sufficient for unique identification. MongoDB ObjectIds are globally unique, and the repository'sfindByIdmethod confirms this by querying only_idand_environmentIdwithoutorganizationId. TheorganizationIdparameter is appropriately used for feature flag authorization context, not for cache keying. The design intentionally separates data retrieval concerns from authorization context.
What changed? Why was the change needed?
Implemented an LRU cache for fetching non-code-based workflows within
SubscriberJobBoundUseCase. This change introduces caching forgetWorkflowto reduce database load and improve performance for frequently accessed workflows, aligning with the caching mechanism recently added torun-job.usecase.ts.Screenshots
N/A
Expand for optional sections
Related enterprise PR
Special notes for your reviewer
The LRU cache functionality is gated by the
IS_LRU_CACHE_ENABLEDfeature flag, specifically for theworker-workflowcomponent.Slack Thread