Prevent race conditions in concurrent MLS commit requests

[stefanwire]

https://wearezeta.atlassian.net/browse/FS-437

Checklist

• The PR Title explains the impact of the change.
• The PR description provides context as to why the change should occur and what the code contributes to that effect. This could also be a link to a JIRA ticket or a Github issue, if there is one.
• If a cassandra schema migration has been added, I ran make git-add-cassandra-schema to update the cassandra schema documentation.
• changelog.d contains the following bits of information (details):
  • A file with the changelog entry in one or more suitable sub-sections. The sub-sections are marked by directories inside changelog.d.
  • If a cassandra schema migration is backwards incompatible (see also these docs), measures to be taken by on-premise instance operators are explained.

[smatting]

TODO left

[pcapriotti]

Looks good if CI agrees. Minor comments follow.

[pcapriotti]

Why not acquire the lock on the opening side of the bracket?

[stefanwire]

done.

[pcapriotti]

I'm surprised that this works. I was under the impression that you can't have TTL values as "question mark" parameters.

[stefanwire]

We tried it locally by temporarily removing the finishing part of the bracket and observed the entries appearing and finally disappearing after the TTL expired. Would this manual test count or should I look deeper into it?

[pcapriotti]

Ah, one more thing. What happens if the TTL expires before commit processing is finished? I think in that case there's a race between the thread that started the lock, and some potential threads receiving a competing commit, since the former behaves as if it is still holding the lock. For extra safety, maybe we should add a timeout T to the commit processing block, and correspondingly set the TTL for the lock to slightly more than T. This should minimise the probability of such failures.

[smatting]

Not sure if this improves things. If we add a timeout and the commit processing can't meet it, then it's quite likely we're left with a broken state. If we don't add a timeout and the commit processing doesn't finish in time we're losing the guarantee the lock gives, but then there's still the chance that no commit arrives before the commit processing is finished.

Alternatively we could increase the TTL of the lock to make this problem less likely to occur.