Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
87313: ui: add optional white background to PageConfig r=absterr08 a=absterr08

Previously, the PageConfig component had a fixed grey
background to match the page background, since it is a sticky
component that needs a background color to cover content that
scrolls beneath it. However, the background color is changing
to white in CockroachCloud, so the background color needs to
be able to match the new background. Until we apply the layout
changes to `db-console`, this change allows us to specify a
different background color in CockroachCloud.

This change uses CockroachCloudContext to determine whether or
not the PageConfig background color should be white.

**Without white background**
<img width="1783" alt="Screen Shot 2022-08-31 at 12 11 39 PM" src="https://user-images.githubusercontent.com/16804318/187727284-9535aa46-e91e-4e9c-974f-7dec62f9b4e3.png">

**With white background**
<img width="1788" alt="Screen Shot 2022-08-31 at 12 11 56 PM" src="https://user-images.githubusercontent.com/16804318/187727321-034b83b2-6229-4de8-8860-b1a94be6dd9a.png">

Release note: None

Release justification: ui change for cluster-ui components used
in cockroach cloud

87392: roachtests/awsdms: remove unrequired session setting r=rafiss a=otan

Now that schema changer removes the rowid column, we can no longer test
with the expect and ignore hidden columns setting.

Release note: None

Release justification: test only change

87471: colbuilder: fix invalid sharing of a memory account by the columnarizer r=yuzefovich a=yuzefovich

We recently merged a couple of changes to the memory accounting in the columnarizer which relied on its memory account not being shared with any other component. This, however, wasn't true since we passed the "streaming" memory account that many components can use. This commit fixes that oversight by always creating a separate account.

The previous behavior could result in the columnarizer clearing the account when another user of that account attempts to shrink it (in production builds this would only trigger a sentry report with no panic). For example, this situation could occur when we plan a vectorized ordered aggregator that uses a default aggregate function and then we handle the rendering with a wrapped row-by-row processor which requires a creation of the columnarizer.

Fixes: #87016.

Release justification: bug fix.

Release note: None (no release with this bug)

Co-authored-by: Abby Hersh <[email protected]>
Co-authored-by: Oliver Tan <[email protected]>
Co-authored-by: Yahor Yuzefovich <[email protected]>
  • Loading branch information
4 people committed Sep 7, 2022
4 parents cfeb4cc + d9632c0 + acd07db + ff6bc94 commit 7a3edae
Show file tree
Hide file tree
Showing 4 changed files with 12 additions and 6 deletions.
1 change: 0 additions & 1 deletion pkg/cmd/roachtest/tests/awsdms.go
Original file line number Diff line number Diff line change
Expand Up @@ -281,7 +281,6 @@ func setupCockroachDBCluster(ctx context.Context, t test.Test, c cluster.Cluster
for _, stmt := range []string{
fmt.Sprintf("CREATE USER %s", awsdmsCRDBUser),
fmt.Sprintf("GRANT admin TO %s", awsdmsCRDBUser),
fmt.Sprintf("ALTER USER %s SET expect_and_ignore_not_visible_columns_in_copy = true", awsdmsCRDBUser),
} {
if _, err := db.Exec(stmt); err != nil {
return err
Expand Down
4 changes: 1 addition & 3 deletions pkg/sql/colexec/colbuilder/execplan.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,6 @@ func wrapRowSources(
flowCtx *execinfra.FlowCtx,
inputs []colexecargs.OpWithMetaInfo,
inputTypes [][]*types.T,
streamingMemAcc *mon.BoundAccount,
monitorRegistry *colexecargs.MonitorRegistry,
processorID int32,
newToWrap func([]execinfra.RowSource) (execinfra.RowSource, error),
Expand Down Expand Up @@ -121,7 +120,7 @@ func wrapRowSources(
if !isProcessor {
return nil, nil, errors.AssertionFailedf("unexpectedly %T is not an execinfra.Processor", toWrap)
}
batchAllocator := colmem.NewAllocator(ctx, streamingMemAcc, factory)
batchAllocator := colmem.NewAllocator(ctx, monitorRegistry.NewStreamingMemAccount(flowCtx), factory)
metadataAllocator := colmem.NewAllocator(ctx, monitorRegistry.NewStreamingMemAccount(flowCtx), factory)
var c *colexec.Columnarizer
if proc.MustBeStreaming() {
Expand Down Expand Up @@ -548,7 +547,6 @@ func (r opResult) createAndWrapRowSource(
flowCtx,
inputs,
inputTypes,
args.StreamingMemAccount,
args.MonitorRegistry,
processorID,
func(inputs []execinfra.RowSource) (execinfra.RowSource, error) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,10 @@
z-index: 2;
background-color: $colors--background;

&__white-background {
background-color: $colors--white
}

&__list {
display: flex;
justify-content: flex-start;
Expand Down
9 changes: 7 additions & 2 deletions pkg/ui/workspaces/cluster-ui/src/pageConfig/pageConfig.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,10 @@
// by the Apache License, Version 2.0, included in the file
// licenses/APL.txt.

import React from "react";
import React, { useContext } from "react";
import classnames from "classnames/bind";
import styles from "./pageConfig.module.scss";
import { CockroachCloudContext } from "../contexts";

export interface PageConfigProps {
layout?: "list" | "spread";
Expand All @@ -20,13 +21,17 @@ export interface PageConfigProps {
const cx = classnames.bind(styles);

export function PageConfig(props: PageConfigProps): React.ReactElement {
const isCockroachCloud = useContext(CockroachCloudContext);

const classes = cx({
"page-config__list": props.layout !== "spread",
"page-config__spread": props.layout === "spread",
});

return (
<div className={cx("page-config")}>
<div
className={cx("page-config", { "page-config__white-background": isCockroachCloud })}
>
<ul className={classes}>{props.children}</ul>
</div>
);
Expand Down

0 comments on commit 7a3edae

Please sign in to comment.