VTOrc Code Cleanup - generate_base, replace cluster_name with keyspac…

…e and shard. (#12012) (#1469) * feat: refactor generate commands of VTOrc to be in a single file Signed-off-by: Manan Gupta <[email protected]> * refactor: cleanup create table formatting Signed-off-by: Manan Gupta <[email protected]> * feat: cleanup the usage of IsSQLite and IsMySQL Signed-off-by: Manan Gupta <[email protected]> * feat: remove unused minimal instance Signed-off-by: Manan Gupta <[email protected]> * feat: remove unused table cluster_domain_name Signed-off-by: Manan Gupta <[email protected]> * feat: fix vtorc database to store keyspace and shard instead of cluster Signed-off-by: Manan Gupta <[email protected]> * feat: remove unused attributes Signed-off-by: Manan Gupta <[email protected]> * feat: remove unused cluster domain Signed-off-by: Manan Gupta <[email protected]> * feat: change GetClusterName to GetKeyspaceAndShardName Signed-off-by: Manan Gupta <[email protected]> * feat: fix insertion into database_instance Signed-off-by: Manan Gupta <[email protected]> * feat: fix SnapshotTopologies Signed-off-by: Manan Gupta <[email protected]> * feat: remove inject unseen primary and inject seed Signed-off-by: Manan Gupta <[email protected]> * feat: remove ClusterName from Instance Signed-off-by: Manan Gupta <[email protected]> * feat: fix Audit operations Signed-off-by: Manan Gupta <[email protected]> * feat: add Keyspace and Shard to cluster information to replace ClusterName Signed-off-by: Manan Gupta <[email protected]> * feat: fix attempt failure detection registeration Signed-off-by: Manan Gupta <[email protected]> * feat: fix blocked topology recoveries Signed-off-by: Manan Gupta <[email protected]> * feat: fix topology recovery Signed-off-by: Manan Gupta <[email protected]> * feat: reading recovery instances Signed-off-by: Manan Gupta <[email protected]> * feat: fix get replication and analysis Signed-off-by: Manan Gupta <[email protected]> * feat: fix bug in query Signed-off-by: Manan Gupta <[email protected]> * test: add tests to check that filtering by keyspace works for APIs Signed-off-by: Manan Gupta <[email protected]> * feat: remove remaining usages of ClusterName Signed-off-by: Manan Gupta <[email protected]> * refactor: fix comment explaining sleep in the test Signed-off-by: Manan Gupta <[email protected]> * feat: add code to prevent filtering just by shard and add tests for it Signed-off-by: Manan Gupta <[email protected]> Signed-off-by: Manan Gupta <[email protected]> Signed-off-by: Manan Gupta <[email protected]>
vitessio · Jan 10, 2023 · e0c178b · e0c178b
1 parent 5f2209f
commit e0c178b
Show file tree

Hide file tree

Showing 27 changed files with 1,010 additions and 1,658 deletions.
diff --git a/go/cmd/vtorc/status.go b/go/cmd/vtorc/status.go
@@ -24,7 +24,7 @@ import (
 // addStatusParts adds UI parts to the /debug/status page of VTOrc
 func addStatusParts() {
 	servenv.AddStatusPart("Recent Recoveries", logic.TopologyRecoveriesTemplate, func() any {
-		recoveries, _ := logic.ReadRecentRecoveries("", false, 0)
+		recoveries, _ := logic.ReadRecentRecoveries(false, 0)
 		return recoveries
 	})
 }
diff --git a/go/test/endtoend/vtorc/api/api_test.go b/go/test/endtoend/vtorc/api/api_test.go
@@ -107,10 +107,20 @@ func TestProblemsAPI(t *testing.T) {
 		assert.Equal(t, 200, status, resp)
 		assert.Contains(t, resp, fmt.Sprintf(`"Port": %d`, replica.MySQLPort))
 
+		// Verify that filtering by keyspace also works in the API as intended
+		status, resp = utils.MakeAPICall(t, vtorc, "/api/replication-analysis?keyspace=ks")
+		assert.Equal(t, 200, status, resp)
+		assert.Contains(t, resp, fmt.Sprintf(`"Port": %d`, replica.MySQLPort))
+
 		// Check that filtering using keyspace and shard works
 		status, resp = utils.MakeAPICall(t, vtorc, "/api/replication-analysis?keyspace=ks&shard=80-")
 		assert.Equal(t, 200, status, resp)
 		assert.Equal(t, "[]", resp)
+
+		// Check that filtering using just the shard fails
+		status, resp = utils.MakeAPICall(t, vtorc, "/api/replication-analysis?shard=0")
+		assert.Equal(t, 400, status, resp)
+		assert.Equal(t, "Filtering by shard without keyspace isn't supported\n", resp)
 	})
 
 	t.Run("Enable Recoveries API", func(t *testing.T) {
@@ -150,9 +160,19 @@ func TestProblemsAPI(t *testing.T) {
 		assert.Equal(t, 200, status, resp)
 		assert.Contains(t, resp, fmt.Sprintf(`"InstanceAlias": "%v"`, replica.Alias))
 
+		// Check that filtering using keyspace works
+		status, resp = utils.MakeAPICall(t, vtorc, "/api/problems?keyspace=ks")
+		assert.Equal(t, 200, status, resp)
+		assert.Contains(t, resp, fmt.Sprintf(`"InstanceAlias": "%v"`, replica.Alias))
+
 		// Check that filtering using keyspace and shard works
 		status, resp = utils.MakeAPICall(t, vtorc, "/api/problems?keyspace=ks&shard=80-")
 		assert.Equal(t, 200, status, resp)
 		assert.Equal(t, "null", resp)
+
+		// Check that filtering using just the shard fails
+		status, resp = utils.MakeAPICall(t, vtorc, "/api/problems?shard=0")
+		assert.Equal(t, 400, status, resp)
+		assert.Equal(t, "Filtering by shard without keyspace isn't supported\n", resp)
 	})
 }
diff --git a/go/test/endtoend/vtorc/readtopologyinstance/main_test.go b/go/test/endtoend/vtorc/readtopologyinstance/main_test.go
@@ -103,7 +103,6 @@ func TestReadTopologyInstanceBufferable(t *testing.T) {
 	assert.False(t, primaryInstance.HasReplicationCredentials)
 	assert.Equal(t, primaryInstance.ReplicationIOThreadState, inst.ReplicationThreadStateNoThread)
 	assert.Equal(t, primaryInstance.ReplicationSQLThreadState, inst.ReplicationThreadStateNoThread)
-	assert.Equal(t, fmt.Sprintf("%v:%v", keyspace.Name, shard0.Name), primaryInstance.ClusterName)
 
 	// insert an errant GTID in the replica
 	_, err = utils.RunSQL(t, "insert into vt_insert_test(id, msg) values (10173, 'test 178342')", replica, "vt_ks")
@@ -159,5 +158,4 @@ func TestReadTopologyInstanceBufferable(t *testing.T) {
 	assert.False(t, replicaInstance.HasReplicationFilters)
 	assert.LessOrEqual(t, int(replicaInstance.SecondsBehindPrimary.Int64), 1)
 	assert.False(t, replicaInstance.AllowTLS)
-	assert.Equal(t, fmt.Sprintf("%v:%v", keyspace.Name, shard0.Name), replicaInstance.ClusterName)
 }
diff --git a/go/vt/vtorc/attributes/attributes.go b/go/vt/vtorc/attributes/attributes.go
diff --git a/go/vt/vtorc/attributes/attributes_dao.go b/go/vt/vtorc/attributes/attributes_dao.go
diff --git a/go/vt/vtorc/config/config.go b/go/vt/vtorc/config/config.go
@@ -162,23 +162,13 @@ func newConfiguration() *Configuration {
 }
 
 func (config *Configuration) postReadAdjustments() error {
-	if config.IsSQLite() && config.SQLite3DataFile == "" {
+	if config.SQLite3DataFile == "" {
 		return fmt.Errorf("SQLite3DataFile must be set")
 	}
 
 	return nil
 }
 
-// TODO: Simplify the callers and delete this function
-func (config *Configuration) IsSQLite() bool {
-	return true
-}
-
-// TODO: Simplify the callers and delete this function
-func (config *Configuration) IsMySQL() bool {
-	return false
-}
-
 // read reads configuration from given file, or silently skips if the file does not exist.
 // If the file does exist, then it is expected to be in valid JSON format or the function bails out.
 func read(fileName string) (*Configuration, error) {

diff --git a/go/vt/vtorc/db/db.go b/go/vt/vtorc/db/db.go
@@ -54,10 +54,6 @@ func (dummyRes DummySQLResult) RowsAffected() (int64, error) {
 	return 1, nil
 }
 
-func IsSQLite() bool {
-	return config.Config.IsSQLite()
-}
-
 // OpenTopology returns the DB instance for the vtorc backed database
 func OpenVTOrc() (db *sql.DB, err error) {
 	var fromCache bool
@@ -73,11 +69,8 @@ func OpenVTOrc() (db *sql.DB, err error) {
 	return db, err
 }
 
-func translateStatement(statement string) (string, error) {
-	if IsSQLite() {
-		statement = sqlutils.ToSqlite3Dialect(statement)
-	}
-	return statement, nil
+func translateStatement(statement string) string {
+	return sqlutils.ToSqlite3Dialect(statement)
 }
 
 // registerVTOrcDeployment updates the vtorc_metadata table upon successful deployment
@@ -102,30 +95,8 @@ func deployStatements(db *sql.DB, queries []string) error {
 	if err != nil {
 		log.Fatal(err.Error())
 	}
-	// Ugly workaround ahead.
-	// Origin of this workaround is the existence of some "timestamp NOT NULL," column definitions,
-	// where in NO_ZERO_IN_DATE,NO_ZERO_DATE sql_mode are invalid (since default is implicitly "0")
-	// This means installation of vtorc fails on such configured servers, and in particular on 5.7
-	// where this setting is the dfault.
-	// For purpose of backwards compatability, what we do is force sql_mode to be more relaxed, create the schemas
-	// along with the "invalid" definition, and then go ahead and fix those definitions via following ALTER statements.
-	// My bad.
-	originalSQLMode := ""
-	if config.Config.IsMySQL() {
-		_ = tx.QueryRow(`select @@session.sql_mode`).Scan(&originalSQLMode)
-		if _, err := tx.Exec(`set @@session.sql_mode=REPLACE(@@session.sql_mode, 'NO_ZERO_DATE', '')`); err != nil {
-			log.Fatal(err.Error())
-		}
-		if _, err := tx.Exec(`set @@session.sql_mode=REPLACE(@@session.sql_mode, 'NO_ZERO_IN_DATE', '')`); err != nil {
-			log.Fatal(err.Error())
-		}
-	}
 	for _, query := range queries {
-		query, err := translateStatement(query)
-		if err != nil {
-			log.Fatalf("Cannot initiate vtorc: %+v; query=%+v", err, query)
-			return err
-		}
+		query = translateStatement(query)
 		if _, err := tx.Exec(query); err != nil {
 			if strings.Contains(err.Error(), "syntax error") {
 				log.Fatalf("Cannot initiate vtorc: %+v; query=%+v", err, query)
@@ -144,11 +115,6 @@ func deployStatements(db *sql.DB, queries []string) error {
 			}
 		}
 	}
-	if config.Config.IsMySQL() {
-		if _, err := tx.Exec(`set session sql_mode=?`, originalSQLMode); err != nil {
-			log.Fatal(err.Error())
-		}
-	}
 	if err := tx.Commit(); err != nil {
 		log.Fatal(err.Error())
 	}
@@ -160,36 +126,27 @@ func deployStatements(db *sql.DB, queries []string) error {
 func initVTOrcDB(db *sql.DB) error {
 	log.Info("Initializing vtorc")
 	log.Info("Migrating database schema")
-	_ = deployStatements(db, generateSQLBase)
-	_ = deployStatements(db, generateSQLPatches)
+	_ = deployStatements(db, vtorcBackend)
 	_ = registerVTOrcDeployment(db)
 
-	if IsSQLite() {
-		_, _ = ExecVTOrc(`PRAGMA journal_mode = WAL`)
-		_, _ = ExecVTOrc(`PRAGMA synchronous = NORMAL`)
-	}
+	_, _ = ExecVTOrc(`PRAGMA journal_mode = WAL`)
+	_, _ = ExecVTOrc(`PRAGMA synchronous = NORMAL`)
 
 	return nil
 }
 
 // execInternal
 func execInternal(db *sql.DB, query string, args ...any) (sql.Result, error) {
 	var err error
-	query, err = translateStatement(query)
-	if err != nil {
-		return nil, err
-	}
+	query = translateStatement(query)
 	res, err := sqlutils.ExecNoPrepare(db, query, args...)
 	return res, err
 }
 
 // ExecVTOrc will execute given query on the vtorc backend database.
 func ExecVTOrc(query string, args ...any) (sql.Result, error) {
 	var err error
-	query, err = translateStatement(query)
-	if err != nil {
-		return nil, err
-	}
+	query = translateStatement(query)
 	db, err := OpenVTOrc()
 	if err != nil {
 		return nil, err
@@ -200,11 +157,7 @@ func ExecVTOrc(query string, args ...any) (sql.Result, error) {
 
 // QueryVTOrcRowsMap
 func QueryVTOrcRowsMap(query string, onRow func(sqlutils.RowMap) error) error {
-	query, err := translateStatement(query)
-	if err != nil {
-		log.Fatalf("Cannot query vtorc: %+v; query=%+v", err, query)
-		return err
-	}
+	query = translateStatement(query)
 	db, err := OpenVTOrc()
 	if err != nil {
 		return err
@@ -215,11 +168,7 @@ func QueryVTOrcRowsMap(query string, onRow func(sqlutils.RowMap) error) error {
 
 // QueryVTOrc
 func QueryVTOrc(query string, argsArray []any, onRow func(sqlutils.RowMap) error) error {
-	query, err := translateStatement(query)
-	if err != nil {
-		log.Fatalf("Cannot query vtorc: %+v; query=%+v", err, query)
-		return err
-	}
+	query = translateStatement(query)
 	db, err := OpenVTOrc()
 	if err != nil {
 		return err