Reduce contention on batchTaskRun when setting expected count #1662
Conversation
|
|
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
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. internal-packages/database/src/transaction.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. WalkthroughThe changes update the batch processing and database transaction logic. In the batch service, an Changes
Sequence Diagram(s)sequenceDiagram
participant C as Client
participant B as BatchTriggerV3Service
participant DB as Database
C->>B: Initiate batch processing
B->>B: Call #processBatchTaskRunItems
loop Process Each Item
B->>B: Call #processBatchTaskRunItem
B-->>B: Returns Boolean (success/failure)
B->>B: Increment expectedCount if successful
end
B->>B: Check if completed count == expectedCount
B->>DB: Update batch status to COMPLETED (with 10s timeout)
DB-->>B: Confirmation update
sequenceDiagram
participant App as Application
participant TX as $transaction Function
participant DB as Database
App->>TX: Execute transaction
TX->>DB: Run database operations
alt Serialization failure ("P2034")
TX->>TX: Check maxRetries and attempt count
TX->>DB: Retry transaction call (recursive)
else Successful operation
DB-->>TX: Return results
end
TX-->>App: Transaction result
Possibly related PRs
Poem
Tip 🌐 Web search-backed reviews and chat
✨ 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 (2)
apps/webapp/app/v3/services/batchTriggerV3.server.ts (2)
763-763: Name alignment.
expectedCounthere tracks how many new items are “expected” to complete based on creation. Consider a name likenewlyCreatedItemsor similar to distinguish it from theexpectedCountcolumn on the batch more explicitly.
990-992: Timeout choice.A 10-second transaction timeout may be sufficient for typical load, but under heavy database contention it could lead to frequent retries or partial updates. Validate this value in load testing.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/webapp/app/v3/services/batchTriggerV3.server.ts(5 hunks)internal-packages/database/src/transaction.ts(2 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: typecheck / typecheck
- GitHub Check: units / 🧪 Unit Tests
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (7)
internal-packages/database/src/transaction.ts (3)
33-39: Well-documented addition ofmaxRetries.The inline references to the Prisma docs provide clarity on when to adjust
maxRetries. This is a straightforward and helpful enhancement.
46-47: Consider clarifying recursion boundaries.Using
attempt = 0is fine as a default, but ensure large values ofmaxRetrieswon't inadvertently create very deep recursion if an error persists. You could potentially mitigate this by adding an upper bound check or considering an iterative retry approach for extreme edge cases.
57-63: Good handling of Prisma "P2034" retry logic.The condition ensures retries happen only for serialization failures, and the recursion is guarded by the
attempt < options.maxRetriescheck. This is a clean and robust solution.apps/webapp/app/v3/services/batchTriggerV3.server.ts (4)
767-776: Incrementing the batch’s “expected count.”Your approach of incrementing
expectedCountonly if an item is newly created avoids double counting, which is good. Ensure concurrency is handled if multiple chunks process in parallel; the final persisted value should accurately sum all newly created items across chunks.
794-803: Piecing together the “expectedCount.”Wrapping the single update in a separate logic block reduces the number of writes to the database, which helps avoid repeated row locks. This looks aligned with the PR’s goal of minimizing contention.
928-948: Use minimal update for changing item status.Marking the item as
"COMPLETED"only if it was previously"PENDING"is straightforward and prevents redundant updates. This helps keep the transaction light and reduces possible contention on high-traffic tables.
950-987: Batch finalization logic.Incrementing the
completedCountand checkingcompletedCount === expectedCount && sealedto mark the batchCOMPLETEDis clear. Note that race conditions can occur if multiple items complete simultaneously. Your check on the batch’sstatusbeing"PENDING"mitigates the risk of double-completing the same batch.
@trigger.dev/build
trigger.dev
@trigger.dev/react-hooks
@trigger.dev/core
@trigger.dev/rsc
@trigger.dev/sdk
commit: |
Summary by CodeRabbit