Skip to content

Commit

Permalink
VTOrc Code Cleanup - generate_base, replace cluster_name with keyspac…
Browse files Browse the repository at this point in the history
…e and shard. (vitessio#12012)

* feat: refactor generate commands of VTOrc to be in a single file

Signed-off-by: Manan Gupta <manan@planetscale.com>

* refactor: cleanup create table formatting

Signed-off-by: Manan Gupta <manan@planetscale.com>

* feat: cleanup the usage of IsSQLite and IsMySQL

Signed-off-by: Manan Gupta <manan@planetscale.com>

* feat: remove unused minimal instance

Signed-off-by: Manan Gupta <manan@planetscale.com>

* feat: remove unused table cluster_domain_name

Signed-off-by: Manan Gupta <manan@planetscale.com>

* feat: fix vtorc database to store keyspace and shard instead of cluster

Signed-off-by: Manan Gupta <manan@planetscale.com>

* feat: remove unused attributes

Signed-off-by: Manan Gupta <manan@planetscale.com>

* feat: remove unused cluster domain

Signed-off-by: Manan Gupta <manan@planetscale.com>

* feat: change GetClusterName to GetKeyspaceAndShardName

Signed-off-by: Manan Gupta <manan@planetscale.com>

* feat: fix insertion into database_instance

Signed-off-by: Manan Gupta <manan@planetscale.com>

* feat: fix SnapshotTopologies

Signed-off-by: Manan Gupta <manan@planetscale.com>

* feat: remove inject unseen primary and inject seed

Signed-off-by: Manan Gupta <manan@planetscale.com>

* feat: remove ClusterName from Instance

Signed-off-by: Manan Gupta <manan@planetscale.com>

* feat: fix Audit operations

Signed-off-by: Manan Gupta <manan@planetscale.com>

* feat: add Keyspace and Shard to cluster information to replace ClusterName

Signed-off-by: Manan Gupta <manan@planetscale.com>

* feat: fix attempt failure detection registeration

Signed-off-by: Manan Gupta <manan@planetscale.com>

* feat: fix blocked topology recoveries

Signed-off-by: Manan Gupta <manan@planetscale.com>

* feat: fix topology recovery

Signed-off-by: Manan Gupta <manan@planetscale.com>

* feat: reading recovery instances

Signed-off-by: Manan Gupta <manan@planetscale.com>

* feat: fix get replication and analysis

Signed-off-by: Manan Gupta <manan@planetscale.com>

* feat: fix bug in query

Signed-off-by: Manan Gupta <manan@planetscale.com>

* test: add tests to check that filtering by keyspace works for APIs

Signed-off-by: Manan Gupta <manan@planetscale.com>

* feat: remove remaining usages of ClusterName

Signed-off-by: Manan Gupta <manan@planetscale.com>

* refactor: fix comment explaining sleep in the test

Signed-off-by: Manan Gupta <manan@planetscale.com>

* feat: add code to prevent filtering just by shard and add tests for it

Signed-off-by: Manan Gupta <manan@planetscale.com>

Signed-off-by: Manan Gupta <manan@planetscale.com>
  • Loading branch information
GuptaManan100 authored and timvaillancourt committed Sep 11, 2023
1 parent f9de118 commit a725f1a
Show file tree
Hide file tree
Showing 27 changed files with 1,010 additions and 1,656 deletions.
2 changes: 1 addition & 1 deletion go/cmd/vtorc/status.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
})
}
20 changes: 20 additions & 0 deletions go/test/endtoend/vtorc/api/api_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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)
})
}
2 changes: 0 additions & 2 deletions go/test/endtoend/vtorc/readtopologyinstance/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,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")
Expand Down Expand Up @@ -160,5 +159,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)
}
26 changes: 0 additions & 26 deletions go/vt/vtorc/attributes/attributes.go

This file was deleted.

109 changes: 0 additions & 109 deletions go/vt/vtorc/attributes/attributes_dao.go

This file was deleted.

12 changes: 1 addition & 11 deletions go/vt/vtorc/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
71 changes: 10 additions & 61 deletions go/vt/vtorc/db/db.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,10 +53,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
Expand All @@ -72,11 +68,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
Expand All @@ -101,30 +94,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)
Expand All @@ -143,11 +114,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())
}
Expand All @@ -168,36 +134,27 @@ func ClearVTOrcDatabase() {
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
Expand All @@ -208,11 +165,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
Expand All @@ -223,11 +176,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
Expand Down
Loading

0 comments on commit a725f1a

Please sign in to comment.