Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

kvflowcontroller: mutex contention #105508

Closed
Tracked by #104154
erikgrinaker opened this issue Jun 24, 2023 · 2 comments · Fixed by #109170
Closed
Tracked by #104154

kvflowcontroller: mutex contention #105508

erikgrinaker opened this issue Jun 24, 2023 · 2 comments · Fixed by #109170
Assignees
Labels
A-admission-control C-performance Perf of queries or internals. Solution not expected to change functional behavior. T-kv KV Team

Comments

@erikgrinaker
Copy link
Contributor

erikgrinaker commented Jun 24, 2023

kv0/enc=false/nodes=3/cpu=96 is showing significant mutex contention on kvflowcontroller.Controller.mu:

Screenshot 2023-06-24 at 15 41 42

This is possibly the cause of a significant performance regression on this benchmark:

Screenshot 2023-06-24 at 15 42 40

Jira issue: CRDB-29088

@irfansharif
Copy link
Contributor

irfansharif commented Jul 7, 2023

Here are two runs of this same roachtest, with and without kvflowcontrol:

image image

That's a 21% reduction in throughput, likely what we're seeing in roachperf above. Looking.

irfansharif added a commit to irfansharif/cockroach that referenced this issue Jul 8, 2023
@irfansharif
Copy link
Contributor

Clawed back the throughput loss with #106466.

image

irfansharif added a commit to irfansharif/cockroach that referenced this issue Aug 21, 2023
Fixes cockroachdb#105508.

Under kv0/enc=false/nodes=3/cpu=96 we observed significant mutex
contention on kvflowcontroller.Controller.mu. We were using a single
mutex to adjust flow tokens across all replication streams. There's a
natural sharding available here - by replication stream - that
eliminates the contention and fixes the throughput drop.

The kv0 test surfaced other performance optimizations (mutex contention,
allocated objects, etc.) that we'll address in subsequent PRs.

Release note: None
craig bot pushed a commit that referenced this issue Aug 23, 2023
108444: plpgsql: add support for WHILE loops r=mgartner a=mgartner

#### plpgsql/parser: parse WHILE loops

Release note: None

#### opt: support PL/pgSQL WHILE loops

This commit adds support for PL/pgSQL `WHILE` loops. A `WHILE` loop is
syntactic sugar for a `LOOP` with that exits conditionally, so we build
a `WHILE` loop with a simple transformation:

    WHILE [cond] LOOP
      [body];
    END LOOP;
    =>
    LOOP
      IF [cond] THEN
        [body];
      ELSE
        EXIT;
      END IF;
    END LOOP;

This transformation allows us to avoid adding complicated logic
specifically for `WHILE` loops and instead rely on the existing
implementations for `LOOP` and `IF/ELSE` statements.

Epic: CRDB-799

Release note: None

#### opt: add unimplemented errors for EXIT/CONTINUE WHEN

Release note: None


109170: kvflowcontroller: eliminate mutex contention r=irfansharif a=irfansharif

Fixes #105508.

Under kv0/enc=false/nodes=3/cpu=96 we observed significant mutex contention on kvflowcontroller.Controller.mu. We were using a single mutex to adjust flow tokens across all replication streams. There's a natural sharding available here - by replication stream - that eliminates the contention and fixes the throughput drop.

The kv0 test surfaced other performance optimizations (mutex contention, allocated objects, etc.) that we'll address in subsequent PRs.

Release note: None

Co-authored-by: Marcus Gartner <[email protected]>
Co-authored-by: irfan sharif <[email protected]>
@craig craig bot closed this as completed in 4d81146 Aug 23, 2023
@github-project-automation github-project-automation bot moved this to Closed in KV Aug 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-admission-control C-performance Perf of queries or internals. Solution not expected to change functional behavior. T-kv KV Team
Projects
No open projects
Archived in project
2 participants