resolved conflict

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
vitessio · Jun 9, 2024 · f3a4a97 · f3a4a97
2 parents 78234e0 + 87ecae7
commit f3a4a97
Show file tree

Hide file tree

Showing 7 changed files with 257 additions and 54 deletions.
diff --git a/changelog/20.0/20.0.0/summary.md b/changelog/20.0/20.0.0/summary.md
@@ -183,7 +183,7 @@ This makes reparenting in Vitess resilient to client errors, and prevents Planne
 
 In order to preserve the old behaviour, the users can set the flag back to `0 seconds` causing open transactions to never be shutdown, but in that case, they run the risk of PlannedReparentShard calls timing out.
 
-#### <a id="unmanaged-tablet"/> New `unmanaged` Flag and `disable_active_reparents` deprecation
+#### <a id="unmanaged-flag"/> New `unmanaged` Flag and `disable_active_reparents` deprecation
 
 New flag `--unmanaged` has been introduced in this release to make it easier to flag unmanaged tablets. It also runs validations to make sure the unmanaged tablets are configured properly. `--disable_active_reparents` flag has been deprecated for `vttablet`, `vtcombo` and `vttestserver` binaries and will be removed in future releases. Specifying the `--unmanaged` flag will also block replication commands and replication repairs.
 
@@ -338,7 +338,7 @@ VTTablet exposes two new counter stats:
  * `QueryCacheHits`: Query engine query cache hits
  * `QueryCacheMisses`: Query engine query cache misses
 
-### <a id="#vttablet-query-text-characters-processed"/>VTTablet Query Text Characters Processed
+### <a id="vttablet-query-text-characters-processed"/>VTTablet Query Text Characters Processed
 
 VTGate and VTTablet expose a new counter stat `QueryTextCharactersProcessed` to reflect the number of query text characters processed.
 

diff --git a/go/test/endtoend/onlineddl/scheduler/onlineddl_scheduler_test.go b/go/test/endtoend/onlineddl/scheduler/onlineddl_scheduler_test.go
@@ -198,7 +198,7 @@ func waitForReadyToComplete(t *testing.T, uuid string, expected bool) {
 		case <-ticker.C:
 		case <-ctx.Done():
 		}
-		require.NoError(t, ctx.Err())
+		require.NoError(t, ctx.Err(), "waiting for ready_to_complete=%t for %v", expected, uuid)
 	}
 }
 
@@ -422,6 +422,14 @@ func testScheduler(t *testing.T) {
 			assert.GreaterOrEqual(t, endTime2, endTime1)
 		})
 	}
+	testTableCompletionAndStartTimes := func(t *testing.T, uuid1, uuid2 string) {
+		// expect uuid1 to complete before uuid2
+		t.Run("Compare t1, t2 completion times", func(t *testing.T) {
+			endTime1 := testReadTimestamp(t, uuid1, "completed_timestamp")
+			startedTime2 := testReadTimestamp(t, uuid2, "started_timestamp")
+			assert.GreaterOrEqual(t, startedTime2, endTime1)
+		})
+	}
 	testAllowConcurrent := func(t *testing.T, name string, uuid string, expect int64) {
 		t.Run("verify allow_concurrent: "+name, func(t *testing.T) {
 			rs := onlineddl.ReadMigrations(t, &vtParams, uuid)
@@ -434,7 +442,7 @@ func testScheduler(t *testing.T) {
 	}
 
 	// CREATE
-	t.Run("CREATE TABLEs t1, t1", func(t *testing.T) {
+	t.Run("CREATE TABLEs t1, t2", func(t *testing.T) {
 		{ // The table does not exist
 			t1uuid = testOnlineDDLStatement(t, createParams(createT1Statement, ddlStrategy, "vtgate", "just-created", "", false))
 			onlineddl.CheckMigrationStatus(t, &vtParams, shards, t1uuid, schema.OnlineDDLStatusComplete)
@@ -1183,6 +1191,36 @@ func testScheduler(t *testing.T) {
 		})
 	})
 	// in-order-completion
+	t.Run("in-order-completion: multiple drops for nonexistent tables and views, sequential", func(t *testing.T) {
+		u, err := schema.CreateOnlineDDLUUID()
+		require.NoError(t, err)
+
+		sqls := []string{
+			fmt.Sprintf("drop table if exists t4_%s", u),
+			fmt.Sprintf("drop view  if exists t1_%s", u),
+			fmt.Sprintf("drop table if exists t2_%s", u),
+			fmt.Sprintf("drop view  if exists t3_%s", u),
+		}
+		sql := strings.Join(sqls, ";")
+		var vuuids []string
+		t.Run("drop multiple tables and views, in-order-completion", func(t *testing.T) {
+			uuidList := testOnlineDDLStatement(t, createParams(sql, ddlStrategy+" --in-order-completion", "vtctl", "", "", true)) // skip wait
+			vuuids = strings.Split(uuidList, "\n")
+			assert.Len(t, vuuids, 4)
+			for _, uuid := range vuuids {
+				status := onlineddl.WaitForMigrationStatus(t, &vtParams, shards, uuid, normalWaitTime, schema.OnlineDDLStatusComplete, schema.OnlineDDLStatusFailed)
+				fmt.Printf("# Migration status (for debug purposes): <%s>\n", status)
+				onlineddl.CheckMigrationStatus(t, &vtParams, shards, uuid, schema.OnlineDDLStatusComplete)
+			}
+		})
+		require.Len(t, vuuids, 4)
+		for i := range vuuids {
+			if i > 0 {
+				testTableCompletionTimes(t, vuuids[i-1], vuuids[i])
+				testTableCompletionAndStartTimes(t, vuuids[i-1], vuuids[i])
+			}
+		}
+	})
 	t.Run("in-order-completion: multiple drops for nonexistent tables and views", func(t *testing.T) {
 		u, err := schema.CreateOnlineDDLUUID()
 		require.NoError(t, err)
@@ -1198,20 +1236,93 @@ func testScheduler(t *testing.T) {
 		t.Run("drop multiple tables and views, in-order-completion", func(t *testing.T) {
 			uuidList := testOnlineDDLStatement(t, createParams(sql, ddlStrategy+" --allow-concurrent --in-order-completion", "vtctl", "", "", true)) // skip wait
 			vuuids = strings.Split(uuidList, "\n")
-			assert.Equal(t, 4, len(vuuids))
+			assert.Len(t, vuuids, 4)
 			for _, uuid := range vuuids {
 				status := onlineddl.WaitForMigrationStatus(t, &vtParams, shards, uuid, normalWaitTime, schema.OnlineDDLStatusComplete, schema.OnlineDDLStatusFailed)
 				fmt.Printf("# Migration status (for debug purposes): <%s>\n", status)
 				onlineddl.CheckMigrationStatus(t, &vtParams, shards, uuid, schema.OnlineDDLStatusComplete)
 			}
 		})
-		require.Equal(t, 4, len(vuuids))
+		require.Len(t, vuuids, 4)
 		for i := range vuuids {
 			if i > 0 {
 				testTableCompletionTimes(t, vuuids[i-1], vuuids[i])
 			}
 		}
 	})
+
+	t.Run("in-order-completion: bail out on first error", func(t *testing.T) {
+		u, err := schema.CreateOnlineDDLUUID()
+		require.NoError(t, err)
+
+		sqls := []string{
+			fmt.Sprintf("drop table if exists t4_%s", u),
+			fmt.Sprintf("drop view  if exists t1_%s", u),
+			fmt.Sprintf("drop table t2_%s", u), // non existent
+			fmt.Sprintf("drop view  if exists t3_%s", u),
+		}
+		sql := strings.Join(sqls, ";")
+		var vuuids []string
+		t.Run("apply schema", func(t *testing.T) {
+			uuidList := testOnlineDDLStatement(t, createParams(sql, ddlStrategy+" --in-order-completion", "vtctl", "", "", true)) // skip wait
+			vuuids = strings.Split(uuidList, "\n")
+			assert.Len(t, vuuids, 4)
+			for _, uuid := range vuuids[0:2] {
+				status := onlineddl.WaitForMigrationStatus(t, &vtParams, shards, uuid, normalWaitTime, schema.OnlineDDLStatusComplete, schema.OnlineDDLStatusFailed)
+				fmt.Printf("# Migration status (for debug purposes): <%s>\n", status)
+				onlineddl.CheckMigrationStatus(t, &vtParams, shards, uuid, schema.OnlineDDLStatusComplete)
+			}
+			{
+				uuid := vuuids[2] // the failed one
+				status := onlineddl.WaitForMigrationStatus(t, &vtParams, shards, uuid, normalWaitTime, schema.OnlineDDLStatusComplete, schema.OnlineDDLStatusFailed)
+				fmt.Printf("# Migration status (for debug purposes): <%s>\n", status)
+				onlineddl.CheckMigrationStatus(t, &vtParams, shards, uuid, schema.OnlineDDLStatusFailed)
+			}
+			{
+				uuid := vuuids[3] // should consequently fail without even running
+				status := onlineddl.WaitForMigrationStatus(t, &vtParams, shards, uuid, normalWaitTime, schema.OnlineDDLStatusComplete, schema.OnlineDDLStatusFailed)
+				fmt.Printf("# Migration status (for debug purposes): <%s>\n", status)
+				onlineddl.CheckMigrationStatus(t, &vtParams, shards, uuid, schema.OnlineDDLStatusQueued, schema.OnlineDDLStatusFailed)
+
+				rs := onlineddl.ReadMigrations(t, &vtParams, uuid)
+				require.NotNil(t, rs)
+				for _, row := range rs.Named().Rows {
+					message := row["message"].ToString()
+					require.Contains(t, message, vuuids[2]) // Indicating this migration failed due to vuuids[2] failure
+				}
+			}
+		})
+		testTableCompletionTimes(t, vuuids[0], vuuids[1])
+		testTableCompletionAndStartTimes(t, vuuids[0], vuuids[1])
+		testTableCompletionAndStartTimes(t, vuuids[1], vuuids[2])
+	})
+	t.Run("in-order-completion concurrent: bail out on first error", func(t *testing.T) {
+		sqls := []string{
+			`alter table t1_test force`,
+			`alter table t2_test force`,
+		}
+		sql := strings.Join(sqls, ";")
+		var vuuids []string
+		t.Run("apply schema", func(t *testing.T) {
+			uuidList := testOnlineDDLStatement(t, createParams(sql, ddlStrategy+" --in-order-completion --postpone-completion --allow-concurrent", "vtctl", "", "", true)) // skip wait
+			vuuids = strings.Split(uuidList, "\n")
+			assert.Len(t, vuuids, 2)
+			for _, uuid := range vuuids {
+				waitForReadyToComplete(t, uuid, true)
+			}
+			t.Run("cancel 1st migration", func(t *testing.T) {
+				onlineddl.CheckCancelMigration(t, &vtParams, shards, vuuids[0], true)
+				status := onlineddl.WaitForMigrationStatus(t, &vtParams, shards, vuuids[0], normalWaitTime, schema.OnlineDDLStatusFailed, schema.OnlineDDLStatusCancelled)
+				fmt.Printf("# Migration status (for debug purposes): <%s>\n", status)
+				onlineddl.CheckMigrationStatus(t, &vtParams, shards, vuuids[0], schema.OnlineDDLStatusCancelled)
+			})
+			t.Run("expect 2nd migration to fail", func(t *testing.T) {
+				status := onlineddl.WaitForMigrationStatus(t, &vtParams, shards, vuuids[1], normalWaitTime, schema.OnlineDDLStatusFailed, schema.OnlineDDLStatusCancelled)
+				fmt.Printf("# Migration status (for debug purposes): <%s>\n", status)
+				onlineddl.CheckMigrationStatus(t, &vtParams, shards, vuuids[1], schema.OnlineDDLStatusFailed)
+			})
+		})
+	})
 	t.Run("in-order-completion: two new views, one depends on the other", func(t *testing.T) {
 		u, err := schema.CreateOnlineDDLUUID()
 		require.NoError(t, err)
@@ -1225,14 +1336,14 @@ func testScheduler(t *testing.T) {
 		t.Run("create two views, expect both complete", func(t *testing.T) {
 			uuidList := testOnlineDDLStatement(t, createParams(sql, ddlStrategy+" --allow-concurrent --in-order-completion", "vtctl", "", "", true)) // skip wait
 			vuuids = strings.Split(uuidList, "\n")
-			assert.Equal(t, 2, len(vuuids))
+			assert.Len(t, vuuids, 2)
 			for _, uuid := range vuuids {
 				status := onlineddl.WaitForMigrationStatus(t, &vtParams, shards, uuid, normalWaitTime, schema.OnlineDDLStatusComplete, schema.OnlineDDLStatusFailed)
 				fmt.Printf("# Migration status (for debug purposes): <%s>\n", status)
 				onlineddl.CheckMigrationStatus(t, &vtParams, shards, uuid, schema.OnlineDDLStatusComplete)
 			}
 		})
-		require.Equal(t, 2, len(vuuids))
+		require.Len(t, vuuids, 2)
 		testTableCompletionTimes(t, vuuids[0], vuuids[1])
 	})
 	t.Run("in-order-completion: new table column, new view depends on said column", func(t *testing.T) {

diff --git a/go/vt/schemadiff/schema_diff_test.go b/go/vt/schemadiff/schema_diff_test.go
@@ -416,13 +416,15 @@ func TestSchemaDiff(t *testing.T) {
 			instantCapability: InstantDDLCapabilityIrrelevant,
 		},
 		{
-			name: "add view with over",
+			name: "add view with over and keyword table",
 			toQueries: append(
 				createQueries,
-				"create view v2 as SELECT *, ROW_NUMBER() OVER(PARTITION BY info) AS row_num1, ROW_NUMBER() OVER(PARTITION BY info ORDER BY id) AS row_num2 FROM t1;\n",
+				"create table `order` (id int primary key, info int not null);",
+				"create view v2 as SELECT *, ROW_NUMBER() OVER(PARTITION BY info) AS row_num1, ROW_NUMBER() OVER(PARTITION BY info ORDER BY id) AS row_num2 FROM `order`;",
 			),
-			expectDiffs:       1,
-			entityOrder:       []string{"v2"},
+			expectDiffs:       2,
+			expectDeps:        1,
+			entityOrder:       []string{"order", "v2"},
 			instantCapability: InstantDDLCapabilityIrrelevant,
 		},
 		{

diff --git a/go/vt/schemadiff/schema_test.go b/go/vt/schemadiff/schema_test.go
@@ -125,6 +125,15 @@ func TestNewSchemaFromQueriesUnresolved(t *testing.T) {
 	assert.Equal(t, "CREATE VIEW `v7` AS SELECT * FROM `v8`, `t2`", v.Create().CanonicalStatementString())
 }
 
+func TestNewSchemaFromQueriesWithSQLKeyword(t *testing.T) {
+	queries := []string{
+		"create table `order` (id int primary key, info int not null)",
+		"create view v2 as SELECT *, ROW_NUMBER() OVER(PARTITION BY info) AS row_num1, ROW_NUMBER() OVER(PARTITION BY info ORDER BY id) AS row_num2 FROM `order`;",
+	}
+	_, err := NewSchemaFromQueries(NewTestEnv(), queries)
+	assert.NoError(t, err)
+}
+
 func TestNewSchemaFromQueriesUnresolvedAlias(t *testing.T) {
 	// v8 does not exist
 	queries := append(schemaTestCreateQueries,

diff --git a/go/vt/schemadiff/semantics.go b/go/vt/schemadiff/semantics.go
@@ -51,7 +51,7 @@ func newDeclarativeSchemaInformation(env *Environment) *declarativeSchemaInforma
 
 // FindTableOrVindex implements the SchemaInformation interface
 func (si *declarativeSchemaInformation) FindTableOrVindex(tablename sqlparser.TableName) (*vindexes.Table, vindexes.Vindex, string, topodatapb.TabletType, key.Destination, error) {
-	table := si.Tables[sqlparser.String(tablename)]
+	table := si.Tables[tablename.Name.String()]
 	return table, nil, "", 0, nil, nil
 }