Use repeatable read for pnl tick generation. #2578
Conversation
WalkthroughThe changes involve modifying the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🔇 Additional comments (3)indexer/services/roundtable/src/tasks/create-pnl-ticks.ts (3)
The IsolationLevel import is correctly placed with other postgres-related imports and is necessary for the transaction isolation level modification. Line range hint Since getPnlTicksCreateObjects is now called within a REPEATABLE_READ transaction, it should be documented whether this function:
Let's check the implementation: #!/bin/bash
# Description: Analyze getPnlTicksCreateObjects implementation
# Look for database queries in the function
ast-grep --pattern 'function getPnlTicksCreateObjects($_) {
$$$
}'
# Check for any explicit transaction isolation requirements in comments
rg -B 2 "getPnlTicksCreateObjects"
The implementation correctly sets the isolation level to REPEATABLE_READ immediately after starting the transaction, which will ensure consistent reads across the transaction as intended. However, this higher isolation level might impact performance due to additional locking overhead. Consider monitoring these metrics after deployment:
Let's verify the transaction usage pattern: ✅ Verification successfulBased on the shell script results, I can now provide a final response: REPEATABLE_READ isolation level is appropriate for PNL tick creation The implementation correctly uses REPEATABLE_READ isolation level for the following reasons:
The performance impact should be minimal because:
🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Description: Check for other transaction isolation level settings and potential conflicts
# Look for other transaction isolation level settings
rg -A 5 "setIsolationLevel"
# Look for concurrent operations on transfers and perpetual_positions tables
ast-grep --pattern 'Transaction.start()'
Length of output: 3975 Script: #!/bin/bash
# Let's check for transaction isolation level settings and transaction patterns
# Look for transaction isolation level settings
rg -A 5 "IsolationLevel\." --type ts
# Look for transaction start patterns with context
rg -B 2 -A 5 "Transaction\.start\(\)" --type ts
# Look for tables accessed within the transaction
rg -A 10 "getPnlTicksCreateObjects" indexer/services/roundtable/src/helpers/pnl-ticks-helper.ts
Length of output: 21643 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 (
|
(cherry picked from commit 898fb93)
Co-authored-by: vincentwschau <[email protected]>
Changelist
By default all transactions in postgres use the
read committedisolation level. This means that anySELECTwithin a transaction will only see data that was committed before the select was run. However, this means thatSELECTs within a transaction done at different times may see different data. This is an issue for the PnL task which gets data from thetransferstable and theperpetual_positionstable to determine the PnL snapshot of a subaccount, as there can be a race condition between reading the transfers and perpetual positions ifenderis updating both tables.Instead we should use the
repeatable readtransaction isolation level, that guarantees within a transaction allSELECTs will read the same snapshot of data. We only use this for the read-only transaction in the pnl task to ensure no rollbacks / retries need to be added.Transaction isolation level reference
Test Plan
Unit tests, running in a deployed environment.
Author/Reviewer Checklist
state-breakinglabel.indexer-postgres-breakinglabel.PrepareProposalorProcessProposal, manually add the labelproposal-breaking.feature:[feature-name].backport/[branch-name].refactor,chore,bug.Summary by CodeRabbit
New Features
Bug Fixes