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

ui: add mvcc info to Database page #84037

Merged
merged 1 commit into from
Jul 12, 2022
Merged

ui: add mvcc info to Database page #84037

merged 1 commit into from
Jul 12, 2022

Conversation

maryliag
Copy link
Contributor

@maryliag maryliag commented Jul 7, 2022 •
edited
Loading

This commit adds the mvcc info to:

  • Column on the tables list inside Database page
  • Table Details page

Fixes #82617

Screen Shot 2022-07-11 at 5 20 04 PM

Screen Shot 2022-07-11 at 5 20 53 PM

Screen Shot 2022-07-08 at 5 35 49 PM

Release note (ui change): Adds mvcc info to tables list inside
the Database page and on the Tables details page.

@maryliag maryliag requested a review from a team July 7, 2022 20:50
@maryliag maryliag requested review from a team as code owners July 7, 2022 20:50
@maryliag maryliag requested a review from a team July 7, 2022 20:50
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@maryliag maryliag force-pushed the mvcc-ui branch 2 times, most recently from 5573fd9 to f42c356 Compare July 7, 2022 22:14
@maryliag maryliag requested review from kevin-v-ngo and Annebirzin and removed request for a team July 7, 2022 22:15
kevin-v-ngo
kevin-v-ngo reviewed Jul 7, 2022
View reviewed changes
Copy link

@kevin-v-ngo kevin-v-ngo left a comment

Choose a reason for hiding this comment

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

Replication Size and Total doesn't seem to align. Any reason why? I would've guessed that Total was the non-replication size but that doesn't seem to be the case since they're so close. What's the replication factor of this cluster?

@ajwerner
Copy link
Contributor

ajwerner commented Jul 8, 2022

One note here is that it won't work in multi-tenant clusters as a general approach because we pack the tables together into a range. This isn't new and this does mean also that the live bytes calculation is garbage there too. We need some new tools. This fact imperils #81008.

@maryliag
Copy link
Contributor Author

maryliag commented Jul 8, 2022

To clarify, the changes from this PR (and the one with the backend changes) will not affect the Database page on Cloud Dedicated and Serverless, since that is a completely different page. The changes here affect DB Console only.
Cloud Dedicated is able to see this page if they open DB Console inside their CC Console, but Serverless don't have access to this.

@knz
Copy link
Contributor

knz commented Jul 8, 2022 •
edited
Loading

The changes here affect DB Console only.

ok, but...

it won't work in multi-tenant clusters as a general approach because we pack the tables together into a range

This fact remains true and endangers our roadmap for INI-214.

Andrew and Marylia, could you collaborate to file a follow-up issue that describes the work needed to make this work on the DB Console of a multi-tenant dedicated cluster?

Then you can set "Epic" field to CRDB-16704 in the issue description to link them together.

cc @ajstorm for tracking.

@ajwerner
Copy link
Contributor

ajwerner commented Jul 8, 2022

I filed #84105 to build a mechanism which we can use for sub-range storage calculations and epic linked it. It may not contain enough details about hooking up that new mechanism to all the various use cases.

@knz
Copy link
Contributor

knz commented Jul 8, 2022

thank you!

@maryliag maryliag requested review from a team as code owners July 8, 2022 21:30
@maryliag maryliag requested a review from a team July 8, 2022 21:30
@maryliag maryliag requested review from a team as code owners July 8, 2022 21:30
@maryliag maryliag requested a review from a team July 8, 2022 21:30
@maryliag maryliag requested review from a team as code owners July 8, 2022 21:30
@maryliag maryliag requested a review from a team July 8, 2022 21:30
@maryliag maryliag requested a review from a team as a code owner July 8, 2022 21:30
@maryliag maryliag requested review from livlobo and removed request for a team, erikgrinaker, rhu713 and livlobo July 8, 2022 21:30
@maryliag maryliag force-pushed the mvcc-ui branch 2 times, most recently from 66131e6 to 73a6bf2 Compare July 8, 2022 21:41
@maryliag
Copy link
Contributor Author

maryliag commented Jul 8, 2022

Updates made (commit and PR message/images) to show info about live instead of non-live

matthewtodd
matthewtodd approved these changes Jul 11, 2022
View reviewed changes
Copy link
Contributor

@matthewtodd matthewtodd left a comment

Choose a reason for hiding this comment

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

Frontend code LGTM

@kevin-v-ngo
Copy link

Updates made (commit and PR message/images) to show info about live instead of non-live

Hi Marylia, why is the % so low in the screenshot?

image

@maryliag
Copy link
Contributor Author

@kevin-v-ngo nice catch! Turns out I forgot to update the percentage from non-live to live. Fixed now! I'll update the images later on

@maryliag
ui: add mvcc info to Database page
534f18f 
This commit adds the mvcc info to:
- Column on the tables list inside Database page
- Table Details page

Fixes cockroachdb#82617

Release note (ui change): Adds mvcc info to tables list inside
the Database page and on the Tables details page.
@maryliag
Copy link
Contributor Author

TFTR!
bors r+

@craig
Copy link
Contributor

craig bot commented Jul 12, 2022

Build succeeded:

@craig craig bot merged commit 999ffa6 into cockroachdb:master Jul 12, 2022
@cockroach-teamcity cockroach-teamcity mentioned this pull request Jul 12, 2022
Closed
@maryliag maryliag deleted the mvcc-ui branch July 12, 2022 19:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kevin-v-ngo kevin-v-ngo kevin-v-ngo left review comments

@Annebirzin Annebirzin Annebirzin approved these changes

@matthewtodd matthewtodd matthewtodd approved these changes

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

Successfully merging this pull request may close these issues.

Add information on MVCC garbage data on the databases page
7 participants
@maryliag @cockroach-teamcity @ajwerner @knz @kevin-v-ngo @matthewtodd @Annebirzin