An install is seeing an issue where Phabricator's close-on-commit logic races external Conduit logic (via differental.createcomment) and both update the revision simultaneously. The revision state change from close-on-commit ends up being overwritten. Specifically, this sequence occurs:
- The worker fires close-on-commit.
- At around the same time, an external script fires differential.createcomment.
- The close-on-commit applies first.
- The differential.createcomment appears to undo the state change.
This is probably a slightly broader issue applying approximately to all edits on all objects.
The first-order fix is approximately to make shouldReadLock() return true by default (or possibly remove it entirely, so it is always-on).
A second-order fix may be to try to walk that back so that some subset of changes do not need read locks, but there's probably very little actual value in doing this: we don't expect writes to be exceptionally cheap and doing read locking should not generally change the overall cadence of edits.
Conpherence uses read locking already, so it is unlikely that enabling it has weird or far-reaching consequences.