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.

Sign up for GitHub

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

Jump to bottom

release-22.2: ui: change selection order of workload insights #92936

Merged
merged 1 commit into from
Dec 2, 2022
Merged

release-22.2: ui: change selection order of workload insights #92936

merged 1 commit into from
Dec 2, 2022

Conversation

maryliag
Copy link
Contributor

@maryliag maryliag commented Dec 2, 2022 •
edited
Loading

Previously, the Workload Insights view were first
Transaction and then Statements. Since 22.2 has more information on a
Statement level, this commit changes the order of views, so Statement is first
and Transactions is second.

Fixes #92931

Before
Screen Shot 2022-12-02 at 1 06 55 PM

After
Screen Shot 2022-12-02 at 1 03 54 PM

Release note (ui change): Switch order of Transaction and Statement
views on Workload Insights.

Release Justification: small ui change

@maryliag
ui: change selection order of workload insights
f0bc602 
Previously, the Workload Insights view were first
Transaction and then Statements. Since 22.2 has more information on a
Statement level, this commit changes the order of views, so Statement is first
and Transactions is second.

Fixes cockroachdb#92931

Release note (ui change): Switch order of Transaction and Statement
views on Workload Insights.
@maryliag maryliag requested a review from a team December 2, 2022 18:11
@blathers-crl
Copy link

blathers-crl bot commented Dec 2, 2022 •
edited by maryliag
Loading

Thanks for opening a backport.

Please check the backport criteria before merging:

  • Patches should only be created for serious issues or test-only changes.
  • Patches should not break backwards-compatibility.
  • Patches should change as little code as possible.
  • Patches should not change on-disk formats or node communication protocols.
  • Patches should not add new functionality.
  • Patches must not add, edit, or otherwise modify cluster versions; or add version gates.
If some of the basic criteria cannot be satisfied, ensure that the exceptional criteria are satisfied within.
  • There is a high priority need for the functionality that cannot wait until the next release and is difficult to address in another way.
  • The new functionality is additive-only and only runs for clusters which have specifically “opted in” to it (e.g. by a cluster setting).
  • New code is protected by a conditional check that is trivial to verify and ensures that it only runs for opt-in clusters.
  • The PM and TL on the team that owns the changed code have signed off that the change obeys the above rules.

Add a brief release justification to the body of your PR to justify this backport.

Some other things to consider:

  • What did we do to ensure that a user that doesn’t know & care about this backport, has no idea that it happened?
  • Will this work in a cluster of mixed patch versions? Did we test that?
  • If a user upgrades a patch version, uses this feature, and then downgrades, what happens?

@maryliag maryliag requested a review from kevin-v-ngo December 2, 2022 18:11
@cockroach-teamcity
Copy link
Member

This change is Reviewable

j82w
j82w approved these changes Dec 2, 2022
View reviewed changes
Copy link
Contributor

@j82w j82w left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewed 2 of 2 files at r1, all commit messages.
Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @kevin-v-ngo)

@maryliag maryliag merged commit 923796d into cockroachdb:release-22.2 Dec 2, 2022
@maryliag maryliag deleted the change-order branch December 2, 2022 20:51
@cockroach-teamcity cockroach-teamcity mentioned this pull request Dec 9, 2022
Open
ericharmeling added a commit to ericharmeling/cockroach that referenced this pull request Feb 16, 2023
@ericharmeling
ui: tenant-gate transaction insights, change selection order of workl…
09f1e61 
…oad insights

Related to cockroachdb#96353, cockroachdb#92936.

This commit hides the dropdown to switch workload insights from
statement insights to transaction insights on tenants. When cockroachdb#96353
is resolved, we can re-enable transaction insights for tenant
clusters.

The commit also switches the order of workload insights, to show
statement insights first (see cockroachdb#92936).

Release note: None
@ericharmeling ericharmeling mentioned this pull request Feb 16, 2023
Merged
ericharmeling added a commit to ericharmeling/cockroach that referenced this pull request Feb 17, 2023
@ericharmeling
ui: tenant-gate transaction insights, change selection order of workl…
508a9c2 
…oad insights

Related to cockroachdb#96353, cockroachdb#92936.

This commit hides the dropdown to switch workload insights from
statement insights to transaction insights on tenants. When cockroachdb#96353
is resolved, we can re-enable transaction insights for tenant
clusters.

The commit also switches the order of workload insights, to show
statement insights first (see cockroachdb#92936).

Release note: None
craig bot pushed a commit that referenced this pull request Feb 17, 2023
@irfansharif @ericharmeling
Merge #96642 #97256
c95bef0 
96642: kvflowhandle: implement kvflowcontrol.Handle r=irfansharif a=irfansharif

Part of #95563.

kvflowcontrol.Handle is used to interface with replication flow control;
it's typically backed by a node-level kvflowcontrol.Controller. Handles
are held on replicas initiating replication traffic, i.e. are both the
leaseholder and raft leader, and manage multiple streams underneath
(typically one per active member of the raft group).

When replicating log entries, these replicas choose the log position
(term+index) the data is to end up at, and use this handle to track the
token deductions on a per log position basis. When informed of admitted
log entries on the receiving end of the stream, we free up tokens by
specifying the highest log position up to which we've admitted
(below-raft admission, for a given priority, takes log position into
account -- see kvflowcontrolpb.AdmittedRaftLogEntries for more details).

We also extend the testing framework introduced in #95905 to also
support writing tests for kvflowcontrol.Handle -- it's now pulled into
its own kvflowsimulator package. We're able to write tests that look
like the following:

    # Set up a triply connected handle (to s1, s2, s3) and start issuing
    # writes at 1MiB/s. For two of the streams, return tokens at exactly
    # the rate its being deducted (1MiB/s). For the third stream (s3),
    # we return flow tokens at only 0.5MiB/s.
    timeline
    t=0s         handle=h op=connect    stream=t1/s1   log-position=1/0
    t=0s         handle=h op=connect    stream=t1/s2   log-position=1/0
    t=0s         handle=h op=connect    stream=t1/s3   log-position=1/0
    t=[0s,50s)   handle=h class=regular adjust=-1MiB/s   rate=10/s
    t=[0.2s,50s) handle=h class=regular adjust=+1MiB/s   rate=10/s stream=t1/s1
    t=[0.2s,50s) handle=h class=regular adjust=+1MiB/s   rate=10/s stream=t1/s2
    t=[0.2s,50s) handle=h class=regular adjust=+0.5MiB/s rate=10/s stream=t1/s3
    ----

    # Observe:
    # - Total available tokens flatlines at 32MiB since flow tokens for
    #   s3 eventually depletes and later bounces off of 0MiB. We
    #   initially have 3*16MiB = 48MiB worth of flow tokens, and end
    #   up at 48MiB-16MiB = 32MiB.
    # - Initially the rate of token deductions (3*1MiB/s = 3MiB/s) is
    #   higher than the token returns (1MiB/s+1MiB/s+0.5MiB/s =
    #   2.5MiB/s), but after we start shaping it to the slowest
    #   stream, they end up matching at (0.5MiB/s*3 = 1.5MiB/s).
    # - The blocked stream count bounces between 0 and 1 as the s3
    #   stream gets blocked/unblocked as tokens are
    #   deducted/returned. The demand for tokens (1MiB/s) is higher
    #   than the replenishment rate (0.5MiB/s).
    # - The overall admission rate is reduced from 30 reqs/s to 25
    #   reqs/s, mapping to token deduction rate of 3MiB/s to 2.5MiB/s
    #   (1MiB/s divvied across 10 reqs). The difference between 30
    #   reqs/s and 25 reqs/s is found in the +5 reqs/s accumulating in
    #   the wait queue.
    plot
    kvadmission.flow_controller.regular_tokens_available            unit=MiB
    kvadmission.flow_controller.regular_tokens_{deducted,returned}  unit=MiB/s rate=true
    kvadmission.flow_controller.regular_blocked_stream_count        unit=streams
    kvadmission.flow_controller.regular_requests_{admitted,waiting} unit=reqs/s rate=true
    ----
    ----
     47.7 ┼╮
     46.6 ┤╰─╮
     45.6 ┤  ╰─╮
     44.5 ┤    ╰╮
     43.5 ┤     ╰─╮
     42.4 ┤       ╰╮
     41.4 ┤        ╰─╮
     40.3 ┤          ╰─╮
     39.3 ┤            ╰╮
     38.2 ┤             ╰─╮
     37.2 ┤               ╰─╮
     36.1 ┤                 ╰╮
     35.1 ┤                  ╰─╮
     34.0 ┤                    ╰─╮
     33.0 ┤                      ╰╮
     31.9 ┤                       ╰───────────────
                regular_tokens_available (MiB)

     3.0 ┤╭───────────────────────╮
     2.8 ┤│                       │
     2.6 ┤╭────────────────────────╮
     2.4 ┤│                       ╰│
     2.2 ┤│                        │
     2.0 ┤│                        │
     1.8 ┤│                        │
     1.6 ┤│                        ╰─────────────
     1.4 ┤│
     1.2 ┤│
     1.0 ┤│
     0.8 ┤│
     0.6 ┤│
     0.4 ┤│
     0.2 ┤│
     0.0 ┼╯
          rate(regular_tokens_{deducted,returned}) (MiB/s)

     1.0 ┤                                 ╭╮   ╭
     0.9 ┤                            ╭╮   ││   │
     0.9 ┤                            ││   ││   │
     0.8 ┤                            ││   ││   │
     0.7 ┤                            ││   ││   │
     0.7 ┤                         ╭╮ ││╭╮ ││   │
     0.6 ┤                         ││ ││││ ││╭─╮│
     0.5 ┤                         │╰╮│││╰╮│││ ││
     0.5 ┤                         │ ││││ ││││ ││
     0.4 ┤                         │ ││││ ││││ ││
     0.3 ┤                         │ ││││ ││││ ││
     0.3 ┤                         │ ╰╯││ ││││ ││
     0.2 ┤                         │   ││ ╰╯╰╯ ╰╯
     0.1 ┤                        ╭╯   ╰╯
     0.1 ┤                        │
     0.0 ┼────────────────────────╯
           regular_blocked_stream_count (streams)

     30.0 ┤╭───────────────────────╮
     28.0 ┤│                       ╰╮
     26.0 ┤│                        ╰─────────────
     24.0 ┤│
     22.0 ┤│
     20.0 ┤│
     18.0 ┤│
     16.0 ┤│
     14.0 ┤│
     12.0 ┤│
     10.0 ┤│
      8.0 ┤│
      6.0 ┤│                        ╭─────────────
      4.0 ┤│                        │
      2.0 ┤│                       ╭╯
      0.0 ┼────────────────────────╯
           rate(regular_requests_{admitted,waiting}) (reqs/s)
    ----
    ----

Release note: None

97256: ui: tenant-gate transaction insights, change selection order of workload insights r=ericharmeling a=ericharmeling

Related to #96353, #92936.

This commit hides the dropdown to switch workload insights from statement insights to transaction insights on tenants. When #96353 is resolved, we can re-enable transaction insights for tenant clusters.

The commit also switches the order of workload insights, to show statement insights first (see #92936).

`@gemma-shay` We should add a note on the [Insights docs](https://www.cockroachlabs.com/docs/stable/ui-insights-page.html) and/or the serverless docs that Transaction Insights aren't available on serverless.

Release note: None

Epic: None

Co-authored-by: irfan sharif <irfanmahmoudsharif@gmail.com>
Co-authored-by: Eric Harmeling <eric.harmeling@cockroachlabs.com>
ericharmeling added a commit to ericharmeling/cockroach that referenced this pull request Feb 18, 2023
@ericharmeling
ui: tenant-gate transaction insights, change selection order of workl…
64e4392 
…oad insights

Related to cockroachdb#96353, cockroachdb#92936.

This commit hides the dropdown to switch workload insights from
statement insights to transaction insights on tenants. When cockroachdb#96353
is resolved, we can re-enable transaction insights for tenant
clusters.

The commit also switches the order of workload insights, to show
statement insights first (see cockroachdb#92936).

Release note: None
@ericharmeling ericharmeling mentioned this pull request Feb 18, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@j82w j82w j82w approved these changes

@kevin-v-ngo kevin-v-ngo Awaiting requested review from kevin-v-ngo

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@maryliag @cockroach-teamcity @j82w