Merge #104259 #106224

104259: ui: fix storybook for db-console and make it run with bazel r=koorosh a=koorosh

This change includes several changes that enables better experience for developers to build visual components with Storybook, now it is possible to run Storybook with `dev` ie:
```
dev ui storybook --project cluster-ui --port 9009
or
dev ui storybook # db-console project is default one
```

- fixed storybook configuration for Db Console project. It required to install `core-js` package to resolve following issues: https://github.com/storybookjs/storybook/blob/main/MIGRATION.md#core-js-dependency-errors
- updated fixtures for cluster UI storybook that wasn't adjusted to changes in components they rely on.
- Added new `dev ui storybook` subcommand that allows run storybooks for Db Console or Cluster UI projects

Release note: None

Epic: None

106224: backfill: fix possible race in tests r=yuzefovich a=yuzefovich

This commit addresses a race when `Push`ing into `execinfra.RowReceiver` which can happen in tests. In particular, when
`PushesProgressEveryChunk` testing knob is set, there is a concurrency between the main waiter goroutine and worker goroutine, so we now add a mutex around that `Push`. In theory, `RowReceiver.Push` claims to be thread-safe, so it shouldn't be necessary, but two implementations (`DistSQLReceiver` and `copyingRowReceiver`) are actually not thread-safe, and fixing that is left as a TODO.

Fixes: #106161.

Release note: None

Co-authored-by: Andrii Vorobiov <[email protected]>
Co-authored-by: Yahor Yuzefovich <[email protected]>

.aspect/rules/external_repository_action_cache/npm_translate_lock_LTY0NDkzMjUwNQ==

@@ -1,6 +1,6 @@
 # Input hashes for repository rule npm_translate_lock(name = "npm_db_console", pnpm_lock = "//pkg/ui/workspaces/db-console:pnpm-lock.yaml").
 # This file should be checked into version control along with the pnpm-lock.yaml file.
 pkg/ui/.npmrc.pnpm=1714720514
-pkg/ui/workspaces/db-console/pnpm-lock.yaml=307741782
-pkg/ui/workspaces/db-console/yarn.lock=1296929611
-pkg/ui/workspaces/db-console/package.json=-1415963323
+pkg/ui/workspaces/db-console/pnpm-lock.yaml=1591164581
+pkg/ui/workspaces/db-console/yarn.lock=861579387
+pkg/ui/workspaces/db-console/package.json=463935330

pkg/cmd/dev/ui.go

@@ -42,6 +42,7 @@ func makeUICmd(d *dev) *cobra.Command {
 	uiCmd.AddCommand(makeUIWatchCmd(d))
 	uiCmd.AddCommand(makeUIE2eCmd(d))
 	uiCmd.AddCommand(makeMirrorDepsCmd(d))
+	uiCmd.AddCommand(makeUIStorybookCmd(d))
 
 	return uiCmd
 }
@@ -225,6 +226,118 @@ Replaces 'make ui-watch'.`,
 	return watchCmd
 }
 
+// makeUIStorybookCmd initializes the 'ui storybook' subcommand, which runs
+// storybook for either db-console or cluster-ui projects.
+func makeUIStorybookCmd(d *dev) *cobra.Command {
+	const (
+		// projectFlag indicates whether storybook should be run for db-console or
+		// cluster-ui projects
+		projectFlag = "project"
+		// portFlag defines the port to run Storybook
+		portFlag = "port"
+	)
+
+	storybookCmd := &cobra.Command{
+		Use:   "storybook",
+		Short: "Runs Storybook in watch mode",
+		Long:  ``,
+		Args:  cobra.MinimumNArgs(0),
+		RunE: func(cmd *cobra.Command, commandLine []string) error {
+			// Create a context that cancels when OS signals come in.
+			ctx, stop := signal.NotifyContext(d.cli.Context(), os.Interrupt, os.Kill)
+			defer stop()
+
+			// Fetch all node dependencies (through Bazel) to ensure things are up-to-date
+			err := d.exec.CommandContextInheritingStdStreams(
+				ctx,
+				"bazel",
+				"fetch",
+				"//pkg/ui/workspaces/cluster-ui:cluster-ui",
+				"//pkg/ui/workspaces/db-console:db-console-ccl",
+			)
+			if err != nil {
+				log.Fatalf("failed to fetch node dependencies: %v", err)
+			}
+
+			// Build prerequisites for Storybook. It runs Db Console with CCL license.
+			args := []string{
+				"build",
+				"//pkg/ui/workspaces/cluster-ui:cluster-ui",
+				"//pkg/ui/workspaces/db-console/ccl/src/js:crdb-protobuf-client-ccl",
+			}
+
+			logCommand("bazel", args...)
+			err = d.exec.CommandContextInheritingStdStreams(ctx, "bazel", args...)
+			if err != nil {
+				log.Fatalf("failed to build UI watch prerequisites: %v", err)
+				return err
+			}
+
+			if err := arrangeFilesForWatchers(d /* ossOnly */, false); err != nil {
+				log.Fatalf("failed to arrange files for watchers: %v", err)
+				return err
+			}
+
+			dirs, err := getUIDirs(d)
+			if err != nil {
+				log.Fatalf("unable to find cluster-ui and db-console directories: %v", err)
+				return err
+			}
+
+			portNumber, err := cmd.Flags().GetInt16(portFlag)
+			if err != nil {
+				log.Fatalf("unexpected error: %v", err)
+				return err
+			}
+			port := fmt.Sprint(portNumber)
+
+			project, err := cmd.Flags().GetString(projectFlag)
+			if err != nil {
+				log.Fatalf("unexpected error: %v", err)
+				return err
+			}
+
+			projectToPath := map[string]string{
+				"db-console": dirs.dbConsole,
+				"cluster-ui": dirs.clusterUI,
+			}
+
+			cwd, ok := projectToPath[project]
+			if !ok {
+				err = fmt.Errorf("unexpected project name (%s) provided with --project flag", project)
+				log.Fatalf("%v", err)
+				return err
+			}
+
+			nbExec := d.exec.AsNonBlocking()
+
+			args = []string{
+				"--silent",
+				"--cwd", cwd,
+				"start-storybook",
+				"-p", port,
+			}
+
+			err = nbExec.CommandContextInheritingStdStreams(ctx, "yarn", args...)
+			if err != nil {
+				log.Fatalf("Unable to run Storybook for %s : %v", project, err)
+				return err
+			}
+
+			// Wait for OS signals to cancel if we're not in test-mode.
+			if !d.exec.IsDryrun() {
+				<-ctx.Done()
+			}
+			return nil
+		},
+	}
+
+	storybookCmd.Flags().Int16P(portFlag, "p", 6006, "port to run Storybook")
+	storybookCmd.Flags().String(projectFlag, "db-console", "db-console, cluster-ui")
+
+	return storybookCmd
+}
+
 func makeUILintCmd(d *dev) *cobra.Command {
 	const (
 		verboseFlag = "verbose"

pkg/sql/backfill/mvcc_index_merger.go

@@ -136,11 +136,18 @@ func (ibm *IndexBackfillMerger) Run(ctx context.Context, output execinfra.RowRec
 		return prog
 	}
 
+	var outputMu syncutil.Mutex
 	pushProgress := func() {
 		p := getStoredProgressForPush()
 		if p.CompletedSpans != nil {
 			log.VEventf(ctx, 2, "sending coordinator completed spans: %+v", p.CompletedSpans)
 		}
+		// Even though the contract of execinfra.RowReceiver says that Push is
+		// thread-safe, in reality it's not always the case, so we protect it
+		// with a mutex. At the time of writing, the only source of concurrency
+		// for Push()ing is present due to testing knobs though.
+		outputMu.Lock()
+		defer outputMu.Unlock()
 		output.Push(nil, &execinfrapb.ProducerMetadata{BulkProcessorProgress: &p})
 	}
 

pkg/sql/execinfra/base.go

@@ -92,6 +92,9 @@ type RowReceiver interface {
 	// and they might not all be aware of the last status returned).
 	//
 	// Implementations of Push() must be thread-safe.
+	// TODO(yuzefovich): some implementations (DistSQLReceiver and
+	// copyingRowReceiver) are not actually thread-safe. Figure out whether we
+	// want to fix them or to update the contract.
 	Push(row rowenc.EncDatumRow, meta *execinfrapb.ProducerMetadata) ConsumerStatus
 }
 

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

@@ -22,7 +22,8 @@ import {
 import statementsPagePropsFixture from "src/statementsPage/statementsPage.fixture";
 import Long from "long";
 
-const { statements } = statementsPagePropsFixture;
+const statements =
+  statementsPagePropsFixture.statementsResponse.data.statements;
 
 const withinColumn =
   (width = "150px"): DecoratorFn =>

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

@@ -24,9 +24,7 @@ storiesOf("StatementsPage", module)
     <div style={{ backgroundColor: "#F5F7FA" }}>{storyFn()}</div>
   ))
   .add("with data", () => <StatementsPage {...statementsPagePropsFixture} />)
-  .add("without data", () => (
-    <StatementsPage {...statementsPagePropsFixture} statements={[]} />
-  ))
+  .add("without data", () => <StatementsPage {...statementsPagePropsFixture} />)
   .add("with empty search result", () => {
     const props = cloneDeep(statementsPagePropsFixture);
     const { history } = props;
@@ -37,19 +35,13 @@ storiesOf("StatementsPage", module)
       <StatementsPage
         {...props}
         {...statementsPagePropsFixture}
-        statements={[]}
         history={history}
       />
     );
   })
   .add("with error", () => {
-    return (
-      <StatementsPage
-        {...statementsPagePropsWithRequestError}
-        statements={[]}
-      />
-    );
+    return <StatementsPage {...statementsPagePropsWithRequestError} />;
   })
   .add("with loading state", () => {
-    return <StatementsPage {...statementsPagePropsFixture} statements={null} />;
+    return <StatementsPage {...statementsPagePropsFixture} />;
   });

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

@@ -17,17 +17,19 @@ import {
 } from "./statementsTable";
 import statementsPagePropsFixture from "src/statementsPage/statementsPage.fixture";
 import { calculateTotalWorkload } from "src/util";
+import { convertRawStmtsToAggregateStatistics } from "../sqlActivity/util";
 
-const { statements } = statementsPagePropsFixture;
+const statements =
+  statementsPagePropsFixture.statementsResponse.data.statements;
 
 storiesOf("StatementsSortedTable", module)
   .addDecorator(storyFn => <MemoryRouter>{storyFn()}</MemoryRouter>)
   .add("with data", () => (
     <StatementsSortedTable
       className="statements-table"
-      data={statements}
+      data={convertRawStmtsToAggregateStatistics(statements)}
       columns={makeStatementsColumns(
-        statements,
+        convertRawStmtsToAggregateStatistics(statements),
         ["$ internal"],
         calculateTotalWorkload(statements),
         "statement",
@@ -49,9 +51,9 @@ storiesOf("StatementsSortedTable", module)
   .add("with data and VIEWACTIVITYREDACTED role", () => (
     <StatementsSortedTable
       className="statements-table"
-      data={statements}
+      data={convertRawStmtsToAggregateStatistics(statements)}
       columns={makeStatementsColumns(
-        statements,
+        convertRawStmtsToAggregateStatistics(statements),
         ["$ internal"],
         calculateTotalWorkload(statements),
         "statement",

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

@@ -13,11 +13,9 @@ import { storiesOf } from "@storybook/react";
 import { MemoryRouter } from "react-router-dom";
 import { noop } from "lodash";
 import {
-  error,
   nodeRegions,
   routeProps,
   timeScale,
-  transaction,
   transactionDetailsData,
   transactionFingerprintId,
 } from "./transactionDetails.fixture";
@@ -37,9 +35,6 @@ storiesOf("Transactions Details", module)
       {...routeProps}
       timeScale={timeScale}
       transactionFingerprintId={transactionFingerprintId.toString()}
-      transaction={transaction}
-      isLoading={false}
-      statements={transactionDetailsData.statements}
       nodeRegions={nodeRegions}
       isTenant={false}
       hasViewActivityRedactedRole={false}
@@ -48,21 +43,23 @@ storiesOf("Transactions Details", module)
       refreshUserSQLRoles={noop}
       onTimeScaleChange={noop}
       refreshNodes={noop}
-      lastUpdated={moment("0001-01-01T00:00:00Z")}
       refreshTransactionInsights={noop}
       limit={100}
       reqSortSetting={StatsSortOptions.SERVICE_LAT}
-      isDataValid={true}
+      txnStatsResp={{
+        lastUpdated: moment(),
+        error: null,
+        inFlight: false,
+        valid: true,
+        data: transactionDetailsData,
+      }}
     />
   ))
   .add("with loading indicator", () => (
     <TransactionDetails
       {...routeProps}
       timeScale={timeScale}
       transactionFingerprintId={transactionFingerprintId.toString()}
-      transaction={null}
-      isLoading={true}
-      statements={undefined}
       nodeRegions={nodeRegions}
       isTenant={false}
       hasViewActivityRedactedRole={false}
@@ -71,35 +68,41 @@ storiesOf("Transactions Details", module)
       refreshUserSQLRoles={noop}
       onTimeScaleChange={noop}
       refreshNodes={noop}
-      lastUpdated={moment("0001-01-01T00:00:00Z")}
       refreshTransactionInsights={noop}
       limit={100}
       reqSortSetting={StatsSortOptions.SERVICE_LAT}
-      isDataValid={true}
+      txnStatsResp={{
+        lastUpdated: moment(),
+        error: null,
+        inFlight: false,
+        valid: true,
+        data: transactionDetailsData,
+      }}
     />
   ))
   .add("with error alert", () => (
     <TransactionDetails
       {...routeProps}
       timeScale={timeScale}
       transactionFingerprintId={undefined}
-      transaction={undefined}
-      isLoading={false}
-      statements={undefined}
       nodeRegions={nodeRegions}
-      error={error}
       isTenant={false}
       hasViewActivityRedactedRole={false}
       transactionInsights={undefined}
       refreshData={noop}
       refreshUserSQLRoles={noop}
       onTimeScaleChange={noop}
       refreshNodes={noop}
-      lastUpdated={moment("0001-01-01T00:00:00Z")}
       refreshTransactionInsights={noop}
       limit={100}
       reqSortSetting={StatsSortOptions.SERVICE_LAT}
-      isDataValid={true}
+      txnStatsResp={{
+        lastUpdated: moment(),
+        error: null,
+        inFlight: false,
+        valid: true,
+        data: transactionDetailsData,
+      }}
     />
   ))
   .add("No data for this time frame; no cached transaction text", () => {
@@ -108,9 +111,6 @@ storiesOf("Transactions Details", module)
         {...routeProps}
         timeScale={timeScale}
         transactionFingerprintId={transactionFingerprintId.toString()}
-        transaction={undefined}
-        isLoading={false}
-        statements={transactionDetailsData.statements}
         nodeRegions={nodeRegions}
         isTenant={false}
         hasViewActivityRedactedRole={false}
@@ -119,11 +119,16 @@ storiesOf("Transactions Details", module)
         refreshUserSQLRoles={noop}
         onTimeScaleChange={noop}
         refreshNodes={noop}
-        lastUpdated={moment("0001-01-01T00:00:00Z")}
         refreshTransactionInsights={noop}
         limit={100}
         reqSortSetting={StatsSortOptions.SERVICE_LAT}
-        isDataValid={true}
+        txnStatsResp={{
+          lastUpdated: moment(),
+          error: null,
+          inFlight: false,
+          valid: true,
+          data: transactionDetailsData,
+        }}
       />
     );
   });

pkg/ui/workspaces/db-console/.storybook/webpack.config.js

@@ -8,7 +8,8 @@
 // by the Apache License, Version 2.0, included in the file
 // licenses/APL.txt.
 
-const custom = require("../webpack.app.js");
+const custom = require("../webpack.config");
+const path = require("path");
 
 const appConfig = custom({dist: "ccl"}, {mode: "development"});
 
@@ -17,8 +18,13 @@ module.exports = async ({ config, mode }) => {
     ...config,
     resolve: {
       ...config.resolve,
-      modules: appConfig.resolve.modules,
+      modules: [
+        path.resolve(__dirname, "..", "ccl"),
+        path.resolve(__dirname, ".."),
+        "node_modules",
+      ],
       extensions: appConfig.resolve.extensions,
+      alias: appConfig.resolve.alias,
     },
     module: {
       rules: [