Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
83014: ui: add internal app filter to active statements and transactions pages r=ericharmeling a=ericharmeling

This PR adds a single internal app filter option on to the Active Statements and Active Transactions pages. Active
statements and transactions run by internal apps are no longer displayed by default.

See commit message for release note.


https://user-images.githubusercontent.com/27286675/174156635-39d8649a-df91-4550-adb5-b3c167d54ed5.mov



Fixes #81072.

83707: roachtest: run workload from the tenant node r=knz a=stevendanna

The secure URL refers to paths on disk on the clusters in the
node. Since we only create the tenant-scoped certs on the tenant node,
we need to run workload from that node.

Fixes #82266
Depends on #83703

Release note: None

84003: storage: close pebble iter gracefully when NewPebbleSSTIterator fails r=erikgrinaker a=msbutler

Currently, if `pebble.NewExternalIter` sets pebbleIterator.inuse to True, but
then fails, the subsequent `pebbleIterator.destroy()` will panic unecessarily,
since the caller of `pebble.NewExternalIter` is not actually using the iter.
This bug causes TestBackupRestoreChecksum to flake in #83984.

To fix, this patch uses pebble.Close() to  gracefully close the pebbleIterator
if `pebble.NewExternalIter` fails.

Release Note: None

84039: prometheus: use older node_exporter r=nicktrav a=tbg

v1.3.1, the most up to date released version, has a bug that inflates
the bytes written by ~8x for NVMe drives (which in particular includes
the default drives for our GCE roachprod machines). Fundamentally this
is caused by the fact that these devices use a 4K sector size whereas
the kernel will always report based on a 512B sector size.

This took us a while to figure out, and to avoid repeating this exercise
periodically, downgrade node_exporter to 1.2.2, which pre-dates a
refactor that introduces the regression.

See: prometheus/node_exporter#2310

Release note: None


Co-authored-by: Eric Harmeling <eric.harmeling@cockroachlabs.com>
Co-authored-by: Steven Danna <danna@cockroachlabs.com>
Co-authored-by: Michael Butler <butler@cockroachlabs.com>
Co-authored-by: Tobias Grieger <tobias.b.grieger@gmail.com>
  • Loading branch information
5 people committed Jul 7, 2022
5 parents 6b7082a + c9f8474 + a01fa4d + 15ec780 + b6d9372 commit 2dd8e76
Show file tree
Hide file tree
Showing 11 changed files with 161 additions and 34 deletions.
2 changes: 1 addition & 1 deletion pkg/cmd/roachtest/tests/smoketest_secure.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ func multitenantSmokeTest(ctx context.Context, t test.Test, c cluster.Cluster) {

// init kv and check new database was done right
cmd := fmt.Sprintf("./cockroach workload init kv '%s'", ten.secureURL())
err = c.RunE(ctx, c.Node(1), cmd)
err = c.RunE(ctx, c.Node(2), cmd)
require.NoError(t, err)

sqlutils.MakeSQLRunner(db).CheckQueryResultsRetry(t, fmt.Sprintf(`
Expand Down
5 changes: 4 additions & 1 deletion pkg/roachprod/prometheus/prometheus.go
Original file line number Diff line number Diff line change
Expand Up @@ -192,12 +192,15 @@ func Init(
ctx context.Context, l *logger.Logger, c *install.SyncedCluster, cfg Config,
) (_ *Prometheus, _ error) {
if len(cfg.NodeExporter) > 0 {
// NB: when upgrading here, make sure to target a version that picks up this PR:
// https://github.com/prometheus/node_exporter/pull/2311
// At time of writing, there hasn't been a release in over half a year.
if err := c.RepeatRun(ctx, l, os.Stdout, os.Stderr, cfg.NodeExporter,
"download node exporter",
`
(sudo systemctl stop node_exporter || true) &&
rm -rf node_exporter && mkdir -p node_exporter && curl -fsSL \
https://github.com/prometheus/node_exporter/releases/download/v1.3.1/node_exporter-1.3.1.linux-amd64.tar.gz |
https://github.com/prometheus/node_exporter/releases/download/v1.2.2/node_exporter-1.2.2.linux-amd64.tar.gz |
tar zxv --strip-components 1 -C node_exporter
`); err != nil {
return nil, err
Expand Down
2 changes: 1 addition & 1 deletion pkg/storage/pebble_iterator.go
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ func newPebbleSSTIterator(files []sstable.ReadableFile, opts IterOptions) (*pebb

var err error
if p.iter, err = pebble.NewExternalIter(DefaultPebbleOptions(), &p.options, files); err != nil {
p.destroy()
p.Close()
return nil, err
}
return p, nil
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ import {
ActiveTransaction,
ActiveStatement,
SessionStatusType,
ActiveStatementFilters,
ActiveTransactionFilters,
} from "./types";
import * as protos from "@cockroachlabs/crdb-protobuf-client";
import moment from "moment";
Expand All @@ -26,6 +28,7 @@ import {
getActiveStatementsFromSessions,
filterActiveStatements,
filterActiveTransactions,
INTERNAL_APP_NAME_PREFIX,
} from "./activeStatementUtils";

type ActiveQuery = protos.cockroach.server.serverpb.ActiveQuery;
Expand Down Expand Up @@ -108,9 +111,12 @@ describe("test activeStatementUtils", () => {
makeActiveStatement({ executionID: "4", application: "app1" }),
];

const filters = { app: "app1" };
const filtered = filterActiveStatements(statements, filters);

const filters: ActiveStatementFilters = { app: "app1" };
const filtered = filterActiveStatements(
statements,
filters,
INTERNAL_APP_NAME_PREFIX,
);
expect(filtered.length).toBe(2);
expect(filtered[0].executionID).toBe("1");
expect(filtered[1].executionID).toBe("4");
Expand Down Expand Up @@ -140,9 +146,14 @@ describe("test activeStatementUtils", () => {
}),
];

const filters = { app: "app1" };
const filters: ActiveStatementFilters = { app: "app1" };
const search = "SELECT 1";
const filtered = filterActiveStatements(statements, filters, search);
const filtered = filterActiveStatements(
statements,
filters,
INTERNAL_APP_NAME_PREFIX,
search,
);

expect(filtered.length).toBe(2);
expect(filtered[0].executionID).toBe("1");
Expand All @@ -158,8 +169,12 @@ describe("test activeStatementUtils", () => {
makeActiveStatement(),
];

const filters = { app: "" };
const filtered = filterActiveStatements(statements, filters, "");
const filters: ActiveStatementFilters = {};
const filtered = filterActiveStatements(
statements,
filters,
INTERNAL_APP_NAME_PREFIX,
);

expect(filtered.length).toBe(statements.length);
});
Expand Down Expand Up @@ -196,7 +211,7 @@ describe("test activeStatementUtils", () => {
},
],
errors: [],
internal_app_name_prefix: "",
internal_app_name_prefix: INTERNAL_APP_NAME_PREFIX,
toJSON: () => ({}),
};

Expand Down Expand Up @@ -237,7 +252,10 @@ describe("test activeStatementUtils", () => {
makeActiveStatement({ application: "app3" }),
makeActiveStatement({ application: "app4" }),
];
const apps = getAppsFromActiveStatements(activeStatements);
const apps = getAppsFromActiveStatements(
activeStatements,
INTERNAL_APP_NAME_PREFIX,
);
expect(apps).toEqual(["app1", "app2", "app3", "app4"]);
});

Expand Down Expand Up @@ -290,7 +308,7 @@ describe("test activeStatementUtils", () => {
},
],
errors: [],
internal_app_name_prefix: "",
internal_app_name_prefix: INTERNAL_APP_NAME_PREFIX,
toJSON: () => ({}),
};

Expand Down Expand Up @@ -328,8 +346,12 @@ describe("test activeStatementUtils", () => {
makeActiveTxn({ executionID: "4", application: "app1" }),
];

const filters = { app: "app1" };
const filtered = filterActiveTransactions(txns, filters);
const filters: ActiveTransactionFilters = { app: "app1" };
const filtered = filterActiveTransactions(
txns,
filters,
INTERNAL_APP_NAME_PREFIX,
);

expect(filtered.length).toBe(2);
expect(filtered[0].executionID).toBe("1");
Expand Down Expand Up @@ -359,9 +381,14 @@ describe("test activeStatementUtils", () => {
}),
];

const filters = { app: "app1" };
const filters: ActiveTransactionFilters = { app: "app1" };
const search = "SELECT 1";
const filtered = filterActiveTransactions(txns, filters, search);
const filtered = filterActiveTransactions(
txns,
filters,
INTERNAL_APP_NAME_PREFIX,
search,
);

expect(filtered.length).toBe(2);
expect(filtered[0].executionID).toBe("1");
Expand All @@ -377,8 +404,12 @@ describe("test activeStatementUtils", () => {
makeActiveTxn(),
];

const filters = { app: "" };
const filtered = filterActiveTransactions(txns, filters, "");
const filters: ActiveTransactionFilters = {};
const filtered = filterActiveTransactions(
txns,
filters,
INTERNAL_APP_NAME_PREFIX,
);

expect(filtered.length).toBe(txns.length);
});
Expand All @@ -392,7 +423,10 @@ describe("test activeStatementUtils", () => {
makeActiveTxn({ application: "app4" }),
];

const apps = getAppsFromActiveTransactions(activeTxns);
const apps = getAppsFromActiveTransactions(
activeTxns,
INTERNAL_APP_NAME_PREFIX,
);
expect(apps).toEqual(["app1", "app2", "app3", "app4"]);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@

import moment, { Moment } from "moment";
import { byteArrayToUuid } from "src/sessions";
import { TimestampToMoment } from "src/util";
import { TimestampToMoment, unset } from "src/util";
import { ActiveTransaction } from ".";
import {
SessionsResponse,
Expand All @@ -22,17 +22,42 @@ import {
import { ActiveStatement, ActiveStatementFilters } from "./types";

export const ACTIVE_STATEMENT_SEARCH_PARAM = "q";
export const INTERNAL_APP_NAME_PREFIX = "$ internal";

export function filterActiveStatements(
statements: ActiveStatement[],
filters: ActiveStatementFilters,
internalAppNamePrefix: string,
search?: string,
): ActiveStatement[] {
if (statements == null) return [];

let filteredStatements = statements.filter(
stmt => filters.app === "" || stmt.application === filters.app,
);
let filteredStatements = statements;

const isInternal = (statement: ActiveStatement) =>
statement.application.startsWith(internalAppNamePrefix);
if (filters.app) {
filteredStatements = filteredStatements.filter(
(statement: ActiveStatement) => {
const apps = filters.app.toString().split(",");
let showInternal = false;
if (apps.includes(internalAppNamePrefix)) {
showInternal = true;
}
if (apps.includes(unset)) {
apps.push("");
}
return (
(showInternal && isInternal(statement)) ||
apps.includes(statement.application)
);
},
);
} else {
filteredStatements = filteredStatements.filter(
statement => !isInternal(statement),
);
}

if (search) {
filteredStatements = filteredStatements.filter(stmt =>
Expand Down Expand Up @@ -84,27 +109,51 @@ export function getActiveStatementsFromSessions(

export function getAppsFromActiveStatements(
statements: ActiveStatement[] | null,
internalAppNamePrefix: string,
): string[] {
if (statements == null) return [];
return Array.from(
statements.reduce(
(apps, stmt) => apps.add(stmt.application),
new Set<string>(),
),

const uniqueAppNames = new Set(
statements.map(s => {
if (s.application.startsWith(internalAppNamePrefix)) {
return internalAppNamePrefix;
}
return s.application ? s.application : unset;
}),
);

return Array.from(uniqueAppNames).sort();
}

export function filterActiveTransactions(
txns: ActiveTransaction[] | null,
filters: ActiveTransactionFilters,
internalAppNamePrefix: string,
search?: string,
): ActiveTransaction[] {
if (txns == null) return [];

let filteredTxns = txns;

const isInternal = (txn: ActiveTransaction) =>
txn.application.startsWith(internalAppNamePrefix);
if (filters.app) {
filteredTxns = filteredTxns.filter(txn => txn.application === filters.app);
filteredTxns = filteredTxns.filter((txn: ActiveTransaction) => {
const apps = filters.app.toString().split(",");
let showInternal = false;
if (apps.includes(internalAppNamePrefix)) {
showInternal = true;
}
if (apps.includes(unset)) {
apps.push("");
}

return (
(showInternal && isInternal(txn)) || apps.includes(txn.application)
);
});
} else {
filteredTxns = filteredTxns.filter(txn => !isInternal(txn));
}

if (search) {
Expand All @@ -118,12 +167,20 @@ export function filterActiveTransactions(

export function getAppsFromActiveTransactions(
txns: ActiveTransaction[],
internalAppNamePrefix: string,
): string[] {
if (txns == null) return [];

return Array.from(
txns.reduce((apps, txn) => apps.add(txn.application), new Set<string>()),
const uniqueAppNames = new Set(
txns.map(t => {
if (t.application.startsWith(internalAppNamePrefix)) {
return internalAppNamePrefix;
}
return t.application ? t.application : unset;
}),
);

return Array.from(uniqueAppNames).sort();
}

export function getActiveTransactionsFromSessions(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,14 @@ export const selectActiveStatements = createSelector(
},
);

export const selectAppName = createSelector(
(state: AppState) => state.adminUI.sessions,
response => {
if (!response.data) return null;
return response.data.internal_app_name_prefix;
},
);

export const selectSortSetting = (state: AppState): SortSetting =>
localStorageSelector(state)["sortSetting/ActiveStatementsPage"];

Expand Down Expand Up @@ -59,6 +67,7 @@ export const mapStateToActiveStatementsPageProps = (
selectedColumns: selectColumns(state),
sortSetting: selectSortSetting(state),
filters: selectFilters(state),
internalAppNamePrefix: selectAppName(state),
});

export const mapDispatchToActiveStatementsPageProps = (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ export type ActiveStatementsViewStateProps = {
sortSetting: SortSetting;
sessionsError: Error | null;
filters: ActiveStatementFilters;
internalAppNamePrefix: string;
};

export type ActiveStatementsViewProps = ActiveStatementsViewStateProps &
Expand All @@ -68,6 +69,7 @@ export const ActiveStatementsView: React.FC<ActiveStatementsViewProps> = ({
statements,
sessionsError,
filters,
internalAppNamePrefix,
}: ActiveStatementsViewProps) => {
const [pagination, setPagination] = useState<ISortedTablePagination>({
current: 1,
Expand Down Expand Up @@ -152,12 +154,13 @@ export const ActiveStatementsView: React.FC<ActiveStatementsViewProps> = ({
const clearSearch = () => onSubmitSearch("");
const clearFilters = () => onSubmitFilters({ app: inactiveFiltersState.app });

const apps = getAppsFromActiveStatements(statements);
const apps = getAppsFromActiveStatements(statements, internalAppNamePrefix);
const countActiveFilters = calculateActiveFilters(filters);

const filteredStatements = filterActiveStatements(
statements,
filters,
internalAppNamePrefix,
search,
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import { createSelector } from "reselect";
import { getActiveTransactionsFromSessions } from "../activeExecutions/activeStatementUtils";
import { localStorageSelector } from "src/statementsPage/statementsPage.selectors";
import { selectAppName } from "src/statementsPage/activeStatementsPage.selectors";
import {
ActiveTransactionFilters,
ActiveTransactionsViewDispatchProps,
Expand Down Expand Up @@ -62,6 +63,7 @@ export const mapStateToActiveTransactionsPageProps = (
selectedColumns: selectColumns(state),
sortSetting: selectSortSetting(state),
filters: selectFilters(state),
internalAppNamePrefix: selectAppName(state),
});

export const mapDispatchToActiveTransactionsPageProps = (
Expand Down

0 comments on commit 2dd8e76

Please sign in to comment.