ui,admission: observability improvements for admission control by sumeerbhola · Pull Request #68595 · cockroachdb/cockroach

sumeerbhola · 2021-08-09T15:26:11Z

Trace statements for latency incurred in admission queues.
Certain admission control metrics are now included in the
overload dashboard. Specifically,
- Resource bottlenecks can be identified using the
  "KV Admission Slots" and "KV Admission IO Tokens Exhausted
  Duration Per Second" graphs.
- The rate at which admission control is admitting requests
  is in the "Admission Work Rate" graphs and the corresponding
  latency rate (for all requests) is in
  "Admission Latency Rate". Dividing the latter by the former
  gives the mean admission latency.
- The 75th percentile latency for those requests that actually
  waited for admission is in the
  "Admission Latency: 75th percentile" graph.
  When admission control is off most of these graphs will be
  empty or zero, and the total KV admission slots will be 1.

Release note (ui change): admission control metrics are added to
Overload dashboard.

cockroach-teamcity · 2021-08-09T15:26:18Z

This change is

sumeerbhola · 2021-08-09T16:34:10Z

@dhartunian I don't know what to make of the lint failure -- I did not add a Tooltip. Could you help?

[16:00:20]	    console.error
[16:00:20]	      Warning: An update to Tooltip inside a test was not wrapped in act(...).
[16:00:20]	      
[16:00:20]	      When testing, code that causes React state updates should be wrapped into act(...):
[16:00:20]	      
[16:00:20]	      act(() => {
[16:00:20]	        /* fire events that update state */
[16:00:20]	      });
[16:00:20]	      /* assert on the output */
[16:00:20]	      
[16:00:20]	      This ensures that you're testing the behavior the user would see in the browser. Learn more at https://fb.me/react-wrap-tests-with-act
[16:00:20]	          in Tooltip
[16:00:20]	          in td
[16:00:20]	          in Unknown
[16:00:20]	          in tr
[16:00:20]	          in Unknown (created by StatementsSortedTable)
[16:00:20]	          in tbody (created by StatementsSortedTable)
[16:00:20]	          in table (created by StatementsSortedTable)
[16:00:20]	          in div (created by StatementsSortedTable)
[16:00:20]	          in StatementsSortedTable
[16:00:20]	          in section
[16:00:20]	          in div
[16:00:20]	          in Unknown (created by StatementsPage)
[16:00:20]	          in div (created by StatementsPage)
[16:00:20]	          in StatementsPage
[16:00:20]	          in Router (created by MemoryRouter)
[16:00:20]	          in MemoryRouter (created by WrapperComponent)
[16:00:20]	          in WrapperComponent
[16:00:20]	
[16:00:20]	      at warningWithoutStack (node_modules/react-dom/cjs/react-dom.development.js:530:32)
[16:00:20]	      at warnIfNotCurrentlyActingUpdatesInDEV (node_modules/react-dom/cjs/react-dom.development.js:25848:7)
[16:00:20]	      at dispatchAction (node_modules/react-dom/cjs/react-dom.development.js:17072:9)
[16:00:20]	      at fn (node_modules/@cockroachlabs/ui-components/dist/main.js:25421:9)
[16:00:20]	      at Object.forceUpdate (node_modules/@cockroachlabs/ui-components/dist/main.js:365:21)
[16:00:20]	      at node_modules/@cockroachlabs/ui-components/dist/main.js:378:20
[16:00:20]	      at node_modules/@cockroachlabs/ui-components/dist/main.js:377:16

RaduBerinde

I think we'll want to do a review of all new admission-control-related graphs before release, together with PM

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

pkg/ui/src/views/cluster/containers/nodeGraphs/dashboards/overload.tsx, line 149 at r1 (raw file):

    <LineGraph title="Admission Latency Rate" sources={nodeSources}>
      <Axis label="latency rate (micros/sec)">

This is pretty obscure to be looking at directly.. It's hard to reason about what this rate means. I guess it is not possible to show the wait_sum / admitted graph without calculating a new metric?

pkg/ui/src/views/cluster/containers/nodeGraphs/dashboards/overload.tsx, line 179 at r1 (raw file):

    <LineGraph title="Admission Latency: 75th percentile" sources={nodeSources}>
      <Axis label="latency for requests that waited (nanos)">

"latency" can be confusing (could refer to the overall execution latency). Maybe admission delay or wait time?

nathanstilwell · 2021-08-18T13:12:49Z

@sumeerbhola Filling in for @dhartunian, the several warnings about using act(...) around Tooltips are unfortunately a distraction from the actual error which is,

    $ eslint './{src,ccl}/**/*.{tsx,ts,js}'
15:58:12     [BABEL] Note: The code generator has deoptimised the styling of /go/src/github.com/cockroachdb/cockroach/pkg/ui/node_modules/nise/nise.js as it exceeds the max of 500KB.
15:58:15     [BABEL] Note: The code generator has deoptimised the styling of /go/src/github.com/cockroachdb/cockroach/pkg/ui/node_modules/react-dom/cjs/react-dom.development.js as it exceeds the max of 500KB.
15:58:27     
15:58:27     /go/src/github.com/cockroachdb/cockroach/pkg/ui/src/views/cluster/containers/nodeGraphs/dashboards/overload.tsx
15:58:27       105:15  error  Replace `·title="KV·Admission·IO·Tokens·Exhausted·Duration·Per·Second"·sources={nodeSources}` with `⏎······title="KV·Admission·IO·Tokens·Exhausted·Duration·Per·Second"⏎······sources={nodeSources}⏎····`  prettier/prettier
15:58:27       206:18  error  Delete `⏎`                                                                                                                                                                                                 prettier/prettier
15:58:27     
15:58:27     ✖ 2 problems (2 errors, 0 warnings)
15:58:27       2 errors and 0 warnings potentially fixable with the `--fix` option.

We use a code formatter called Prettier as a plugin for our linter Eslint. Often folks working with React will have tools builtin to handle the formatting automatically. We do have a yarn command to auto-format files, but I'm sorry to say it is not something that is a part of the regular build in make. After making React/TypeScript/JavaScript changes, the following should remove these kinds of errors,

# from the root of cockroachdb repo
>  yarn --cwd pkg/ui run lint:fix
yarn run v1.22.10
$ eslint './{src,ccl}/**/*.{tsx,ts,js}' --fix
✨  Done in 17.28s.

# to view any lint error on the front-end (you can also use `make ui-lint`) 
>  yarn --cwd pkg/ui run lint
yarn run v1.22.10
$ eslint './{src,ccl}/**/*.{tsx,ts,js}'
✨  Done in 17.73s.

Please let me know if you run into any issues or if I can provide more clarification.

- Trace statements for latency incurred in admission queues. - Certain admission control metrics are now included in the overload dashboard. Specifically, - Resource bottlenecks can be identified using the "KV Admission Slots" and "KV Admission IO Tokens Exhausted Duration Per Second" graphs. - The rate at which admission control is admitting requests is in the "Admission Work Rate" graphs and the corresponding delay rate (for all requests) is in "Admission Delay Rate". Dividing the latter by the former gives the mean admission delay. - The 75th percentile delay for those requests that actually waited for admission is in the "Admission Delay: 75th percentile" graph. When admission control is off most of these graphs will be empty or zero, and the total KV admission slots will be 1. Informs cockroachdb#65955 Release note (ui change): admission control metrics are added to Overload dashboard.

sumeerbhola · 2021-08-18T20:24:11Z

TFTRs @RaduBerinde @nathanstilwell !

sumeerbhola · 2021-08-19T12:49:28Z

bors r+

craig · 2021-08-19T14:35:32Z

Build succeeded:

GitHub CI (Cockroach)

sumeerbhola requested review from RaduBerinde and ajwerner August 9, 2021 15:26

sumeerbhola mentioned this pull request Aug 10, 2021

admission control: default cluster settings are set to enabled #65955

Closed

14 tasks

RaduBerinde approved these changes Aug 17, 2021

View reviewed changes

sumeerbhola force-pushed the admission_trace branch from a32b990 to 5e44a7f Compare August 18, 2021 20:23

craig bot merged commit 8d0f75e into cockroachdb:master Aug 19, 2021

jseldess mentioned this pull request Sep 8, 2021

ui,admission: observability improvements for admission control cockroachdb/docs#11566

Closed

sumeerbhola deleted the admission_trace branch September 14, 2021 21:34

exalate-issue-sync bot mentioned this pull request Jun 8, 2022

ui,admission: observability improvements for admission control cockroachdb/docs#14155

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ui,admission: observability improvements for admission control#68595

ui,admission: observability improvements for admission control#68595
craig[bot] merged 1 commit intocockroachdb:masterfrom
sumeerbhola:admission_trace

sumeerbhola commented Aug 9, 2021

Uh oh!

cockroach-teamcity commented Aug 9, 2021

Uh oh!

sumeerbhola commented Aug 9, 2021

Uh oh!

RaduBerinde left a comment

Uh oh!

nathanstilwell commented Aug 18, 2021

Uh oh!

sumeerbhola commented Aug 18, 2021

Uh oh!

sumeerbhola commented Aug 19, 2021

Uh oh!

craig bot commented Aug 19, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

sumeerbhola commented Aug 9, 2021

Uh oh!

cockroach-teamcity commented Aug 9, 2021

Uh oh!

sumeerbhola commented Aug 9, 2021

Uh oh!

RaduBerinde left a comment

Choose a reason for hiding this comment

Uh oh!

nathanstilwell commented Aug 18, 2021

Uh oh!

sumeerbhola commented Aug 18, 2021

Uh oh!

sumeerbhola commented Aug 19, 2021

Uh oh!

craig bot commented Aug 19, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants