Skip to content

Commit

Permalink
Merge #47616 #47727
Browse files Browse the repository at this point in the history
47616: kv/concurrency: remove TODO about impossible deadlock scenario r=nvanbenschoten a=nvanbenschoten

This commit removes a TODO that described a potential scenario in which a request-only dependency cycle could deadlock due to a simultaneous transaction abort. I realized after writing a full fix (9a9182f) that such a deadlock was not achievable in practice due to the semantics of the lockTable.

> It may appear that there is a bug here in the handling of request-only
> dependency cycles. If such a cycle was broken by simultaneously aborting
> the transactions responsible for each of the request, there would be no
> guarantee that an aborted pusher would notice that its own transaction
> was aborted before it notices that its pushee's transaction was aborted.
> For example, in the simplest case, imagine two requests deadlocked on
> each other. If their transactions are both aborted and each push notices
> the pushee is aborted first, they will both return here triumphantly and
> wait for the other to exit its lock wait-queues, leading to deadlock.
> Even if they eventually pushed each other again, there would be no
> guarantee that the same thing wouldn't happen.
>
> However, such a situation is not possible in practice because such a
> dependency cycle is never constructed by the lockTable. The lockTable
> assigns each request a monotonically increasing sequence number upon its
> initial entrance to the lockTable. This sequence number is used to
> straighten out dependency chains of requests such that a request only
> waits on conflicting requests with lower sequence numbers than its own
> sequence number. This behavior guarantees that request-only dependency
> cycles are never constructed by the lockTable. Put differently, all
> dependency cycles must include at least one dependency on a lock and,
> therefore, one call to pushLockTxn. Unlike pushRequestTxn, pushLockTxn
> actively removes the conflicting lock and removes the dependency when it
> determines that its pushee transaction is aborted. This means that the
> call to pushLockTxn will continue to make forward progress in the case of
> a simultaneous abort of all transactions behind the members of the cycle,
> preventing such a hypothesized deadlock from ever materializing.

The PR also includes a good amount of other cleanup that I was intending to land with the fix to this deadlock. For instance, it splits off a `LockAcquisition` type from the `LockUpdate` type. It also cleans up the internals of `lock_table.go`.

47727: server: various server construction cleanup r=nvanbenschoten a=nvanbenschoten

This is all cleanup I stumbled upon while catching up on the changes made in the following PRs:
- #46843
- #46975
- #47509
- #47517
- #47570

It's all pretty minor.

Co-authored-by: Nathan VanBenschoten <[email protected]>
  • Loading branch information
craig[bot] and nvanbenschoten committed Apr 22, 2020
3 parents f02a014 + 6ac38be + 8ba5fe5 commit a1296f9
Show file tree
Hide file tree
Showing 41 changed files with 1,316 additions and 576 deletions.
337 changes: 297 additions & 40 deletions c-deps/libroach/protos/roachpb/data.pb.cc

Large diffs are not rendered by default.

284 changes: 256 additions & 28 deletions c-deps/libroach/protos/roachpb/data.pb.h

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit a1296f9

Please sign in to comment.