When applying transactions, acquire a read lock sooner
Summary:
Depends on D20461. Ref T13276. Ref T13054.
Currently, we acquire the transaction read lock after populating "old values" in transactions and filtering transactions with no effect.
This isn't early enough to prevent all weird chaotic races: if two processes try to apply a "close revision" transaction at the same time, this can happen:
PROCESS A PROCESS B Old Value = Open Old Value = Open Transaction OK: Yes Transaction OK: Yes Acquire Read Lock Acquire Read Lock Got Read Lock! Wait... Apply Transactions Wait... New Value = Closed Wait... Release Lock Wait... Got Read Lock! Apply Transactions New Value = Closed Release Lock
That's not great: both processes apply an "Open -> Closed" transaction since this was a valid transaction from the viewpoint of each process when it did the checks.
We actually want this:
PROCESS A PROCESS B Acquire Read Lock Acquire Read Lock Got Read Lock! Wait... Old Value = Open Wait... Transaction OK: Yes Wait... Apply Transactions Wait... New Value = Closed Wait... Release Lock Wait... Got Read Lock! >>> Old Value = Closed >>> Transaction Has No Effect! >>> Do Nothing / Abort Release Lock
Move the "lock" part up so we do that.
This may cause some kind of weird second-order effects, but T13054 went through pretty cleanly and we have to do this to get correct behavior, so we can survive those if/when they arise.
Test Plan:
- Added a sleep(10) before the lock.
- Ran bin/repository message --reparse X in two console windows, where X is a commit that closes revision Y and Y is open.
- Before patch: both windows closed the revision and added duplicate transactions.
- After patch: only one of the processes had an effect.
Reviewers: amckinley
Reviewed By: amckinley
Subscribers: jmeador
Maniphest Tasks: T13276, T13054
Differential Revision: https://secure.phabricator.com/D20462