From f11a64ee5e9d14972b23ed607d0364fe9ec4e7dd Mon Sep 17 00:00:00 2001 From: David Hartunian Date: Wed, 14 Oct 2020 14:49:40 -0400 Subject: [PATCH] ui: databases page requires click to load table info Currently, the databases page automatically loads stats and info on all tables in all database simultaneously. This can cause heavy load and contention on clusters with hundreds of tables and/or databases. This change introduces a quick fix to prevent the immediate problem with the expectation that we will revisit the databases page for a more thorough redesign in the near future. This change adds a button next to each database in the databases page titled "Load stats for all tables" which triggers the queries to load table-specific stats for just that one database. The tables will be loaded in sequence by converting the loading loop to use async/await to ensure one query is executed at a time. One problem this introduces is that the database stats summary box on the right computes its data incrementally and will show changing data as more tables load for a specific database. We have chosen to not show the updated data until all table info has been loaded for the given database. Release note (admin ui change): Loading table-level stats requires a button click per-database in order to prevent contention for clusters with many databases and/or tables. In addition, the loading of table data is staggered by table instead of triggered simultaneously for all tables. --- .../containers/databaseSummary/index.tsx | 43 ++-- .../databaseTables/databaseTables.styl | 24 +++ .../containers/databaseTables/index.tsx | 201 +++++++++--------- .../databases/containers/databases/index.tsx | 4 +- pkg/ui/src/views/databases/data/tableInfo.tsx | 4 + 5 files changed, 151 insertions(+), 125 deletions(-) diff --git a/pkg/ui/src/views/databases/containers/databaseSummary/index.tsx b/pkg/ui/src/views/databases/containers/databaseSummary/index.tsx index 7785ada4a64d..8b89abca2602 100644 --- a/pkg/ui/src/views/databases/containers/databaseSummary/index.tsx +++ b/pkg/ui/src/views/databases/containers/databaseSummary/index.tsx @@ -24,6 +24,7 @@ import { TableInfo } from "src/views/databases/data/tableInfo"; // on a DatabaseSummary component. export interface DatabaseSummaryExplicitData { name: string; + updateOnLoad?: boolean; } // DatabaseSummaryConnectedData describes properties which are applied to a @@ -44,45 +45,53 @@ interface DatabaseSummaryActions { refreshTableStats: typeof refreshTableStats; } -type DatabaseSummaryProps = DatabaseSummaryExplicitData & DatabaseSummaryConnectedData & DatabaseSummaryActions; +export type DatabaseSummaryProps = DatabaseSummaryExplicitData & DatabaseSummaryConnectedData & DatabaseSummaryActions; + +interface DatabaseSummaryState { + finishedLoadingTableData: boolean; +} // DatabaseSummaryBase implements common lifecycle methods for DatabaseSummary // components, which differ primarily by their render() method. // TODO(mrtracy): We need to find a better abstraction for the common // "refresh-on-mount-or-receiveProps" we have in many of our connected // components; that would allow us to avoid this inheritance. -export class DatabaseSummaryBase extends React.Component { +export class DatabaseSummaryBase extends React.Component { // loadTableDetails loads data for each table which have no info in the store. // TODO(mrtracy): Should this be refreshing data always? Not sure if there // is a performance concern with invalidation periods. - loadTableDetails(props = this.props) { + async loadTableDetails(props = this.props) { if (props.tableInfos && props.tableInfos.length > 0) { - _.each(props.tableInfos, (tblInfo) => { - if (_.isUndefined(tblInfo.numColumns)) { - props.refreshTableDetails(new protos.cockroach.server.serverpb.TableDetailsRequest({ + for (const tblInfo of props.tableInfos) { + // TODO(davidh): this is a stopgap inserted to deal with DBs containing hundreds of tables + await Promise.all([ + _.isUndefined(tblInfo.numColumns) ? props.refreshTableDetails(new protos.cockroach.server.serverpb.TableDetailsRequest({ database: props.name, table: tblInfo.name, - })); - } - if (_.isUndefined(tblInfo.physicalSize)) { - props.refreshTableStats(new protos.cockroach.server.serverpb.TableStatsRequest({ + })) : null, + _.isUndefined(tblInfo.physicalSize) ? props.refreshTableStats(new protos.cockroach.server.serverpb.TableStatsRequest({ database: props.name, table: tblInfo.name, - })); - } - }); + })) : null, + ]); + } } + this.setState({finishedLoadingTableData: true}); } // Refresh when the component is mounted. - componentDidMount() { + async componentDidMount() { this.props.refreshDatabaseDetails(new protos.cockroach.server.serverpb.DatabaseDetailsRequest({ database: this.props.name })); - this.loadTableDetails(); + if (this.props.updateOnLoad) { + await this.loadTableDetails(); + } } // Refresh when the component receives properties. - componentDidUpdate() { - this.loadTableDetails(this.props); + async componentDidUpdate() { + if (this.props.updateOnLoad) { + await this.loadTableDetails(this.props); + } } render(): React.ReactElement { diff --git a/pkg/ui/src/views/databases/containers/databaseTables/databaseTables.styl b/pkg/ui/src/views/databases/containers/databaseTables/databaseTables.styl index e535e51f8ff0..059ab70c8126 100644 --- a/pkg/ui/src/views/databases/containers/databaseTables/databaseTables.styl +++ b/pkg/ui/src/views/databases/containers/databaseTables/databaseTables.styl @@ -8,6 +8,8 @@ // by the Apache License, Version 2.0, included in the file // licenses/APL.txt. +@require "~styl/base/palette.styl" + .empty-state display flex justify-content center @@ -22,3 +24,25 @@ position relative top 3px right 2px + +.sort-table__cell a + color: $text-color; + &:hover + color: $link-color; + +.table-name a + display flex + align-items center + width 100% + height 100% + svg + margin-right 11px + &:hover + path + fill $colors--primary-blue-3 + +.database-summary-title + display flex + +.database-summary-load-button + margin-left 11px diff --git a/pkg/ui/src/views/databases/containers/databaseTables/index.tsx b/pkg/ui/src/views/databases/containers/databaseTables/index.tsx index 036229dab3c4..dd90a7147767 100644 --- a/pkg/ui/src/views/databases/containers/databaseTables/index.tsx +++ b/pkg/ui/src/views/databases/containers/databaseTables/index.tsx @@ -8,9 +8,7 @@ // by the Apache License, Version 2.0, included in the file // licenses/APL.txt. -import tableIcon from "!!raw-loader!assets/tableIcon.svg"; import _ from "lodash"; -import { SummaryCard } from "oss/src/views/shared/components/summaryCard"; import React from "react"; import { connect } from "react-redux"; import { Link } from "react-router-dom"; @@ -18,13 +16,21 @@ import { refreshDatabaseDetails, refreshTableDetails, refreshTableStats } from " import { LocalSetting } from "src/redux/localsettings"; import { AdminUIState } from "src/redux/state"; import { Bytes } from "src/util/format"; -import { trustIcon } from "src/util/trust"; -import { databaseDetails, DatabaseSummaryBase, DatabaseSummaryExplicitData, grants, tableInfos as selectTableInfos } from "src/views/databases/containers/databaseSummary"; +import { + databaseDetails, + DatabaseSummaryBase, + DatabaseSummaryExplicitData, + DatabaseSummaryProps, + grants, + tableInfos as selectTableInfos, +} from "src/views/databases/containers/databaseSummary"; import { TableInfo } from "src/views/databases/data/tableInfo"; import { SortSetting } from "src/views/shared/components/sortabletable"; import { SortedTable } from "src/views/shared/components/sortedtable"; -import { SummaryBar, SummaryHeadlineStat } from "src/views/shared/components/summaryBar"; import "./databaseTables.styl"; +import { SummaryCard } from "src/views/shared/components/summaryCard"; +import { SummaryHeadlineStat } from "src/views/shared/components/summaryBar"; +import { Button } from "src/components"; const databaseTablesSortSetting = new LocalSetting( "databases/sort_setting/tables", (s) => s.localSettings, @@ -32,119 +38,103 @@ const databaseTablesSortSetting = new LocalSetting( class DatabaseTableListSortedTable extends SortedTable {} -class DatabaseTableListEmpty extends React.Component { - render() { - return ( - - - - - - - - - - - - - - - -
- Table Name - - Size - - Ranges - - # of Columns - - # of Indices -
-
-
- - This database has no tables. -
-
-
- ); - } -} - // DatabaseSummaryTables displays a summary section describing the tables // contained in a single database. -class DatabaseSummaryTables extends DatabaseSummaryBase { +export class DatabaseSummaryTables extends DatabaseSummaryBase { + constructor(props: DatabaseSummaryProps) { + super(props); + + this.state = { + finishedLoadingTableData: props.tableInfos && props.tableInfos.every(ti => ti.detailsAndStatsLoaded()), + }; + } + totalSize() { const tableInfos = this.props.tableInfos; - return _.sumBy(tableInfos, (ti) => ti.physicalSize); + if (this.state.finishedLoadingTableData) { + return _.sumBy(tableInfos, (ti) => ti.physicalSize); + } else { + return null; + } } totalRangeCount() { const tableInfos = this.props.tableInfos; - return _.sumBy(tableInfos, (ti) => ti.rangeCount); + if (this.state.finishedLoadingTableData) { + return _.sumBy(tableInfos, (ti) => ti.rangeCount); + } else { + return null; + } } + noDatabaseResults = () => ( + <> +

This database has no tables.

+ + ) + render() { - const { tableInfos, sortSetting } = this.props; + const { tableInfos, dbResponse, sortSetting } = this.props; const dbID = this.props.name; - + const loading = dbResponse ? !!dbResponse.inFlight : true; const numTables = tableInfos && tableInfos.length || 0; - return (
- -
-

{dbID}

-
-
-
-
+
+

{dbID}

+ {this.state.finishedLoadingTableData || numTables === 0 ? null : + + } +
+
+
+ this.props.setSort(setting)} + firstCellBordered + columns={[ + { + title: "Table Name", + cell: (tableInfo) => { + return ( +
+ {tableInfo.name} +
+ ); + }, + sort: (tableInfo) => tableInfo.name, + className: "expand-link", // don't pad the td element to allow the link to expand + }, { - (numTables === 0) ? : - this.props.setSort(setting)} - columns={[ - { - title: "Table Name", - cell: (tableInfo) => { - return ( -
- {tableInfo.name} -
- ); - }, - sort: (tableInfo) => tableInfo.name, - className: "expand-link", // don't pad the td element to allow the link to expand - }, - { - title: "Size", - cell: (tableInfo) => Bytes(tableInfo.physicalSize), - sort: (tableInfo) => tableInfo.physicalSize, - }, - { - title: "Ranges", - cell: (tableInfo) => tableInfo.rangeCount, - sort: (tableInfo) => tableInfo.rangeCount, - }, - { - title: "# of Columns", - cell: (tableInfo) => tableInfo.numColumns, - sort: (tableInfo) => tableInfo.numColumns, - }, - { - title: "# of Indices", - cell: (tableInfo) => tableInfo.numIndices, - sort: (tableInfo) => tableInfo.numIndices, - }, - ]} /> - } -
-
-
- + title: "Size", + cell: (tableInfo) => _.isUndefined(tableInfo.physicalSize) ? "" : Bytes(tableInfo.physicalSize), + sort: (tableInfo) => tableInfo.physicalSize, + }, + { + title: "Ranges", + cell: (tableInfo) => tableInfo.rangeCount, + sort: (tableInfo) => tableInfo.rangeCount, + }, + { + title: "# of Columns", + cell: (tableInfo) => tableInfo.numColumns, + sort: (tableInfo) => tableInfo.numColumns, + }, + { + title: "# of Indices", + cell: (tableInfo) => tableInfo.numIndices, + sort: (tableInfo) => tableInfo.numIndices, + }, + ]} + loading={loading} + renderNoResult={loading ? undefined : this.noDatabaseResults()} + /> +
+
+ - -
+
- +
); } @@ -170,7 +159,7 @@ class DatabaseSummaryTables extends DatabaseSummaryBase { const mapStateToProps = (state: AdminUIState, ownProps: DatabaseSummaryExplicitData) => ({ // RootState contains declaration for whole state tableInfos: selectTableInfos(state, ownProps.name), sortSetting: databaseTablesSortSetting.selector(state), - dbResponse: databaseDetails(state)[ownProps.name] && databaseDetails(state)[ownProps.name].data, + dbResponse: databaseDetails(state)[ownProps.name], grants: grants(state, ownProps.name), }); diff --git a/pkg/ui/src/views/databases/containers/databases/index.tsx b/pkg/ui/src/views/databases/containers/databases/index.tsx index 68063243feb4..709c419d50c8 100644 --- a/pkg/ui/src/views/databases/containers/databases/index.tsx +++ b/pkg/ui/src/views/databases/containers/databases/index.tsx @@ -101,11 +101,11 @@ class DatabaseTablesList extends React.Component {
{ - user.map(n => ) + user.map(n => ) }
{ - system.map(n => ) + system.map(n => ) }
diff --git a/pkg/ui/src/views/databases/data/tableInfo.tsx b/pkg/ui/src/views/databases/data/tableInfo.tsx index d32d237911c9..16cf25425102 100644 --- a/pkg/ui/src/views/databases/data/tableInfo.tsx +++ b/pkg/ui/src/views/databases/data/tableInfo.tsx @@ -43,4 +43,8 @@ export class TableInfo { this.physicalSize = FixLong(stats.approximate_disk_bytes).toNumber(); } } + + public detailsAndStatsLoaded(): boolean { + return this.id !== undefined && this.physicalSize !== undefined; + } }