From 3505f62d98d8830cc7dcef522b49336c4f9db290 Mon Sep 17 00:00:00 2001 From: Alex Lunev Date: Fri, 27 Aug 2021 14:29:07 -0700 Subject: [PATCH 1/3] liveness: make CreateLivenessRecord retry on a TransactionError Fixes #50977 CreateLivenessRecord has no fault tolerance built in, which makes the server startup facility flaky. The startup procesure allocates a nodeId, storeId and then creates a liveness record. If there is a hiccup or brief period of unavailability during the liveness creation we end up cycing through node/store ids which is undesirable. This made the TestRemoveDeadReplicas test flaky because nodes ended up cycling through a few node ids on startup, making it no possible to match epected node ids with the actual ones. To fix this issue we retry liveness creation when it encounters an AmbigiousError or TransactionError. This is similar behavior to the update code path for the liveness record. Release note: None --- pkg/cli/debug_test.go | 1 - pkg/kv/kvserver/liveness/liveness.go | 101 +++++++++++++++----------- pkg/kv/kvserver/node_liveness_test.go | 70 ++++++++++++++++++ 3 files changed, 127 insertions(+), 45 deletions(-) diff --git a/pkg/cli/debug_test.go b/pkg/cli/debug_test.go index 8ddb779bb9d3..f6f3f255f0c0 100644 --- a/pkg/cli/debug_test.go +++ b/pkg/cli/debug_test.go @@ -132,7 +132,6 @@ func TestOpenReadOnlyStore(t *testing.T) { func TestRemoveDeadReplicas(t *testing.T) { defer leaktest.AfterTest(t)() - skip.WithIssue(t, 50977, "flaky test") defer log.Scope(t).Close(t) // This test is pretty slow under race (200+ cpu-seconds) because it diff --git a/pkg/kv/kvserver/liveness/liveness.go b/pkg/kv/kvserver/liveness/liveness.go index ddf4a3dfdf40..0d4f9bd0f011 100644 --- a/pkg/kv/kvserver/liveness/liveness.go +++ b/pkg/kv/kvserver/liveness/liveness.go @@ -76,6 +76,22 @@ func (e *errRetryLiveness) Error() string { return fmt.Sprintf("%T: %s", *e, e.error) } +func isErrRetryLiveness(ctx context.Context, err error) bool { + if errors.HasType(err, (*roachpb.AmbiguousResultError)(nil)) { + // We generally want to retry ambiguous errors immediately, except if the + // ctx is canceled - in which case the ambiguous error is probably caused + // by the cancellation (and in any case it's pointless to retry with a + // canceled ctx). + return ctx.Err() == nil + } else if errors.HasType(err, (*roachpb.TransactionStatusError)(nil)) { + // 21.2 nodes can return a TransactionStatusError when they should have + // returned an AmbiguousResultError. + // TODO(andrei): Remove this in 22.2. + return true + } + return false +} + // Node liveness metrics counter names. var ( metaLiveNodes = metric.Metadata{ @@ -527,41 +543,50 @@ type livenessUpdate struct { // NB: An existing liveness record is not overwritten by this method, we return // an error instead. func (nl *NodeLiveness) CreateLivenessRecord(ctx context.Context, nodeID roachpb.NodeID) error { - // We start off at epoch=0, entrusting the initial heartbeat to increment it - // to epoch=1 to signal the very first time the node is up and running. - liveness := livenesspb.Liveness{NodeID: nodeID, Epoch: 0} - - // We skip adding an expiration, we only really care about the liveness - // record existing within KV. + for r := retry.StartWithCtx(ctx, base.DefaultRetryOptions()); r.Next(); { + // We start off at epoch=0, entrusting the initial heartbeat to increment it + // to epoch=1 to signal the very first time the node is up and running. + liveness := livenesspb.Liveness{NodeID: nodeID, Epoch: 0} + + // We skip adding an expiration, we only really care about the liveness + // record existing within KV. + + v := new(roachpb.Value) + err := nl.db.Txn(ctx, func(ctx context.Context, txn *kv.Txn) error { + b := txn.NewBatch() + key := keys.NodeLivenessKey(nodeID) + if err := v.SetProto(&liveness); err != nil { + log.Fatalf(ctx, "failed to marshall proto: %s", err) + } + // Given we're looking to create a new liveness record here, we don't + // expect to find anything. + b.CPut(key, v, nil) + + // We don't bother adding a gossip trigger, that'll happen with the + // first heartbeat. We still keep it as a 1PC commit to avoid leaving + // write intents. + b.AddRawRequest(&roachpb.EndTxnRequest{ + Commit: true, + Require1PC: true, + }) + return txn.Run(ctx, b) + }) - v := new(roachpb.Value) - if err := nl.db.Txn(ctx, func(ctx context.Context, txn *kv.Txn) error { - b := txn.NewBatch() - key := keys.NodeLivenessKey(nodeID) - if err := v.SetProto(&liveness); err != nil { - log.Fatalf(ctx, "failed to marshall proto: %s", err) + if err == nil { + // We'll learn about this liveness record through gossip eventually, so we + // don't bother updating our in-memory view of node liveness. + log.Infof(ctx, "created liveness record for n%d", nodeID) + return nil } - // Given we're looking to create a new liveness record here, we don't - // expect to find anything. - b.CPut(key, v, nil) - - // We don't bother adding a gossip trigger, that'll happen with the - // first heartbeat. We still keep it as a 1PC commit to avoid leaving - // write intents. - b.AddRawRequest(&roachpb.EndTxnRequest{ - Commit: true, - Require1PC: true, - }) - return txn.Run(ctx, b) - }); err != nil { + if !isErrRetryLiveness(ctx, err) { + return err + } + log.VEventf(ctx, 2, "failed to create liveness record for node %d, because of %s. retrying...", nodeID, err) + } + if err := ctx.Err(); err != nil { return err } - - // We'll learn about this liveness record through gossip eventually, so we - // don't bother updating our in-memory view of node liveness. - - log.Infof(ctx, "created liveness record for n%d", nodeID) - return nil + return errors.AssertionFailedf("unexpected problem while creating liveness record for node %d", nodeID) } func (nl *NodeLiveness) setMembershipStatusInternal( @@ -1318,19 +1343,7 @@ func (nl *NodeLiveness) updateLivenessAttempt( return Record{}, errors.Wrapf(err, "couldn't update node liveness from CPut actual value") } return Record{}, handleCondFailed(Record{Liveness: actualLiveness, raw: tErr.ActualValue.TagAndDataBytes()}) - } else if errors.HasType(err, (*roachpb.AmbiguousResultError)(nil)) { - // We generally want to retry ambiguous errors immediately, except if the - // ctx is canceled - in which case the ambiguous error is probably caused - // by the cancellation (and in any case it's pointless to retry with a - // canceled ctx). - if ctx.Err() != nil { - return Record{}, err - } - return Record{}, &errRetryLiveness{err} - } else if errors.HasType(err, (*roachpb.TransactionStatusError)(nil)) { - // 21.2 nodes can return a TransactionStatusError when they should have - // returned an AmbiguousResultError. - // TODO(andrei): Remove this in 22.2. + } else if isErrRetryLiveness(ctx, err) { return Record{}, &errRetryLiveness{err} } return Record{}, err diff --git a/pkg/kv/kvserver/node_liveness_test.go b/pkg/kv/kvserver/node_liveness_test.go index ba149a471fc6..ea674c464815 100644 --- a/pkg/kv/kvserver/node_liveness_test.go +++ b/pkg/kv/kvserver/node_liveness_test.go @@ -1053,6 +1053,76 @@ func TestNodeLivenessRetryAmbiguousResultError(t *testing.T) { } } +// This tests the create code path for node liveness, for that we need to create +// a cluster of nodes, because we need to exercise to join RPC codepath. +func TestNodeLivenessRetryAmbiguousResultOnCreateError(t *testing.T) { + defer leaktest.AfterTest(t)() + defer log.Scope(t).Close(t) + + var injectError atomic.Value + type injectErrorData struct { + shouldError bool + count int32 + } + + injectError.Store(map[roachpb.NodeID]injectErrorData{ + roachpb.NodeID(1): {true, 0}, + roachpb.NodeID(2): {true, 0}, + roachpb.NodeID(3): {true, 0}, + }) + testingEvalFilter := func(args kvserverbase.FilterArgs) *roachpb.Error { + if req, ok := args.Req.(*roachpb.ConditionalPutRequest); ok { + if val := injectError.Load(); val != nil { + var liveness livenesspb.Liveness + if err := req.Value.GetProto(&liveness); err != nil { + return nil + } + injectErrorMap := val.(map[roachpb.NodeID]injectErrorData) + if injectErrorMap[liveness.NodeID].shouldError { + if liveness.NodeID != 1 { + // We expect this to come from the create code path on all nodes + // except the first. Make sure that is actually true. + assert.Equal(t, liveness.Epoch, int64(0)) + } + injectErrorMap[liveness.NodeID] = injectErrorData{false, injectErrorMap[liveness.NodeID].count + 1} + injectError.Store(injectErrorMap) + return roachpb.NewError(roachpb.NewAmbiguousResultError("test")) + } + } + } + return nil + } + ctx := context.Background() + tc := testcluster.StartTestCluster(t, 3, + base.TestClusterArgs{ + ReplicationMode: base.ReplicationManual, + ServerArgs: base.TestServerArgs{ + Knobs: base.TestingKnobs{ + Store: &kvserver.StoreTestingKnobs{ + EvalKnobs: kvserverbase.BatchEvalTestingKnobs{ + TestingEvalFilter: testingEvalFilter, + }, + }, + }, + }, + }) + defer tc.Stopper().Stop(ctx) + + for _, s := range tc.Servers { + // Verify retry of the ambiguous result for heartbeat loop. + testutils.SucceedsSoon(t, func() error { + return verifyLivenessServer(s, 3) + }) + nl := s.NodeLiveness().(*liveness.NodeLiveness) + _, ok := nl.Self() + assert.True(t, ok) + + injectErrorMap := injectError.Load().(map[roachpb.NodeID]injectErrorData) + assert.NotNil(t, injectErrorMap) + assert.Equal(t, int32(1), injectErrorMap[s.NodeID()].count) + } +} + // Test that, although a liveness heartbeat is generally retried on // AmbiguousResultError (see test above), it is not retried when the error is // caused by a canceled context. From 4e2384e1abfeaa04c55d422a68f9a333fe3534bc Mon Sep 17 00:00:00 2001 From: Oliver Tan Date: Wed, 20 Oct 2021 07:03:34 +1100 Subject: [PATCH 2/3] roachtest: fix schemachange/during/kv Resolves #71608 Some more fallout from the PRIMARY KEY rename. Release note: None --- pkg/cmd/roachtest/tests/schemachange.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cmd/roachtest/tests/schemachange.go b/pkg/cmd/roachtest/tests/schemachange.go index 334d069cf530..ccdf057eed61 100644 --- a/pkg/cmd/roachtest/tests/schemachange.go +++ b/pkg/cmd/roachtest/tests/schemachange.go @@ -133,7 +133,7 @@ func waitForSchemaChanges(ctx context.Context, l *logger.Logger, db *gosql.DB) e } // Queries to hone in on index validation problems. indexValidationQueries := []string{ - "SELECT count(k) FROM test.kv@primary AS OF SYSTEM TIME %s WHERE created_at > $1 AND created_at <= $2", + "SELECT count(k) FROM test.kv@kv_pkey AS OF SYSTEM TIME %s WHERE created_at > $1 AND created_at <= $2", "SELECT count(v) FROM test.kv@foo AS OF SYSTEM TIME %s WHERE created_at > $1 AND created_at <= $2", } return runValidationQueries(ctx, l, db, start, validationQueries, indexValidationQueries) From e5734152172cee5b7383c1aa16ccb3df258ebebd Mon Sep 17 00:00:00 2001 From: Marylia Gutierrez Date: Wed, 20 Oct 2021 11:46:23 -0400 Subject: [PATCH 3/3] ui: update url params when switching tabs on sql activity When the user clicks on a tab on the new SQL Activity page now the search params on the url is updated with the tab selection. This commit also create a common function to sync history so all the pages can use it and remove duplicate code. Release note: None --- .../src/sessions/sessionDetails.tsx | 7 +- .../cluster-ui/src/sessions/sessionsPage.tsx | 32 ++----- .../src/statementDetails/statementDetails.tsx | 2 +- .../src/statementsPage/statementsPage.tsx | 88 +++++++++---------- .../src/transactionsPage/transactionsPage.tsx | 84 +++++++++--------- .../cluster-ui/src/util/query/query.ts | 25 +++++- pkg/ui/workspaces/db-console/src/app.spec.tsx | 28 ++++-- pkg/ui/workspaces/db-console/src/app.tsx | 13 +-- .../src/views/sqlActivity/sqlActivityPage.tsx | 7 +- 9 files changed, 148 insertions(+), 138 deletions(-) diff --git a/pkg/ui/workspaces/cluster-ui/src/sessions/sessionDetails.tsx b/pkg/ui/workspaces/cluster-ui/src/sessions/sessionDetails.tsx index bf27a336d530..40450d93799d 100644 --- a/pkg/ui/workspaces/cluster-ui/src/sessions/sessionDetails.tsx +++ b/pkg/ui/workspaces/cluster-ui/src/sessions/sessionDetails.tsx @@ -124,12 +124,9 @@ export class SessionDetails extends React.Component { } backToSessionsPage = (): void => { - const { history, location, onBackButtonClick } = this.props; + const { history, onBackButtonClick } = this.props; onBackButtonClick && onBackButtonClick(); - history.push({ - ...location, - pathname: "/sql-activity/sessions", - }); + history.push("/sql-activity?tab=sessions"); }; render(): React.ReactElement { diff --git a/pkg/ui/workspaces/cluster-ui/src/sessions/sessionsPage.tsx b/pkg/ui/workspaces/cluster-ui/src/sessions/sessionsPage.tsx index 8a66c03107b7..dc317b232fb2 100644 --- a/pkg/ui/workspaces/cluster-ui/src/sessions/sessionsPage.tsx +++ b/pkg/ui/workspaces/cluster-ui/src/sessions/sessionsPage.tsx @@ -9,9 +9,9 @@ // licenses/APL.txt. import React from "react"; -import { forIn, isNil, merge } from "lodash"; +import { isNil, merge } from "lodash"; -import { getMatchParamByName } from "src/util/query"; +import { getMatchParamByName, syncHistory } from "src/util/query"; import { appAttr } from "src/util/constants"; import { makeSessionsColumns, @@ -43,11 +43,9 @@ import { ICancelQueryRequest, } from "src/store/terminateQuery"; -import sortedTableStyles from "src/sortedtable/sortedtable.module.scss"; import statementsPageStyles from "src/statementsPage/statementsPage.module.scss"; import sessionPageStyles from "./sessionPage.module.scss"; -const sortableTableCx = classNames.bind(sortedTableStyles); const statementsPageCx = classNames.bind(statementsPageStyles); const sessionsPageCx = classNames.bind(sessionPageStyles); @@ -113,21 +111,6 @@ export class SessionsPage extends React.Component< }; }; - syncHistory = (params: Record): void => { - const { history } = this.props; - const currentSearchParams = new URLSearchParams(history.location.search); - forIn(params, (value, key) => { - if (!value) { - currentSearchParams.delete(key); - } else { - currentSearchParams.set(key, value); - } - }); - - history.location.search = currentSearchParams.toString(); - history.replace(history.location); - }; - changeSortSetting = (ss: SortSetting): void => { const { onSortingChange } = this.props; onSortingChange && onSortingChange(ss.columnTitle); @@ -136,10 +119,13 @@ export class SessionsPage extends React.Component< sortSetting: ss, }); - this.syncHistory({ - ascending: ss.ascending.toString(), - columnTitle: ss.columnTitle, - }); + syncHistory( + { + ascending: ss.ascending.toString(), + columnTitle: ss.columnTitle, + }, + this.props.history, + ); }; resetPagination = (): void => { diff --git a/pkg/ui/workspaces/cluster-ui/src/statementDetails/statementDetails.tsx b/pkg/ui/workspaces/cluster-ui/src/statementDetails/statementDetails.tsx index 609d815cd0e9..00f66c1c8358 100644 --- a/pkg/ui/workspaces/cluster-ui/src/statementDetails/statementDetails.tsx +++ b/pkg/ui/workspaces/cluster-ui/src/statementDetails/statementDetails.tsx @@ -386,7 +386,7 @@ export class StatementDetails extends React.Component< }; backToStatementsClick = (): void => { - this.props.history.push("/sql-activity/statements"); + this.props.history.push("/sql-activity?tab=statements"); if (this.props.onBackToStatementsClick) { this.props.onBackToStatementsClick(); } diff --git a/pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPage.tsx b/pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPage.tsx index 64877eda468f..b9be2351ca80 100644 --- a/pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPage.tsx +++ b/pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPage.tsx @@ -36,6 +36,7 @@ import { unique, containAny, queryByName, + syncHistory, } from "src/util"; import { AggregateStatistics, @@ -180,31 +181,18 @@ export class StatementsPage extends React.Component< }; }; - syncHistory = (params: Record): void => { - const { history } = this.props; - const nextSearchParams = new URLSearchParams(history.location.search); - - Object.entries(params).forEach(([key, value]) => { - if (!value) { - nextSearchParams.delete(key); - } else { - nextSearchParams.set(key, value); - } - }); - - history.location.search = nextSearchParams.toString(); - history.replace(history.location); - }; - changeSortSetting = (ss: SortSetting): void => { this.setState({ sortSetting: ss, }); - this.syncHistory({ - ascending: ss.ascending.toString(), - columnTitle: ss.columnTitle, - }); + syncHistory( + { + ascending: ss.ascending.toString(), + columnTitle: ss.columnTitle, + }, + this.props.history, + ); if (this.props.onSortingChange) { this.props.onSortingChange("Statements", ss.columnTitle, ss.ascending); } @@ -273,9 +261,12 @@ export class StatementsPage extends React.Component< onSubmitSearchField = (search: string): void => { this.setState({ search }); this.resetPagination(); - this.syncHistory({ - q: search, - }); + syncHistory( + { + q: search, + }, + this.props.history, + ); }; onSubmitFilters = (filters: Filters): void => { @@ -288,22 +279,28 @@ export class StatementsPage extends React.Component< }); this.resetPagination(); - this.syncHistory({ - app: filters.app, - timeNumber: filters.timeNumber, - timeUnit: filters.timeUnit, - sqlType: filters.sqlType, - database: filters.database, - regions: filters.regions, - nodes: filters.nodes, - }); + syncHistory( + { + app: filters.app, + timeNumber: filters.timeNumber, + timeUnit: filters.timeUnit, + sqlType: filters.sqlType, + database: filters.database, + regions: filters.regions, + nodes: filters.nodes, + }, + this.props.history, + ); }; onClearSearchField = (): void => { this.setState({ search: "" }); - this.syncHistory({ - q: undefined, - }); + syncHistory( + { + q: undefined, + }, + this.props.history, + ); }; onClearFilters = (): void => { @@ -314,15 +311,18 @@ export class StatementsPage extends React.Component< activeFilters: 0, }); this.resetPagination(); - this.syncHistory({ - app: undefined, - timeNumber: undefined, - timeUnit: undefined, - sqlType: undefined, - database: undefined, - regions: undefined, - nodes: undefined, - }); + syncHistory( + { + app: undefined, + timeNumber: undefined, + timeUnit: undefined, + sqlType: undefined, + database: undefined, + regions: undefined, + nodes: undefined, + }, + this.props.history, + ); }; filteredStatementsData = (): AggregateStatistics[] => { diff --git a/pkg/ui/workspaces/cluster-ui/src/transactionsPage/transactionsPage.tsx b/pkg/ui/workspaces/cluster-ui/src/transactionsPage/transactionsPage.tsx index 76ff011111f5..7c6c7e74c83c 100644 --- a/pkg/ui/workspaces/cluster-ui/src/transactionsPage/transactionsPage.tsx +++ b/pkg/ui/workspaces/cluster-ui/src/transactionsPage/transactionsPage.tsx @@ -34,15 +34,12 @@ import { generateRegionNode, getTrxAppFilterOptions, statementFingerprintIdsToText, -} from "./utils"; -import { searchTransactionsData, filterTransactions, getStatementsByFingerprintIdAndTime, } from "./utils"; -import { forIn } from "lodash"; import Long from "long"; -import { getSearchParams, unique } from "src/util"; +import { getSearchParams, unique, syncHistory } from "src/util"; import { EmptyTransactionsPlaceholder } from "./emptyTransactionsPlaceholder"; import { Loading } from "../loading"; import { PageConfig, PageConfigItem } from "../pageConfig"; @@ -152,30 +149,17 @@ export class TransactionsPage extends React.Component< this.refreshData(); } - syncHistory = (params: Record): void => { - const { history } = this.props; - const currentSearchParams = new URLSearchParams(history.location.search); - - forIn(params, (value, key) => { - if (!value) { - currentSearchParams.delete(key); - } else { - currentSearchParams.set(key, value); - } - }); - - history.location.search = currentSearchParams.toString(); - history.replace(history.location); - }; - onChangeSortSetting = (ss: SortSetting): void => { this.setState({ sortSetting: ss, }); - this.syncHistory({ - ascending: ss.ascending.toString(), - columnTitle: ss.columnTitle, - }); + syncHistory( + { + ascending: ss.ascending.toString(), + columnTitle: ss.columnTitle, + }, + this.props.history, + ); }; onChangePage = (current: number): void => { @@ -196,17 +180,23 @@ export class TransactionsPage extends React.Component< onClearSearchField = (): void => { this.setState({ search: "" }); - this.syncHistory({ - q: undefined, - }); + syncHistory( + { + q: undefined, + }, + this.props.history, + ); }; onSubmitSearchField = (search: string): void => { this.setState({ search }); this.resetPagination(); - this.syncHistory({ - q: search, - }); + syncHistory( + { + q: search, + }, + this.props.history, + ); }; onSubmitFilters = (filters: Filters): void => { @@ -217,13 +207,16 @@ export class TransactionsPage extends React.Component< }, }); this.resetPagination(); - this.syncHistory({ - app: filters.app, - timeNumber: filters.timeNumber, - timeUnit: filters.timeUnit, - regions: filters.regions, - nodes: filters.nodes, - }); + syncHistory( + { + app: filters.app, + timeNumber: filters.timeNumber, + timeUnit: filters.timeUnit, + regions: filters.regions, + nodes: filters.nodes, + }, + this.props.history, + ); }; onClearFilters = (): void => { @@ -233,13 +226,16 @@ export class TransactionsPage extends React.Component< }, }); this.resetPagination(); - this.syncHistory({ - app: undefined, - timeNumber: undefined, - timeUnit: undefined, - regions: undefined, - nodes: undefined, - }); + syncHistory( + { + app: undefined, + timeNumber: undefined, + timeUnit: undefined, + regions: undefined, + nodes: undefined, + }, + this.props.history, + ); }; handleDetails = (transaction?: TransactionInfo): void => { diff --git a/pkg/ui/workspaces/cluster-ui/src/util/query/query.ts b/pkg/ui/workspaces/cluster-ui/src/util/query/query.ts index 14dba7f7acf2..fdc27b9fe3e7 100644 --- a/pkg/ui/workspaces/cluster-ui/src/util/query/query.ts +++ b/pkg/ui/workspaces/cluster-ui/src/util/query/query.ts @@ -8,7 +8,7 @@ // by the Apache License, Version 2.0, included in the file // licenses/APL.txt. -import { Location } from "history"; +import { Location, History } from "history"; import { match as Match } from "react-router-dom"; interface ParamsObj { @@ -64,3 +64,26 @@ export function getMatchParamByName( } return null; } + +export function syncHistory( + params: Record, + history: History, +): void { + const nextSearchParams = new URLSearchParams(history.location.search); + + Object.entries(params).forEach(([key, value]) => { + if (!value) { + nextSearchParams.delete(key); + } else { + nextSearchParams.set(key, value); + } + }); + + history.location.search = nextSearchParams.toString(); + history.replace(history.location); +} + +export function clearHistory(history: History): void { + history.location.search = ""; + history.replace(history.location); +} diff --git a/pkg/ui/workspaces/db-console/src/app.spec.tsx b/pkg/ui/workspaces/db-console/src/app.spec.tsx index 6f20dbf9151d..4b314d66fa3f 100644 --- a/pkg/ui/workspaces/db-console/src/app.spec.tsx +++ b/pkg/ui/workspaces/db-console/src/app.spec.tsx @@ -339,10 +339,13 @@ describe("Routing to", () => { }); describe("'/statement' path", () => { - it("redirected to '/sql-activity/statements'", () => { + it("redirected to '/sql-activity?tab=statements'", () => { navigateToPath("/statement"); const location = history.location; - assert.equal(location.pathname, "/sql-activity/statements"); + assert.equal( + location.pathname + location.search, + "/sql-activity?tab=statements", + ); }); }); @@ -584,26 +587,35 @@ describe("Routing to", () => { }); describe("'/statements' path", () => { - it("redirected to '/sql-activity/statements'", () => { + it("redirected to '/sql-activity?tab=statements'", () => { navigateToPath("/statements"); const location = history.location; - assert.equal(location.pathname, "/sql-activity/statements"); + assert.equal( + location.pathname + location.search, + "/sql-activity?tab=statements", + ); }); }); describe("'/sessions' path", () => { - it("redirected to '/sql-activity/sessions'", () => { + it("redirected to '/sql-activity?tab=sessions'", () => { navigateToPath("/sessions"); const location = history.location; - assert.equal(location.pathname, "/sql-activity/sessions"); + assert.equal( + location.pathname + location.search, + "/sql-activity?tab=sessions", + ); }); }); describe("'/transactions' path", () => { - it("redirected to '/sql-activity/transactions'", () => { + it("redirected to '/sql-activity?tab=transactions'", () => { navigateToPath("/transactions"); const location = history.location; - assert.equal(location.pathname, "/sql-activity/transactions"); + assert.equal( + location.pathname + location.search, + "/sql-activity?tab=transactions", + ); }); }); }); diff --git a/pkg/ui/workspaces/db-console/src/app.tsx b/pkg/ui/workspaces/db-console/src/app.tsx index 31dfd297d696..d3e3c8f2065f 100644 --- a/pkg/ui/workspaces/db-console/src/app.tsx +++ b/pkg/ui/workspaces/db-console/src/app.tsx @@ -178,17 +178,12 @@ export const App: React.FC = (props: AppProps) => { {/* SQL activity */} - {/* statement statistics */} = (props: AppProps) => { {/* sessions */} = (props: AppProps) => { {/* debug pages */} diff --git a/pkg/ui/workspaces/db-console/src/views/sqlActivity/sqlActivityPage.tsx b/pkg/ui/workspaces/db-console/src/views/sqlActivity/sqlActivityPage.tsx index 2c8c7438fc03..de1ff8990347 100644 --- a/pkg/ui/workspaces/db-console/src/views/sqlActivity/sqlActivityPage.tsx +++ b/pkg/ui/workspaces/db-console/src/views/sqlActivity/sqlActivityPage.tsx @@ -13,21 +13,22 @@ import React from "react"; import { Tabs } from "antd"; -import { commonStyles } from "@cockroachlabs/cluster-ui"; +import { commonStyles, util } from "@cockroachlabs/cluster-ui"; import SessionsPageConnected from "src/views/sessions/sessionsPage"; import TransactionsPageConnected from "src/views/transactions/transactionsPage"; import StatementsPageConnected from "src/views/statements/statementsPage"; -import { getMatchParamByName } from "src/util/query"; import { RouteComponentProps } from "react-router-dom"; const { TabPane } = Tabs; const SQLActivityPage = (props: RouteComponentProps) => { - const defaultTab = getMatchParamByName(props.match, "tab") || "sessions"; + const defaultTab = util.queryByName(props.location, "tab") || "sessions"; const [currentTab, setCurrentTab] = React.useState(defaultTab); const onTabChange = (tabId: string): void => { setCurrentTab(tabId); + util.clearHistory(props.history); + util.syncHistory({ tab: tabId }, props.history); }; return (