Skip to content

kv,kvcoord: assign high priority for admission control if the txn has…#69337

Merged
craig[bot] merged 1 commit intocockroachdb:masterfrom
sumeerbhola:admission_lock_inversion
Aug 25, 2021
Merged

kv,kvcoord: assign high priority for admission control if the txn has…#69337
craig[bot] merged 1 commit intocockroachdb:masterfrom
sumeerbhola:admission_lock_inversion

Conversation

@sumeerbhola
Copy link
Copy Markdown
Collaborator

@sumeerbhola sumeerbhola commented Aug 24, 2021

… locks

This is a crude way to limit priority inversion where a txn holding locks
could be waiting in an admission queue while admitted requests are waiting
in the lock table queues for this txn to make progress and release locks.

It can also fare better than no admission control, since work from txns
holding locks will get prioritized, versus no prioritization in goroutine
scheduler.

A tpcc run with 3000 warehouses shows 2x reduction in lock waiters and
10+% improvement in txn throughput with this change (both before and
after experiments running with admission control enabled). When compared
with admission control disabled, we see even higher improvements in lock
waiters and txn throughput.

Some before/after graphs from running tpcc with 3000 warehouses, and
comparison with no admission control.

before (lock waiters):
Screen Shot 2021-08-24 at 5 53 48 PM
after (lock waiters):
Screen Shot 2021-08-24 at 5 54 14 PM
no admission control (lock waiters):
Screen Shot 2021-08-24 at 7 53 10 PM

before (txn throughput and latency):
Screen Shot 2021-08-24 at 5 53 01 PM
after (txn throughput and latency):
Screen Shot 2021-08-24 at 5 53 17 PM
no admission control (txn throughput and latency):
Screen Shot 2021-08-24 at 7 52 12 PM

Informs #65955

Release note: None

Release justification: Low-risk update to new functionality.

@sumeerbhola sumeerbhola requested a review from a team as a code owner August 24, 2021 22:30
@cockroach-teamcity
Copy link
Copy Markdown
Member

This change is Reviewable

Copy link
Copy Markdown
Member

@RaduBerinde RaduBerinde left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm: What a dramatic difference in tail latencies!

Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @ajwerner)

… locks

This is a crude way to limit priority inversion where a txn holding locks
could be waiting in an admission queue while admitted requests are waiting
in the lock table queues for this txn to make progress and release locks.

It can also fare better than no admission control, since work from txns
holding locks will get prioritized, versus no prioritization in goroutine
scheduler.

A tpcc run with 3000 warehouses shows 2x reduction in lock waiters and
10+% improvement in txn throughput with this change (both before and
after experiments running with admission control enabled). When compared
with admission control disabled, we see even higher improvements in lock
waiters and txn throughput.

Informs cockroachdb#65955

Release note: None

Release justification: Low-risk update to new functionality.
@sumeerbhola sumeerbhola force-pushed the admission_lock_inversion branch from 590e667 to 120ac09 Compare August 24, 2021 23:59
Copy link
Copy Markdown
Collaborator Author

@sumeerbhola sumeerbhola left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TFTR!
I've also done a run with no admission control, since lack of prioritization of lock holders could make that worse. The throughput and latency is worse with no admission control. See attached screenshots. In this experiment both cpu and storage became a bottleneck in one node, whose runnable goroutines exceeded 100 and L0 sub-levels exceeded 30.

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (and 1 stale) (waiting on @ajwerner)

@RaduBerinde
Copy link
Copy Markdown
Member

Very cool results!

@sumeerbhola
Copy link
Copy Markdown
Collaborator Author

bors r+

@craig
Copy link
Copy Markdown
Contributor

craig Bot commented Aug 25, 2021

Build succeeded:

@craig craig Bot merged commit 41614b3 into cockroachdb:master Aug 25, 2021
@sumeerbhola sumeerbhola deleted the admission_lock_inversion branch September 14, 2021 21:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants