Use idempotency key index when finding existing runs by idempotency key in batch #1677
Conversation
|
WalkthroughThis update refactors the batch processing logic in two modules. In the BatchTriggerV3Service, the #prepareRunData method now groups items by their taskIdentifier and performs parallel database queries using Promise.all, while also enhancing error handling for unique constraint violations. In the allV2TestTask function, an additional context parameter has been added and payloads now include an options property with an idempotencyKey generated via randomUUID to strengthen idempotency during task triggers. Changes
Sequence Diagram(s)sequenceDiagram
participant Service as BatchTriggerV3Service
participant Group as Item Grouper
participant DB as Database
participant Logger as Error Logger
Service->>Group: Group items by taskIdentifier
Group-->>Service: Grouped items
par Parallel Queries
Service->>DB: findMany(query for taskIdentifier 1)
Service->>DB: findMany(query for taskIdentifier 2)
end
DB-->>Service: Return cached runs
alt Unique constraint violation detected
Service->>Logger: Log debug message (item exists)
end
sequenceDiagram
participant Caller as Task Initiator
participant Task as allV2TestTask
participant Batch as Batch Trigger
Caller->>Task: Call run({ triggerSequentially? }, { ctx })
Task->>Task: Generate payload with options { idempotencyKey: randomUUID() }
Task->>Batch: Trigger batch with payload
Batch-->>Task: Return result
Task-->>Caller: Return execution outcome
Possibly related PRs
Suggested reviewers
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 ESLint
references/v3-catalog/src/trigger/batch.tsOops! Something went wrong! :( ESLint: 8.45.0 ESLint couldn't find the config "custom" to extend from. Please check that the name of the config is correct. The config "custom" was referenced from the config file in "/.eslintrc.js". If you still have problems, please stop by https://eslint.org/chat/help to chat with the team. apps/webapp/app/v3/services/batchTriggerV3.server.tsOops! Something went wrong! :( ESLint: 8.45.0 ESLint couldn't find the config "custom" to extend from. Please check that the name of the config is correct. The config "custom" was referenced from the config file in "/.eslintrc.js". If you still have problems, please stop by https://eslint.org/chat/help to chat with the team. ✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/webapp/app/v3/services/batchTriggerV3.server.ts (1)
343-365: Excellent optimization of database queries.The changes improve performance by:
- Fetching cached runs for each task identifier separately
- Using Promise.all for parallel execution
- Leveraging database indexes effectively through the taskIdentifier
This approach significantly reduces database load by:
- Making better use of indexes
- Reducing the number of database roundtrips
- Enabling parallel query execution
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/webapp/app/v3/services/batchTriggerV3.server.ts(1 hunks)references/v3-catalog/src/trigger/batch.ts(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: units / 🧪 Unit Tests
- GitHub Check: typecheck / typecheck
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (3)
references/v3-catalog/src/trigger/batch.ts (2)
129-129: LGTM: Added context parameter to run method.The addition of the
ctxparameter allows access to the execution context, which is a good practice for better context management.
132-146: LGTM: Added idempotency keys for batch items.The addition of unique idempotency keys using
randomUUID()for each batch item is a good practice that ensures:
- Each batch item has a unique identifier
- Prevents duplicate processing
- Enables proper caching and retrieval
apps/webapp/app/v3/services/batchTriggerV3.server.ts (1)
332-341: LGTM: Optimized item grouping by taskIdentifier.The implementation efficiently groups items by their taskIdentifier using reduce, which helps in optimizing subsequent database queries.
Summary by CodeRabbit