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-21.2: ui: Add Hot Ranges page #77594

Merged
merged 8 commits into from
Mar 21, 2022
Merged

release-21.2: ui: Add Hot Ranges page #77594

merged 8 commits into from
Mar 21, 2022

Conversation

koorosh
Copy link
Collaborator

@koorosh koorosh commented Mar 10, 2022 •
edited
Loading

Backport:

Please see individual PRs for details.

/cc @cockroachdb/release

Release justification: bug fixes and low-risk updates to new functionality

@koorosh koorosh requested review from a team as code owners March 10, 2022 12:32
@blathers-crl
Copy link

blathers-crl bot commented Mar 10, 2022 •
edited by koorosh
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?

@cockroach-teamcity
Copy link
Member

This change is Reviewable

@koorosh koorosh force-pushed the backport21.2-74377-76877-77330-77487-77533 branch 9 times, most recently from 1231b8d to ba2889b Compare March 16, 2022 12:55
@koorosh
server: hot ranges api
f8def6a 
This change implements second version of hot ranges api that's required
for UI to represent enhanced Hot Ranges page.

Before, Hot ranges (under Advanced debugging page) used former version of
HotRanges api that provided information about hot ranges with internal/
sensitive data (see: cockroachdb#53212)
that should not be exposed to users.

Now, in addition to existing endpoint, additional one is implemented that is
based on a former version and provides hot ranges information that is only
needed for Hot Ranges page (range id, qps, table, db, and index names for
particular range).

The list of hot ranges and their QPS is provided by `HotRanges` service and
then information like DB, table and index names are retrieved from range's
`StartKey` that might include this info or not (in case if it's meta range,
or range that stores index itself for instance).

`HotRange` and `HotRangeV2` services expect the same request type as an
argument but return different responses.
`HotRangeV2` service returns a flat list of hot ranges instead of grouped
ranges per node/store.

Release note: None

server: add leaseholder node id to hot ranges api

Current change extends `statuspb.HotRangesResponse` to
include `LeaseholderNodeID` field to indicate the
node id that contains leaseholder replica for
current hot range.

This change was made in `localHotRanges` function (that is
used by `HotRanges` that in turn used by `HotRangeV2` service)
to reuse existing logic of iteration over the stores and querying
hot ranges. It extends its response by `LeaseholderNodeID` field.
Otherwise, the same logic should be implemented in `HotRangeV2`
service by calling `VisitStores` iterator.

Release note: None

Release justification: bug fixes and low-risk updates
to new functionality
@koorosh
server: update hot ranges api/v2 version
7b75e0d 
Before, `listHotRanges` request handler of `apiV2Server` relied
on `HotRanges` service that is now should be replaced by new
`HotRangesV2` implementation.

Current change reuses HotRangeV2 service in `api/v2/ranges/hot`
api. It allows to share the same logic between REST and gRPC
endpoints and gradually migrate to new version of API.

Release note: None

Release justification: bug fixes and low-risk updates
to new functionality
@koorosh
server, ui: use POST request method for HotRangesV2 service
4fabb50 
Initially, `HotRangesV2` service in status server was defined
to use GET method to handle HTTP request. It was convenient way
to display response data in Advanced debugging page since it allowed
to render response body right on to page.

But now, hot ranges will be used on user facing page and request
dispatching for hot ranges api should follow generic workflow:
initialize `HotRangesRequest` protobuf message and dispatch request
with `src/util/api` service. This restriction forces to use POST
method (since GET method doesn't allow to provide request body).

In addition, Hot Ranges debugging page is refactored to use `api`
service.

Release note: None

Release justification: bug fixes and low-risk updates
to new functionality
@koorosh
server: add pagination to hot ranges API
6e279d8 
This change extends hot ranges API to support pagination
of responses. It is also possible to avoid pagination when
page size is set to 0.

Release note: None

Release justification: bug fixes and low-risk updates
to new functionality
@koorosh
server: add store_id to hot ranges response api
8fcfd43 
This change extends HotRangesResponseV2 message
to include store ID data. This information can
be useful to investigate cause of hot ranges.

Release note: None

Release justification: bug fixes and low-risk updates to new functionality
@koorosh
ui: add hot ranges page in Db Console
8c8e463 
This change adds a new page in Db Console - Hot Ranges that display list
of hot ranges in the cluster and some useful information related to each
range. The main goal of this page is to help eliminate root cause of
hot ranges.

This page uses already implemented hot ranges API (V2) that provides
all necessary data to show.

Release note (ui change): add Hot Ranges page and link to it on the sidebar

Release justification: bug fixes and low-risk updates to new functionality
@koorosh
ui: add locality field to Hot ranges page
a3aaa81 
Before, Hot Ranges page provided a list of hot ranges
info without localities of nodes where range is stored.
This information is very useful during investigation of
hot ranges.

Current change, adds one more column to hot ranges table
with localities represented in human-readable way.

Release note: None

Release justification: bug fixes and low-risk updates to new functionality
@koorosh
ui: stying fixes for Hot Ranges page
d9b4f0e 
Current change contains list of small visual improvements
for Hot Ranges page:
- Capitalize "Hot Ranges" in navigation panel and page header;
- Update page description and put it inside inline info panel;
- Proper margins between header and text description;

Release note: None

Release justification: bug fixes and low-risk updates to new functionality
@koorosh koorosh force-pushed the backport21.2-74377-76877-77330-77487-77533 branch from ba2889b to d9b4f0e Compare March 17, 2022 09:19
Santamaura
Santamaura approved these changes Mar 21, 2022
View reviewed changes
Copy link
Contributor

@Santamaura Santamaura left a comment

Choose a reason for hiding this comment

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

Reviewed 30 of 30 files at r1, all commit messages.
Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @thtruo and @zachlite)

@koorosh koorosh merged commit 2401516 into cockroachdb:release-21.2 Mar 21, 2022
@cockroach-teamcity cockroach-teamcity mentioned this pull request Mar 21, 2022
Closed
@exalate-issue-sync exalate-issue-sync bot mentioned this pull request Jun 8, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Santamaura Santamaura Santamaura approved these changes

@zachlite zachlite Awaiting requested review from zachlite

@thtruo thtruo Awaiting requested review from thtruo

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
@koorosh @cockroach-teamcity @Santamaura