Skip to content

Commit

Permalink
Merge #97945 #98167
Browse files Browse the repository at this point in the history
97945: ui: disable invalid options for tenant advanced debug r=stevendanna,knz a=dhartunian

Prevously, the Advanced Debug page on the DB console would show a lot of options to the tenant that were either unsupported, unimplemented, or not valid in that context. This change hides those options for now to create a smoother Console experience for tenants.

The changes made are currently implemented via a server-controlled feature flag that is enabled by default on tenants, and not on the system tenant.

The summary of disabled items is as follows:
* Problem ranges: hidden since implementation depends on liveness 
  * Issue to fix: #97941
* Data distribution: hidden due to error on tenants
  * Issue to fix: #97942
* Stores section: not applicable to tenants
* Problem Ranges section: see above
* Raft and Key Visualizer links: not applicable to tenants
* Closed timestamps: not applicable to tenants
* Tracing active operations: ui impl proxies to nodes, needs changes
  * Issue to fix: #97943
* Debug requests/events: not applicable to tenants
* Enqueue range: not applicable to tenants
* Node status: not implemented on tenants
* Single node's ranges: not implemented on tenants
* Hot Ranges (legacy): not implemented on tenants
* Single Node Specific: not implemented on tenants
* Cluster wide: not applicable to tenants
* Allocator: not applicable to tenants

Resolves: #80595
Epic: CRDB-12100

Release note: None

98167: sqlstats: flush stmt and txns in parallel r=xinhaoz a=xinhaoz

When we have a lot of in-memory data, the flush
can take some time to complete. WHy not flush
stmt and txns in parallel, so that we at least
get some data to both system tables while flushing instead of wiating for the stmt flush to complete
to start flushing tot txns, which can delay data
being shown in the app.

Epic: none

Release note: None

Co-authored-by: David Hartunian <[email protected]>
Co-authored-by: Xin Hao Zhang <[email protected]>
  • Loading branch information
3 people committed Mar 9, 2023
3 parents 81a2812 + 027f1a5 + 1a4c344 commit ac7961d
Show file tree
Hide file tree
Showing 8 changed files with 153 additions and 28 deletions.
2 changes: 1 addition & 1 deletion pkg/server/api_v2.go
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ func registerRoutes(
{"databases/{database_name:[\\w.]+}/grants/", a.databaseGrants, true, regularRole, noOption, false},
{"databases/{database_name:[\\w.]+}/tables/", a.databaseTables, true, regularRole, noOption, false},
{"databases/{database_name:[\\w.]+}/tables/{table_name:[\\w.]+}/", a.tableDetails, true, regularRole, noOption, false},
{"rules/", a.listRules, false, regularRole, noOption, false},
{"rules/", a.listRules, false, regularRole, noOption, true},

{"sql/", a.execSQL, true, regularRole, noOption, true},
}
Expand Down
5 changes: 3 additions & 2 deletions pkg/server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -1869,8 +1869,9 @@ func (s *Server) PreStart(ctx context.Context) error {
db: s.db,
}), /* apiServer */
serverpb.FeatureFlags{
CanViewKvMetricDashboards: s.rpcContext.TenantID.Equal(roachpb.SystemTenantID),
}, /* flags */
CanViewKvMetricDashboards: s.rpcContext.TenantID.Equal(roachpb.SystemTenantID),
DisableKvLevelAdvancedDebug: false,
},
); err != nil {
return err
}
Expand Down
8 changes: 6 additions & 2 deletions pkg/server/serverpb/admin.proto
Original file line number Diff line number Diff line change
Expand Up @@ -1459,8 +1459,12 @@ message SetTraceRecordingTypeResponse{}

// FeatureFlags within this struct are used within back-end/front-end code to show/hide features.
message FeatureFlags {
// Whether the server is an instance of the Observability Service
// isObservabiliyService is true when the server is an instance of the Observability Service
bool is_observability_service = 1;
// Whether the logged in user is able to view KV-level metric dashboards.
// CanViewKVMetricDashboards is true when the logged in user is able to view KV-level metric dashboards.
bool can_view_kv_metric_dashboards = 2;
// DisableKVLevelAdvancedDebug is true when the UI should remove options to certain KV-level
// debug operations. This is helpful in application tenant contexsts, where these requests
// can only return errors since the tenant cannot perform the operations.
bool disable_kv_level_advanced_debug = 3;
}
3 changes: 2 additions & 1 deletion pkg/server/tenant.go
Original file line number Diff line number Diff line change
Expand Up @@ -692,7 +692,8 @@ func (s *SQLServerWrapper) PreStart(ctx context.Context) error {
db: s.db,
}), /* apiServer */
serverpb.FeatureFlags{
CanViewKvMetricDashboards: s.rpcContext.TenantID.Equal(roachpb.SystemTenantID),
CanViewKvMetricDashboards: s.rpcContext.TenantID.Equal(roachpb.SystemTenantID),
DisableKvLevelAdvancedDebug: true,
},
); err != nil {
return err
Expand Down
17 changes: 15 additions & 2 deletions pkg/sql/sqlstats/persistedsqlstats/flush.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ package persistedsqlstats
import (
"context"
"fmt"
"sync"
"time"

"github.com/cockroachdb/cockroach/pkg/sql/appstatspb"
Expand Down Expand Up @@ -74,8 +75,20 @@ func (s *PersistedSQLStats) Flush(ctx context.Context) {
if s.stmtsLimitSizeReached(ctx) || s.txnsLimitSizeReached(ctx) {
log.Infof(ctx, "unable to flush fingerprints because table limit was reached.")
} else {
s.flushStmtStats(ctx, aggregatedTs)
s.flushTxnStats(ctx, aggregatedTs)
var wg sync.WaitGroup
wg.Add(2)

go func() {
defer wg.Done()
s.flushStmtStats(ctx, aggregatedTs)
}()

go func() {
defer wg.Done()
s.flushTxnStats(ctx, aggregatedTs)
}()

wg.Wait()
}
}

Expand Down
13 changes: 13 additions & 0 deletions pkg/ui/workspaces/db-console/src/redux/state.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ import { initializeAnalytics } from "./analytics";
import { DataFromServer } from "src/util/dataFromServer";
import { cockroach } from "@cockroachlabs/crdb-protobuf-client";
import FeatureFlags = cockroach.server.serverpb.FeatureFlags;
import { createSelector } from "reselect";

export interface AdminUIState {
cachedData: APIReducersState;
Expand All @@ -51,6 +52,7 @@ export interface AdminUIState {
timeScale: TimeScaleState;
uiData: UIDataState;
login: LoginAPIState;
flags: FeatureFlags;
}

const emptyDataFromServer: DataFromServer = {
Expand All @@ -65,6 +67,15 @@ const emptyDataFromServer: DataFromServer = {
Version: "",
};

export const featureFlagSelector = createSelector(
(state: AdminUIState) => state.flags,
flags => flags,
);

export function flagsReducer(state = emptyDataFromServer.FeatureFlags) {
return state;
}

// createAdminUIStore is a function that returns a new store for the admin UI.
// It's in a function so it can be recreated as necessary for testing.
export function createAdminUIStore(
Expand All @@ -86,6 +97,7 @@ export function createAdminUIStore(
timeScale: timeScaleReducer,
uiData: uiDataReducer,
login: loginReducer,
flags: flagsReducer,
}),
{
login: {
Expand All @@ -96,6 +108,7 @@ export function createAdminUIStore(
oidcLoginEnabled: dataFromServer.OIDCLoginEnabled,
oidcButtonText: dataFromServer.OIDCButtonText,
},
flags: dataFromServer.FeatureFlags,
},
compose(
applyMiddleware(thunk, sagaMiddleware, routerMiddleware(historyInst)),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import { configureStore } from "@reduxjs/toolkit";
import type { PreloadedState } from "@reduxjs/toolkit";
import { Provider } from "react-redux";

import { AdminUIState } from "src/redux/state";
import { AdminUIState, flagsReducer } from "src/redux/state";
import { createMemoryHistory } from "history";
import { apiReducersReducer } from "src/redux/apiReducers";
import { hoverReducer } from "src/redux/hover";
Expand Down Expand Up @@ -44,6 +44,7 @@ export function renderWithProviders(
timeScale: timeScaleReducer,
uiData: uiDataReducer,
login: loginReducer,
flags: flagsReducer,
},
preloadedState,
});
Expand Down
Loading

0 comments on commit ac7961d

Please sign in to comment.