fix: add isolation level in transaction repository #824
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
kushsharma
force-pushed
the
transaction-isolation-level
branch
from
9dc83a1
to
c1ec2e0
Compare
kushsharma
force-pushed
the
transaction-isolation-level
branch
from
c1ec2e0
to
25cde26
Compare
Currently there were chances to have race conditions while writing to transaction repository. I have added a test to verify it doesn't happen. Database is using repeatable read as the isolation level to avoid overlapping transactions. Signed-off-by: Kush Sharma <[email protected]>
kushsharma
force-pushed
the
transaction-isolation-level
branch
from
25cde26
to
b33447d
Compare
Pull Request Test Coverage Report for Build 12078725933Details
💛 - Coveralls |
| }); err != nil { | ||
| err := r.withRetry(ctx, func() error { | ||
| return r.dbc.WithTxn(ctx, sql.TxOptions{ | ||
| Isolation: sql.LevelRepeatableRead, |
There was a problem hiding this comment.
So if I understand correctly, we check balance by always looping over the table and calculating difference. We insert new rows for debit/credit. How will sql.LevelRepeatableRead prevent race condition here? Phantom reads and Write Skew are allowed in Repeatable Read as far I can see. It won't prevent new rows being written and hence change of balance mid transaction.
Can you test if this isolation is working fine? Maybe write a Python or Go code and hit the API concurrently 20-30 times for the same org. If let's say starting balance is 105, and we deduct 10 credit each call then only 10 calls should succeed and the balance should be 5 at the end.
There was a problem hiding this comment.
Agree, Serializable would be best and safest(with some added latency), that's what I proposed in the last PR. Although I don't see the issue happening in the test https://github.com/raystack/frontier/pull/824/files#diff-aaeca6450e0dcd11871799ac2b14da4225d7a76f94c04a9d4636e44ae780773eR1115
Currently there were chances to have race conditions while writing to transaction repository. I have added a test to verify it doesn't happen. Database is using repeatable read as the isolation level to avoid overlapping transactions.