Merge #31195 #31218

31195: sql: fixed flakey tests for cancelled/errored schema changes r=eriktrinh a=eriktrinh

TestBackfillError had the async schema changer execute immediately,
before the synchronous one could see the error.

TestCancelSchemaChange had the GC TTL set to 0 during the SELECT.

Fixes #31175, #31177

Release note: None

31218: ui: surface debug pages r=couchand a=couchand

Add a (somewhat muted) link in the sidebar for the debug pages, an informational note about the provisional status of the pages, and a snazzy top panel for the ones we want to call attention to.

<img width="1351" alt="screen shot 2018-10-10 at 4 38 29 pm" src="https://user-images.githubusercontent.com/793969/46764652-062ebb80-ccab-11e8-990a-4123a1e3f0a5.png">

Closes #20690

Co-authored-by: Erik Trinh <erik@cockroachlabs.com>
Co-authored-by: Andrew Couch <andrew@cockroachlabs.com>
Co-authored-by: Pete Vilter <vilterp@cockroachlabs.com>

pkg/sql/schema_changer_test.go

@@ -439,20 +439,29 @@ CREATE INDEX foo ON t.test (v)
 	}
 }
 
-// checkTableKeyCount returns the number of KVs in the DB, the multiple should be the
-// number of columns.
-func checkTableKeyCount(ctx context.Context, kvDB *client.DB, multiple int, maxValue int) error {
+func getTableKeyCount(ctx context.Context, kvDB *client.DB) (int, error) {
 	tableDesc := sqlbase.GetTableDescriptor(kvDB, "t", "test")
 	tablePrefix := roachpb.Key(keys.MakeTablePrefix(uint32(tableDesc.ID)))
 	tableEnd := tablePrefix.PrefixEnd()
-	if kvs, err := kvDB.Scan(ctx, tablePrefix, tableEnd, 0); err != nil {
+	kvs, err := kvDB.Scan(ctx, tablePrefix, tableEnd, 0)
+	return len(kvs), err
+}
+
+func checkTableKeyCountExact(ctx context.Context, kvDB *client.DB, e int) error {
+	if count, err := getTableKeyCount(ctx, kvDB); err != nil {
 		return err
-	} else if e := multiple * (maxValue + 1); len(kvs) != e {
-		return errors.Errorf("expected %d key value pairs, but got %d", e, len(kvs))
+	} else if count != e {
+		return errors.Errorf("expected %d key value pairs, but got %d", e, count)
 	}
 	return nil
 }
 
+// checkTableKeyCount returns the number of KVs in the DB, the multiple should be the
+// number of columns.
+func checkTableKeyCount(ctx context.Context, kvDB *client.DB, multiple int, maxValue int) error {
+	return checkTableKeyCountExact(ctx, kvDB, multiple*(maxValue+1))
+}
+
 // Run a particular schema change and run some OLTP operations in parallel, as
 // soon as the schema change starts executing its backfill.
 func runSchemaChangeWithOperations(
@@ -871,8 +880,8 @@ func TestBackfillErrors(t *testing.T) {
 
 	params.Knobs = base.TestingKnobs{
 		SQLSchemaChanger: &sql.SchemaChangerTestingKnobs{
-			AsyncExecQuickly:  true,
-			BackfillChunkSize: chunkSize,
+			AsyncExecNotification: asyncSchemaChangerDisabled,
+			BackfillChunkSize:     chunkSize,
 		},
 	}
 
@@ -893,10 +902,6 @@ CREATE TABLE t.test (k INT PRIMARY KEY, v INT);
 	}
 
 	tableDesc := sqlbase.GetTableDescriptor(kvDB, "t", "test")
-	// Add a zone config for the table.
-	if _, err := addImmediateGCZoneConfig(sqlDB, tableDesc.ID); err != nil {
-		t.Fatal(err)
-	}
 
 	// Bulk insert.
 	if err := bulkInsertIntoTable(sqlDB, maxValue); err != nil {
@@ -937,17 +942,23 @@ CREATE UNIQUE INDEX vidx ON t.test (v);
 		t.Fatalf("got err=%s", err)
 	}
 
-	testutils.SucceedsSoon(t, func() error {
-		return checkTableKeyCount(ctx, kvDB, 1, maxValue)
-	})
+	// Index backfill errors at a non-deterministic chunk and the garbage
+	// keys remain because the async schema changer for the rollback stays
+	// disabled in order to assert the next errors. Therefore we do not check
+	// the keycount from this operation and just check that the next failed
+	// operations do not add more.
+	keyCount, err := getTableKeyCount(ctx, kvDB)
+	if err != nil {
+		t.Fatal(err)
+	}
 
 	if _, err := sqlDB.Exec(`
 	   ALTER TABLE t.test ADD COLUMN p DECIMAL NOT NULL DEFAULT (DECIMAL '1-3');
 	   `); !testutils.IsError(err, `could not parse "1-3" as type decimal`) {
 		t.Fatalf("got err=%s", err)
 	}
 
-	if err := checkTableKeyCount(ctx, kvDB, 1, maxValue); err != nil {
+	if err := checkTableKeyCountExact(ctx, kvDB, keyCount); err != nil {
 		t.Fatal(err)
 	}
 
@@ -957,7 +968,7 @@ CREATE UNIQUE INDEX vidx ON t.test (v);
 		t.Fatalf("got err=%s", err)
 	}
 
-	if err := checkTableKeyCount(ctx, kvDB, 1, maxValue); err != nil {
+	if err := checkTableKeyCountExact(ctx, kvDB, keyCount); err != nil {
 		t.Fatal(err)
 	}
 }
@@ -1795,6 +1806,9 @@ CREATE TABLE t.test (k INT PRIMARY KEY, v INT);
 		if len(tableDesc.Mutations) > 0 {
 			return errors.Errorf("%d mutations remaining", len(tableDesc.Mutations))
 		}
+		if len(tableDesc.GCMutations) > 0 {
+			return errors.Errorf("%d gc mutations remaining", len(tableDesc.GCMutations))
+		}
 
 		// Verify that t.public.test has the expected data. Read the table data while
 		// ensuring that the correct table lease is in use.
@@ -3506,14 +3520,6 @@ func TestCancelSchemaChange(t *testing.T) {
 		idx++
 	}
 
-	atomic.StoreUint32(&enableAsyncSchemaChanges, 1)
-	if _, err := addImmediateGCZoneConfig(db, tableDesc.ID); err != nil {
-		t.Fatal(err)
-	}
-	testutils.SucceedsSoon(t, func() error {
-		return checkTableKeyCount(ctx, kvDB, 3, maxValue)
-	})
-
 	// Verify that the index foo over v is consistent, and that column x has
 	// been backfilled properly.
 	rows, err := db.Query(`SELECT v, x from t.test@foo`)
@@ -3544,6 +3550,15 @@ func TestCancelSchemaChange(t *testing.T) {
 	if eCount != count {
 		t.Fatalf("read the wrong number of rows: e = %d, v = %d", eCount, count)
 	}
+
+	// Verify that the data from the canceled CREATE INDEX is cleaned up.
+	atomic.StoreUint32(&enableAsyncSchemaChanges, 1)
+	if _, err := addImmediateGCZoneConfig(db, tableDesc.ID); err != nil {
+		t.Fatal(err)
+	}
+	testutils.SucceedsSoon(t, func() error {
+		return checkTableKeyCount(ctx, kvDB, 3, maxValue)
+	})
 }
 
 // This test checks that when a transaction containing schema changes

pkg/ui/ccl/src/views/shared/components/licenseType/index.tsx

@@ -15,7 +15,9 @@ class CCLLicenseType extends React.Component<{}, {}> {
   render() {
     return (
       <div>
-        <h3>License type: CCL</h3>
+        <span className="license-type__label">License type:</span>
+        {" "}
+        <span className="license-type__license">CCL</span>
       </div>
     );
   }

pkg/ui/src/views/app/components/layoutSidebar/index.tsx

@@ -31,6 +31,7 @@ import databasesIcon from "!!raw-loader!assets/sidebarIcons/databases.svg";
 import jobsIcon from "!!raw-loader!assets/sidebarIcons/jobs.svg";
 import statementsIcon from "!!raw-loader!assets/sidebarIcons/statements.svg";
 import unlockedIcon from "!!raw-loader!assets/unlocked.svg";
+import gearIcon from "!!raw-loader!assets/sidebarIcons/gear.svg";
 
 interface IconLinkProps {
   icon: string;
@@ -60,12 +61,10 @@ class IconLink extends React.Component<IconLinkProps, {}> {
     const router = this.context.router;
     const linkRoutes = [to].concat(activeFor);
     const isActive = _.some(linkRoutes, (route) => router.isActive(route, false));
+    const linkClassName = classNames("icon-link", { active: isActive });
     return (
       <li className={className} >
-        <Link
-          to={to}
-          className={classNames({ active: isActive })}
-        >
+        <Link to={to} className={linkClassName}>
           <div className="image-container"
                dangerouslySetInnerHTML={trustIcon(icon)}/>
           <div>{title}</div>
@@ -102,13 +101,17 @@ function LoginIndicator({ loginState, handleLogout }: LoginIndicatorProps) {
 
   return (
     <li className="login-indicator">
-        <div
-          className="login-indicator__initial"
-          title={`Signed in as ${user}`}
-        >
-          { user[0] }
+      <Link to={LOGOUT_PAGE} onClick={handleLogout}>
+        <div>
+          <div
+            className="login-indicator__initial"
+            title={`Signed in as ${user}`}
+          >
+            {user[0]}
+          </div>
+          Log Out
         </div>
-        <Link to={LOGOUT_PAGE} onClick={handleLogout}>Log Out</Link>
+      </Link>
     </li>
   );
 }
@@ -131,6 +134,10 @@ const LoginIndicatorConnected = connect(
  * the page which is currently active will be highlighted.
  */
 export default class Sidebar extends React.Component {
+  static contextTypes = {
+    router: PropTypes.object,
+  };
+
   render() {
     return (
       <nav className="navigation-bar">
@@ -142,6 +149,7 @@ export default class Sidebar extends React.Component {
           <IconLink to="/jobs" icon={jobsIcon} title="Jobs" />
         </ul>
         <ul className="navigation-bar__list navigation-bar__list--bottom">
+          <IconLink to="/debug" icon={gearIcon} className="normal debug-pages-link" activeFor={["/reports", "/data-distribution", "/raft"]} />
           <LoginIndicatorConnected />
           <IconLink to="/debug" icon={cockroachIcon} className="cockroach" />
         </ul>

pkg/ui/src/views/login/loginPage.styl

@@ -122,12 +122,6 @@
     color #3a7de1
 
 .login-note-box
-  margin-top 20px
-  padding 10px
-  padding-left 25px
-  background-color #ecf7ff
-  border-left 5px solid #357eea
-
   &__sql-command
     border 1px solid lightgrey
     background #f6f6f6
@@ -145,14 +139,6 @@
     .sql-string
       color #3c80e1
 
-  &__heading
-    font-weight bold
-    color #3e7bde
-
-  &__blurb
-    margin-top 10px
-    color $body-color
-
 .login-docs-link
   display block
   margin-top 10px

pkg/ui/src/views/login/loginPage.tsx

@@ -23,6 +23,7 @@ import { AdminUIState } from "src/redux/state";
 import { getDataFromServer } from "src/util/dataFromServer";
 import * as docsURL from "src/util/docs";
 import { trustIcon } from "src/util/trust";
+import InfoBox from "src/views/shared/components/infoBox";
 
 import logo from "assets/crdb.png";
 import docsIcon from "!!raw-loader!assets/docs.svg";
@@ -116,8 +117,8 @@ class LoginPage extends React.Component<LoginPageProps & WithRouterProps, LoginP
                 on secure clusters.
               </p>
             </div>
-            <div className="login-note-box">
-              <div className="login-note-box__heading">Note:</div>
+            <InfoBox>
+              <h4 className="login-note-box__heading">Note:</h4>
               <p className="login-note-box__blurb">
                 Create a user with this SQL command:
               </p>
@@ -135,7 +136,7 @@ class LoginPage extends React.Component<LoginPageProps & WithRouterProps, LoginP
                   <span className="login-docs-link__text">Read more about configuring login</span>
                 </a>
               </p>
-            </div>
+            </InfoBox>
           </section>
           <section className="section login-page__form">
             <p className="version">

pkg/ui/src/views/reports/containers/debug/debug.styl

@@ -0,0 +1,32 @@
+// Copyright 2018 The Cockroach Authors.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
+// implied. See the License for the specific language governing
+// permissions and limitations under the License.
+
+@require "~styl/base/palette.styl"
+
+.debug-url
+  margin-right 200px
+
+  div
+    background-color $background-color
+    border-radius 3px
+    padding 8px 20px 6px 10px
+    min-width 200px
+
+.debug-header
+  position relative
+
+.debug-header__license-type
+  position absolute
+  top 0
+  right 0