Merge cockroachdb#78174 cockroachdb#78203

78174: kvserver: don't use caller's context for ProposalData r=[erikgrinaker,nvanbenschoten] a=tbg

Fixes cockroachdb#75656.

Release note: None


78203: ui: remove unnecessary uses of lodash in database pages r=xinhaoz a=xinhaoz

Closes cockroachdb#68820

This commit removes the ue of functions from the lodash function
where regular JS can be used.

Release note: None

Co-authored-by: Tobias Grieger <[email protected]>
Co-authored-by: Xin Hao Zhang <[email protected]>

pkg/kv/kvserver/replica_application_cmd.go

@@ -51,9 +51,7 @@ type replicatedCmd struct {
 	// Replica's proposal map.
 	proposal *ProposalData
 
-	// ctx is a context that follows from the proposal's context if it was
-	// proposed locally. Otherwise, it will follow from the context passed to
-	// ApplyCommittedEntries.
+	// ctx is a non-cancelable context used to apply the command.
 	ctx context.Context
 	// sp is the tracing span corresponding to ctx. It is closed in
 	// finishTracingSpan. This span "follows from" the proposer's span (even

pkg/kv/kvserver/replica_application_decoder.go

@@ -12,8 +12,10 @@ package kvserver
 
 import (
 	"context"
+	"fmt"
 
 	"github.com/cockroachdb/cockroach/pkg/kv/kvserver/apply"
+	"github.com/cockroachdb/cockroach/pkg/util"
 	"github.com/cockroachdb/cockroach/pkg/util/log"
 	"github.com/cockroachdb/cockroach/pkg/util/quotapool"
 	"github.com/cockroachdb/cockroach/pkg/util/tracing"
@@ -144,8 +146,19 @@ func (d *replicaDecoder) createTracingSpans(ctx context.Context) {
 	var it replicatedCmdBufSlice
 	for it.init(&d.cmdBuf); it.Valid(); it.Next() {
 		cmd := it.cur()
+
 		if cmd.IsLocal() {
-			cmd.ctx, cmd.sp = tracing.ChildSpan(cmd.proposal.ctx, opName)
+			// We intentionally don't propagate the client's cancellation policy (in
+			// cmd.ctx) onto the request. See #75656.
+			propCtx := ctx // raft scheduler's ctx
+			var propSp *tracing.Span
+			// If the client has a trace, put a child into propCtx.
+			if sp := tracing.SpanFromContext(cmd.proposal.ctx); sp != nil {
+				propCtx, propSp = sp.Tracer().StartSpanCtx(
+					propCtx, "local proposal", tracing.WithParent(sp),
+				)
+			}
+			cmd.ctx, cmd.sp = propCtx, propSp
 		} else if cmd.raftCmd.TraceData != nil {
 			// The proposal isn't local, and trace data is available. Extract
 			// the remote span and start a server-side span that follows from it.
@@ -167,6 +180,10 @@ func (d *replicaDecoder) createTracingSpans(ctx context.Context) {
 		} else {
 			cmd.ctx, cmd.sp = tracing.ChildSpan(ctx, opName)
 		}
+
+		if util.RaceEnabled && cmd.ctx.Done() != nil {
+			panic(fmt.Sprintf("cancelable context observed during raft application: %+v", cmd))
+		}
 	}
 }
 

pkg/kv/kvserver/replica_proposal.go

@@ -45,7 +45,7 @@ import (
 // be returned to the caller.
 type ProposalData struct {
 	// The caller's context, used for logging proposals, reproposals, message
-	// sends, and command application. In order to enable safely tracing events
+	// sends, but not command application. In order to enable safely tracing events
 	// beneath, modifying this ctx field in *ProposalData requires holding the
 	// raftMu.
 	ctx context.Context

pkg/ui/workspaces/cluster-ui/src/databaseDetailsPage/databaseDetailsPage.stories.tsx

@@ -20,6 +20,7 @@ import {
 } from "src/storybook/fixtures";
 import {
   DatabaseDetailsPage,
+  DatabaseDetailsPageDataTable,
   DatabaseDetailsPageProps,
   ViewMode,
 } from "./databaseDetailsPage";
@@ -86,41 +87,40 @@ const withoutData: DatabaseDetailsPageProps = {
   },
 };
 
+function createTable(): DatabaseDetailsPageDataTable {
+  const roles = _.uniq(new Array(_.random(1, 3)).map(() => randomRole()));
+  const grants = _.uniq(
+    new Array(_.random(1, 5)).map(() => randomTablePrivilege()),
+  );
+
+  return {
+    name: randomName(),
+    details: {
+      loading: false,
+      loaded: true,
+      columnCount: _.random(5, 42),
+      indexCount: _.random(1, 6),
+      userCount: roles.length,
+      roles: roles,
+      grants: grants,
+      statsLastUpdated: moment("0001-01-01T00:00:00Z"),
+    },
+    stats: {
+      loading: false,
+      loaded: true,
+      replicationSizeInBytes: _.random(1000.0) * 1024 ** _.random(1, 2),
+      rangeCount: _.random(50, 500),
+      nodesByRegionString:
+        "gcp-europe-west1(n8), gcp-us-east1(n1), gcp-us-west1(n6)",
+    },
+  };
+}
+
 const withData: DatabaseDetailsPageProps = {
   loading: false,
   loaded: true,
   name: randomName(),
-  tables: _.map(Array(42), _item => {
-    const roles = _.uniq(
-      _.map(new Array(_.random(1, 3)), _item => randomRole()),
-    );
-    const grants = _.uniq(
-      _.map(new Array(_.random(1, 5)), _item => randomTablePrivilege()),
-    );
-
-    return {
-      name: randomName(),
-      details: {
-        loading: false,
-        loaded: true,
-        columnCount: _.random(5, 42),
-        indexCount: _.random(1, 6),
-        userCount: roles.length,
-        roles: roles,
-        grants: grants,
-        statsLastUpdated: moment("0001-01-01T00:00:00Z"),
-      },
-      showNodeRegionsColumn: true,
-      stats: {
-        loading: false,
-        loaded: true,
-        replicationSizeInBytes: _.random(1000.0) * 1024 ** _.random(1, 2),
-        rangeCount: _.random(50, 500),
-        nodesByRegionString:
-          "gcp-europe-west1(n8), gcp-us-east1(n1), gcp-us-west1(n6)",
-      },
-    };
-  }),
+  tables: [createTable()],
   viewMode: ViewMode.Tables,
   sortSettingTables: {
     ascending: false,

pkg/ui/workspaces/cluster-ui/src/databaseDetailsPage/databaseDetailsPage.tsx

@@ -12,8 +12,6 @@ import React from "react";
 import { Link, RouteComponentProps } from "react-router-dom";
 import { Tooltip } from "antd";
 import classNames from "classnames/bind";
-import _ from "lodash";
-
 import { Breadcrumbs } from "src/breadcrumbs";
 import { Dropdown, DropdownOption } from "src/dropdown";
 import { CaretRight } from "src/icon/caretRight";
@@ -203,7 +201,7 @@ export class DatabaseDetailsPage extends React.Component<
       return this.props.refreshDatabaseDetails(this.props.name);
     }
 
-    _.forEach(this.props.tables, table => {
+    this.props.tables.forEach(table => {
       if (!table.details.loaded && !table.details.loading) {
         return this.props.refreshTableDetails(this.props.name, table.name);
       }
@@ -417,8 +415,8 @@ export class DatabaseDetailsPage extends React.Component<
             Roles
           </Tooltip>
         ),
-        cell: table => _.join(table.details.roles, ", "),
-        sort: table => _.join(table.details.roles, ", "),
+        cell: table => table.details.roles.join(", "),
+        sort: table => table.details.roles.join(", "),
         className: cx("database-table__col-roles"),
         name: "roles",
       },
@@ -428,8 +426,8 @@ export class DatabaseDetailsPage extends React.Component<
             Grants
           </Tooltip>
         ),
-        cell: table => _.join(table.details.grants, ", "),
-        sort: table => _.join(table.details.grants, ", "),
+        cell: table => table.details.grants.join(", "),
+        sort: table => table.details.grants.join(", "),
         className: cx("database-table__col-grants"),
         name: "grants",
       },

pkg/ui/workspaces/cluster-ui/src/databaseTablePage/databaseTablePage.stories.tsx

@@ -10,7 +10,6 @@
 
 import React from "react";
 import { storiesOf } from "@storybook/react";
-import _ from "lodash";
 
 import { withBackground, withRouterProvider } from "src/storybook/decorators";
 import {
@@ -84,15 +83,13 @@ const withData: DatabaseTablePageProps = {
       )
     `,
     replicaCount: 7,
-    indexNames: _.map(Array(3), randomName),
-    grants: _.uniq(
-      _.map(Array(12), () => {
-        return {
-          user: randomRole(),
-          privilege: randomTablePrivilege(),
-        };
-      }),
-    ),
+    indexNames: Array(3).map(randomName),
+    grants: [
+      {
+        user: randomRole(),
+        privilege: randomTablePrivilege(),
+      },
+    ],
     statsLastUpdated: moment("0001-01-01T00:00:00Z"),
   },
   showNodeRegionsSection: true,

pkg/ui/workspaces/cluster-ui/src/databaseTablePage/databaseTablePage.tsx

@@ -12,7 +12,6 @@ import React from "react";
 import { Col, Row, Tabs } from "antd";
 import { RouteComponentProps } from "react-router-dom";
 import classNames from "classnames/bind";
-import _ from "lodash";
 import { Tooltip } from "antd";
 import { Heading } from "@cockroachlabs/ui-components";
 
@@ -421,7 +420,7 @@ export class DatabaseTablePage extends React.Component<
                     />
                     <SummaryCardItem
                       label="Indexes"
-                      value={_.join(this.props.details.indexNames, ", ")}
+                      value={this.props.details.indexNames.join(", ")}
                       className={cx("database-table-page__indexes--value")}
                     />
                   </SummaryCard>

pkg/ui/workspaces/cluster-ui/src/databasesPage/databasesPage.stories.tsx

@@ -76,7 +76,7 @@ const withData: DatabasesPageProps = {
     ascending: false,
     columnTitle: "name",
   },
-  databases: _.map(Array(42), _item => {
+  databases: Array(42).map(() => {
     return {
       loading: false,
       loaded: true,

pkg/ui/workspaces/cluster-ui/src/databasesPage/databasesPage.tsx

@@ -12,7 +12,6 @@ import React from "react";
 import { Link, RouteComponentProps } from "react-router-dom";
 import { Tooltip } from "antd";
 import classNames from "classnames/bind";
-import _ from "lodash";
 
 import { Anchor } from "src/anchor";
 import { StackIcon } from "src/icon/stackIcon";
@@ -175,12 +174,12 @@ export class DatabasesPage extends React.Component<
       return this.props.refreshDatabases();
     }
 
-    _.forEach(this.props.databases, database => {
+    this.props.databases.forEach(database => {
       if (!database.loaded && !database.loading) {
         return this.props.refreshDatabaseDetails(database.name);
       }
 
-      _.forEach(database.missingTables, table => {
+      database.missingTables.forEach(table => {
         if (!table.loading) {
           return this.props.refreshTableStats(database.name, table.name);
         }