Skip to content

ui: add mvcc info to Database page#84037

Merged
craig[bot] merged 1 commit intocockroachdb:masterfrom
maryliag:mvcc-ui
Jul 12, 2022
Merged

ui: add mvcc info to Database page#84037
craig[bot] merged 1 commit intocockroachdb:masterfrom
maryliag:mvcc-ui

Conversation

@maryliag
Copy link
Copy Markdown
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
Copy Markdown
Member

cockroach-teamcity commented Jul 7, 2022

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 Annebirzin and kevin-v-ngo 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
Copy Markdown

@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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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, livlobo and rhu713 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
Copy Markdown
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
Copy Markdown

@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
Copy Markdown

kevin-v-ngo commented Jul 11, 2022

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
Copy Markdown
Contributor Author

maryliag commented Jul 11, 2022

@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
Copy Markdown
Contributor Author

maryliag commented Jul 12, 2022

TFTR!
bors r+

@craig
Copy link
Copy Markdown
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

+1 more reviewer

@matthewtodd matthewtodd matthewtodd approved these changes

Reviewers whose approvals may not affect merge requirements

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