Skip to content

Commit

Permalink
ui: add selected period as part of cached key
Browse files Browse the repository at this point in the history
Previously, the fingerprint id and the app names were used
as a key for a statement details cache. This commits adds
the start and end time (when existing) to the key, so
the details are correctly assigned to the selected period.

This commit also rounds the selected value period to the hour,
since that is what is used on the persisted statistics, with
the start value keeping the hour and the end value adding one
hour, for example:
start: 17:45:23  ->  17:00:00
end:   20:14:32  ->  21:00:00

Partially addresses cockroachdb#72129

Release note: None
Release Justification: Low risk, high benefit change
  • Loading branch information
maryliag committed Mar 9, 2022
1 parent fe889cf commit a432d7f
Show file tree
Hide file tree
Showing 12 changed files with 277 additions and 140 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
// by the Apache License, Version 2.0, included in the file
// licenses/APL.txt.

import Long from "long";
import { createSelector } from "@reduxjs/toolkit";
import { RouteComponentProps } from "react-router-dom";
import { AppState } from "../store";
Expand All @@ -16,47 +17,33 @@ import {
statementAttr,
getMatchParamByName,
queryByName,
generateStmtDetailsToID,
} from "../util";
import { cockroach } from "@cockroachlabs/crdb-protobuf-client";
import { TimeScale, toRoundedDateRange } from "../timeScaleDropdown";
import { selectTimeScale } from "../statementsPage/statementsPage.selectors";
type StatementDetailsResponseMessage = cockroach.server.serverpb.StatementDetailsResponse;

export const generateStmtDetailsToID = (
fingerprintID: string,
appNames: string,
): string => {
if (
appNames &&
(appNames.includes("$ internal") || appNames.includes("unset"))
) {
const apps = appNames.split(",");
for (let i = 0; i < apps.length; i++) {
if (apps[i].includes("$ internal")) {
apps[i] = "$ internal";
}
if (apps[i].includes("unset")) {
apps[i] = "";
}
}
appNames = apps.toString();
}
if (appNames) {
return fingerprintID + appNames;
}
return fingerprintID;
};

export const selectStatementDetails = createSelector(
(_state: AppState, props: RouteComponentProps): string =>
getMatchParamByName(props.match, statementAttr),
(_state: AppState, props: RouteComponentProps): string =>
queryByName(props.location, appNamesAttr),
(state: AppState): TimeScale => selectTimeScale(state),
(state: AppState) => state.adminUI.sqlDetailsStats,
(
fingerprintID,
appNames,
timeScale,
statementDetailsStats,
): StatementDetailsResponseMessage => {
const key = generateStmtDetailsToID(fingerprintID, appNames);
const [start, end] = toRoundedDateRange(timeScale);
const key = generateStmtDetailsToID(
fingerprintID,
appNames,
Long.fromNumber(start.unix()),
Long.fromNumber(end.unix()),
);
if (Object.keys(statementDetailsStats).includes(key)) {
return statementDetailsStats[key].data;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ import { commonStyles } from "src/common";
import { NodeSummaryStats } from "../nodes";
import { UIConfigState } from "../store";
import moment from "moment";
import { TimeScale, toDateRange } from "../timeScaleDropdown";
import { TimeScale, toRoundedDateRange } from "../timeScaleDropdown";
import { StatementDetailsRequest } from "src/api/statementsApi";
import SQLActivityError from "../sqlActivity/errorComponent";
import {
Expand Down Expand Up @@ -162,7 +162,7 @@ const summaryCardStylesCx = classNames.bind(summaryCardStyles);
function statementDetailsRequestFromProps(
props: StatementDetailsProps,
): cockroach.server.serverpb.StatementDetailsRequest {
const [start, end] = toDateRange(props.timeScale);
const [start, end] = toRoundedDateRange(props.timeScale);
const statementFingerprintID = getMatchParamByName(
props.match,
statementAttr,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ describe("SQLDetailsStats sagas", () => {
type: "request",
};
const key =
"SELECT * FROM crdb_internal.node_build_info$ cockroach sql,newname";
"SELECT * FROM crdb_internal.node_build_info/$ cockroach sql,newname/0/0";
const SQLDetailsStatsResponse = new cockroach.server.serverpb.StatementDetailsResponse(
{
statement: {
Expand Down Expand Up @@ -659,7 +659,7 @@ describe("SQLDetailsStats sagas", () => {
)
.withReducer(reducer)
.hasFinalState<Record<string, SQLDetailsStatsState>>({
"SELECT * FROM crdb_internal.node_build_info$ cockroach sql,newname": {
"SELECT * FROM crdb_internal.node_build_info/$ cockroach sql,newname/0/0": {
data: SQLDetailsStatsResponse,
lastError: null,
valid: true,
Expand All @@ -680,7 +680,7 @@ describe("SQLDetailsStats sagas", () => {
)
.withReducer(reducer)
.hasFinalState<Record<string, SQLDetailsStatsState>>({
"SELECT * FROM crdb_internal.node_build_info$ cockroach sql,newname": {
"SELECT * FROM crdb_internal.node_build_info/$ cockroach sql,newname/0/0": {
data: null,
lastError: error,
valid: false,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import {
} from "src/api/statementsApi";
import { actions as sqlDetailsStatsActions } from "./statementDetails.reducer";
import { CACHE_INVALIDATION_PERIOD } from "src/store/utils";
import { generateStmtDetailsToID } from "../../statementDetails/statementDetails.selectors";
import { generateStmtDetailsToID } from "../../util";

export function* refreshSQLDetailsStatsSaga(
action: PayloadAction<StatementDetailsRequest>,
Expand All @@ -33,6 +33,8 @@ export function* requestSQLDetailsStatsSaga(
? generateStmtDetailsToID(
action.payload.fingerprint_id,
action.payload.app_names.toString(),
action.payload.start,
action.payload.end,
)
: "";
try {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,11 @@ import {
TimeScaleDropdownProps,
TimeScaleDropdown,
} from "./timeScaleDropdown";
import { defaultTimeScaleOptions, findClosestTimeScale } from "./utils";
import {
defaultTimeScaleOptions,
findClosestTimeScale,
toRoundedDateRange,
} from "./utils";
import * as timescale from "./timeScaleTypes";
import moment from "moment";
import { MemoryRouter } from "react-router";
Expand Down Expand Up @@ -204,6 +208,32 @@ describe("<TimeScaleDropdown>", function() {
});

describe("timescale utils", (): void => {
describe("toRoundedDateRange", () => {
it("round values", () => {
const ts: TimeScale = {
windowSize: moment.duration(5, "day"),
sampleSize: moment.duration(5, "minutes"),
fixedWindowEnd: moment.utc("2022.01.10 13:42"),
key: "Custom",
};
const [start, end] = toRoundedDateRange(ts);
assert.equal(start.format("YYYY.MM.DD HH:mm:ss"), "2022.01.05 13:00:00");
assert.equal(end.format("YYYY.MM.DD HH:mm:ss"), "2022.01.10 14:00:00");
});

it("already rounded values", () => {
const ts: TimeScale = {
windowSize: moment.duration(5, "day"),
sampleSize: moment.duration(5, "minutes"),
fixedWindowEnd: moment.utc("2022.01.10 13:00"),
key: "Custom",
};
const [start, end] = toRoundedDateRange(ts);
assert.equal(start.format("YYYY.MM.DD HH:mm:ss"), "2022.01.05 13:00:00");
assert.equal(end.format("YYYY.MM.DD HH:mm:ss"), "2022.01.10 14:00:00");
});
});

describe("findClosestTimeScale", () => {
it("should find the correct time scale", () => {
// `seconds` != window size of any of the default options, `startSeconds` not specified.
Expand Down
12 changes: 12 additions & 0 deletions pkg/ui/workspaces/cluster-ui/src/timeScaleDropdown/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,18 @@ export const toDateRange = (ts: TimeScale): [moment.Moment, moment.Moment] => {
return [start, end];
};

export const toRoundedDateRange = (
ts: TimeScale,
): [moment.Moment, moment.Moment] => {
const [start, end] = toDateRange(ts);
const startRounded = start.set({ minute: 0, second: 0, millisecond: 0 });
const endRounded = end
.set({ minute: 0, second: 0, millisecond: 0 })
.add(1, "hours");

return [startRounded, endRounded];
};

export const findClosestTimeScale = (
options: TimeScaleOptions,
seconds: number,
Expand Down
37 changes: 37 additions & 0 deletions pkg/ui/workspaces/cluster-ui/src/util/appStats/appStats.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import {
uniqueLong,
unique,
} from "src/util";
import Long from "long";

export type StatementStatistics = protos.cockroach.sql.IStatementStatistics;
export type ExecStats = protos.cockroach.sql.IExecStats;
Expand Down Expand Up @@ -283,3 +284,39 @@ export function transactionScopedStatementKey(
): string {
return statementKey(stmt) + stmt.transaction_fingerprint_id.toString();
}

export const generateStmtDetailsToID = (
fingerprintID: string,
appNames: string,
start: Long,
end: Long,
): string => {
if (
appNames &&
(appNames.includes("$ internal") || appNames.includes("unset"))
) {
const apps = appNames.split(",");
for (let i = 0; i < apps.length; i++) {
if (apps[i].includes("$ internal")) {
apps[i] = "$ internal";
}
if (apps[i].includes("unset")) {
apps[i] = "";
}
}
appNames = unique(apps)
.sort()
.toString();
}
let generatedID = fingerprintID;
if (appNames) {
generatedID += `/${appNames}`;
}
if (start) {
generatedID += `/${start}`;
}
if (end) {
generatedID += `/${end}`;
}
return generatedID;
};
3 changes: 3 additions & 0 deletions pkg/ui/workspaces/cluster-ui/src/util/dataFromServer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@ export interface DataFromServer {
Tag: string;
Version: string;
NodeID: string;
OIDCAutoLogin: boolean;
OIDCLoginEnabled: boolean;
OIDCButtonText: string;
}

// Tell TypeScript about `window.dataFromServer`, which is set in a script
Expand Down
1 change: 1 addition & 0 deletions pkg/ui/workspaces/db-console/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@
"@types/fetch-mock": "^5.8.1",
"@types/highlight.js": "^10.1.0",
"@types/lodash": "^4.14.149",
"@types/long": "^4.0.1",
"@types/mocha": "^2.2.40",
"@types/node": "^13.7.0",
"@types/nvd3": "^1.8.36",
Expand Down
30 changes: 4 additions & 26 deletions pkg/ui/workspaces/db-console/src/redux/apiReducers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@
import _ from "lodash";
import { combineReducers } from "redux";
import moment from "moment";
import { util } from "@cockroachlabs/cluster-ui";
const { generateStmtDetailsToID } = util;

import {
CachedDataReducer,
Expand Down Expand Up @@ -314,40 +316,16 @@ const queriesReducerObj = new CachedDataReducer(
export const invalidateStatements = queriesReducerObj.invalidateData;
export const refreshStatements = queriesReducerObj.refresh;

// TODO (maryliag) add period selected to generate ID
export const statementDetailsRequestToID = (
req: api.StatementDetailsRequestMessage,
): string =>
generateStmtDetailsToID(
req.fingerprint_id.toString(),
req.app_names.toString(),
req.start,
req.end,
);

export const generateStmtDetailsToID = (
fingerprintID: string,
appNames: string,
): string => {
if (
appNames &&
(appNames.includes("$ internal") || appNames.includes("unset"))
) {
const apps = appNames.split(",");
for (let i = 0; i < apps.length; i++) {
if (apps[i].includes("$ internal")) {
apps[i] = "$ internal";
}
if (apps[i].includes("unset")) {
apps[i] = "";
}
}
appNames = apps.toString();
}
if (appNames) {
return fingerprintID + appNames;
}
return fingerprintID;
};

const queryReducerObj = new KeyedCachedDataReducer(
api.getStatementDetails,
"statementDetails",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,14 @@
import { connect } from "react-redux";
import { withRouter } from "react-router-dom";
import { createSelector } from "reselect";
import Long from "long";

import {
refreshLiveness,
refreshNodes,
refreshStatementDiagnosticsRequests,
refreshStatementDetails,
refreshUserSQLRoles,
generateStmtDetailsToID,
} from "src/redux/apiReducers";
import { RouteComponentProps } from "react-router";
import {
Expand All @@ -30,6 +30,9 @@ import {
StatementDetails,
StatementDetailsDispatchProps,
StatementDetailsStateProps,
TimeScale,
toRoundedDateRange,
util,
} from "@cockroachlabs/cluster-ui";
import {
cancelStatementDiagnosticsReportAction,
Expand All @@ -50,18 +53,29 @@ import { getMatchParamByName, queryByName } from "src/util/query";
import { appNamesAttr, statementAttr } from "src/util/constants";
type IStatementDiagnosticsReport = protos.cockroach.server.serverpb.IStatementDiagnosticsReport;

const { generateStmtDetailsToID } = util;

export const selectStatementDetails = createSelector(
(_state: AdminUIState, props: RouteComponentProps): string =>
getMatchParamByName(props.match, statementAttr),
(_state: AdminUIState, props: RouteComponentProps): string =>
queryByName(props.location, appNamesAttr),
(state: AdminUIState): TimeScale =>
statementsTimeScaleLocalSetting.selector(state),
(state: AdminUIState) => state.cachedData.statementDetails,
(
fingerprintID,
appNames,
timeScale,
statementDetailsStats,
): StatementDetailsResponseMessage => {
const key = generateStmtDetailsToID(fingerprintID, appNames);
const [start, end] = toRoundedDateRange(timeScale);
const key = generateStmtDetailsToID(
fingerprintID,
appNames,
Long.fromNumber(start.unix()),
Long.fromNumber(end.unix()),
);
if (Object.keys(statementDetailsStats).includes(key)) {
return statementDetailsStats[key].data;
}
Expand Down
Loading

0 comments on commit a432d7f

Please sign in to comment.