Skip to content

Commit

Permalink
ui: degrade gracefully when regions aren't known
Browse files Browse the repository at this point in the history
Part of cockroachdb#89949

Previously, when a tenant SQL instance had spun down (leaving us no way
to remember which region it had been in), the SQL Activity pages would
claim that statements and transactions had occurred in an "undefined"
region.

This change moves from saying "undefined" to saying nothing at all, a
slightly nicer user experience.

This broader problem of losing the region mapping has been described
in cockroachdb#93268; we'll begin addressing it shortly.

Release note: None
  • Loading branch information
matthewtodd authored and adityamaru committed Dec 22, 2022
1 parent 2f6b015 commit fefd360
Show file tree
Hide file tree
Showing 6 changed files with 120 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -527,7 +527,9 @@ export class StatementDetails extends React.Component<
(stats.nodes || []).map(node => node.toString()),
).sort();
const regions = unique(
(stats.nodes || []).map(node => nodeRegions[node.toString()]),
(stats.nodes || [])
.map(node => nodeRegions[node.toString()])
.filter(r => r), // Remove undefined / unknown regions.
).sort();

const lastExec =
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
// Copyright 2022 The Cockroach Authors.
//
// Use of this software is governed by the Business Source License
// included in the file licenses/BSL.txt.
//
// As of the Change Date specified in that file, in accordance with
// the Business Source License, use of this software will be governed
// by the Apache License, Version 2.0, included in the file
// licenses/APL.txt.

import { assert } from "chai";
import Long from "long";
import {
AggregateStatistics,
populateRegionNodeForStatements,
} from "./statementsTable";

describe("populateRegionNodeForStatements", () => {
function statementWithNodeIDs(...nodeIDs: number[]): AggregateStatistics {
return {
aggregatedFingerprintID: "",
aggregatedFingerprintHexID: "",
label: "",
summary: "",
aggregatedTs: 0,
aggregationInterval: 0,
implicitTxn: false,
fullScan: false,
database: "",
applicationName: "",
stats: { nodes: nodeIDs.map(id => Long.fromInt(id)) },
};
}

it("maps nodes to regions, sorted", () => {
const statement = statementWithNodeIDs(1, 2);
populateRegionNodeForStatements([statement], {
"1": "gcp-us-west1",
"2": "gcp-us-east1",
});
assert.deepEqual(["gcp-us-east1", "gcp-us-west1"], statement.regions);
});

it("handles statements without nodes", () => {
const statement = statementWithNodeIDs();
populateRegionNodeForStatements([statement], {
"1": "gcp-us-west1",
"2": "gcp-us-east1",
});
assert.deepEqual(statement.regions, []);
});

it("excludes nodes whose region we don't know", () => {
const statement = statementWithNodeIDs(1, 2);
populateRegionNodeForStatements([statement], {
"1": "gcp-us-west1",
});
assert.deepEqual(statement.regions, ["gcp-us-west1"]);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -341,10 +341,13 @@ export function populateRegionNodeForStatements(
// E.g. {"gcp-us-east1" : [1,3,4]}
if (stmt.stats.nodes) {
stmt.stats.nodes.forEach(node => {
if (Object.keys(regions).includes(nodeRegions[node.toString()])) {
regions[nodeRegions[node.toString()]].add(longToInt(node));
} else {
regions[nodeRegions[node.toString()]] = new Set([longToInt(node)]);
const region = nodeRegions[node.toString()];
if (region) {
if (Object.keys(regions).includes(region)) {
regions[region].add(longToInt(node));
} else {
regions[region] = new Set([longToInt(node)]);
}
}
});
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -404,8 +404,6 @@ export class TransactionsPage extends React.Component<
const statements = data?.statements || [];
const { filters } = this.state;

// If the cluster is a tenant cluster we don't show info
// about nodes/regions.
const nodes = Object.keys(nodeRegions)
.map(n => Number(n))
.sort();
Expand Down
47 changes: 47 additions & 0 deletions pkg/ui/workspaces/cluster-ui/src/transactionsPage/utils.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import { assert } from "chai";
import {
filterTransactions,
generateRegion,
getStatementsByFingerprintId,
statementFingerprintIdsToText,
} from "./utils";
Expand All @@ -19,6 +20,8 @@ import { data, nodeRegions } from "./transactions.fixture";
import Long from "long";
import * as protos from "@cockroachlabs/crdb-protobuf-client";

type Statement =
protos.cockroach.server.serverpb.StatementsResponse.ICollectedStatementStatistics;
type Transaction =
protos.cockroach.server.serverpb.StatementsResponse.IExtendedCollectedTransactionStatistics;

Expand Down Expand Up @@ -288,3 +291,47 @@ SELECT _`,
);
});
});

describe("generateRegion", () => {
function transactionWithStatementFingerprintIDs(
...ids: number[]
): Transaction {
return {
stats_data: {
statement_fingerprint_ids: ids.map(id => Long.fromInt(id)),
},
};
}

function statementWithFingerprintAndNodeIDs(
id: number,
...nodeIDs: number[]
): Statement {
return {
id: Long.fromInt(id),
stats: { nodes: nodeIDs.map(id => Long.fromInt(id)) },
};
}

it("gathers up the list of regions for the transaction, sorted", () => {
assert.deepEqual(
generateRegion(
transactionWithStatementFingerprintIDs(42),
[statementWithFingerprintAndNodeIDs(42, 1, 2)],
{ "1": "gcp-us-west1", "2": "gcp-us-east1" },
),
["gcp-us-east1", "gcp-us-west1"],
);
});

it("skips over nodes with unknown regions", () => {
assert.deepEqual(
generateRegion(
transactionWithStatementFingerprintIDs(42),
[statementWithFingerprintAndNodeIDs(42, 1, 2)],
{ "1": "gcp-us-west1" },
),
["gcp-us-west1"],
);
});
});
4 changes: 3 additions & 1 deletion pkg/ui/workspaces/cluster-ui/src/transactionsPage/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,9 @@ export const generateRegion = (
});
});

return Array.from(regions).sort();
return Array.from(regions)
.filter(r => r) // Remove undefined / unknown regions.
.sort();
};

/**
Expand Down

0 comments on commit fefd360

Please sign in to comment.