Draft: Tablet schema initialization refactoring #10533

rsajwani · 2022-06-16T23:31:20Z

Signed-off-by: Rameez Sajwani [email protected]

Description

This is an attempt to consolidated all schema initialization and DB creation at the tablet start-up. The idea is if we can do all schema changes in one place and then seal the db with super-read-only, it will help us avoid errant GTIDs.

Details

We have created Schema_Initialization class which contains list of initialization modules needed to be run during tablet setup. Every module needs to register themselves with schema_Initializer e.g metadata_tables.go, online DDL/executor.go, rpc_replication.go etc.

During tablet initialization at various places in code-base whether we are bootstrapping a completely new tablet or restoring from backup we executes these modules registered as part of their init() routine. Once all the modules are executed we set 'super_read_only' false. This means we won't allow any mutation outside primary tablet. This will help us avoid errand GTIDS.

During tablet promotion we set super-read-only to false and during tablet demotion we set super-read-only to true. This way apart from primary tablet no other replica will be able to produce schema changes from outside.

For Reference

Some related work items done in the past

#10094
#9312
#10448

Unresolved Issued during code changes:

I have to create a separate init_db.sql for maria db since maria db does not have super_read_only. I could either have an if-then-else condition inside maria db but then I choose instead of creating a separate init_db file.
Adding # add custom sql here inside init-db.sql file. This is because in our test we modify the file and add custom slq like (change password of dba etc). Now that we expect super_read_only to execute at the end, we need these custom sql to be added before that last line. See example go/test/endtoend/backup/vtbackup/main_test.go
In one of our end to end test we have to set super-read-only test to false explicitly else our upgrade-downgrad test failed since they run with N-1 vttablet binary, which does not have code to disable super-read-only during InitializeShard. e.g
go/test/endtoend/cluster/cluster_process.go
In tests like backup_only_test.go, I have to induce sleep else the tests are failing. I have a hunch why it is failing but I am not able to find a good solution for that.
Was not able to initialize the schema for health_streamer. Because if we create schema-copy-table beforehand then a test case like vitess.io/vitess/go/test/endtoend/vtgate/schematracker/loadkeyspace -run TestBlockedLoadKeyspace fails. The reason is they expect schema-copy-table to present only if we have enabled schema tracking.
Should we return false / error if any of the schema initialization fails … Right now I just log them but I believe we should return fail tablet initialization if any of the schema initialization fails e.g createReparentJournal.

Related Issue(s)

#10363

Checklist

"Backport me!" label has been added if this change should be backported
Tests were added or are not required
Documentation was added or is not required

Deployment Notes

Signed-off-by: Rameez Sajwani <[email protected]>

GuptaManan100 · 2022-08-22T06:34:35Z

config/init_maria_db.sql

+# This file is executed immediately after mysql_install_db,
+# to initialize a fresh data directory.


Is this separate file created for the super_read_only changes which are only present in the MySQL file?

config/mycnf/mysql57.cnf

GuptaManan100 · 2022-08-22T06:50:44Z

go/cmd/vtbackup/vtbackup.go

+		// We need to switch off super-read-only before we create database.
+		_ = mysqld.SetSuperReadOnly(false)
+		defer func() {
+			_ = mysqld.SetSuperReadOnly(true)
+		}()


Should we not read the super_readonly value here too before we do a defer to true? What if super read-only was turned off in the beginning, do we want to enable it in the end?

GuptaManan100 · 2022-08-22T08:31:56Z

go/mysql/query.go

+		if err := c.WriteComQuery("SELECT @@global.super_read_only"); err != nil {
+			return nil, err
+		}
+		res, _, _, err := c.ReadQueryResult(1, false)
+		if err == nil && len(res.Rows) == 1 {
+			sro := res.Rows[0][0].ToString()
+			if sro == "1" || sro == "ON" {
+				if err = c.WriteComQuery("SET GLOBAL super_read_only='OFF'"); err != nil {
+					return nil, err
+				}
+			}
+		}


Instead, isn't it easier to turn off super read-only regardless of what its value is before, since we'll only end up running one query then?

GuptaManan100 · 2022-08-22T08:32:14Z

go/mysql/query.go

+func (c *Conn) ExecuteSetSuperReadOnly() (result *sqltypes.Result, err error) {
+	// Note: MariaDB does not have super_read_only but support for it is EOL in v14.0+
+	if !c.IsMariaDB() {
+		if err := c.WriteComQuery("SELECT @@global.super_read_only"); err != nil {
+			return nil, err
+		}
+		res, _, _, err := c.ReadQueryResult(1, false)
+		if err == nil && len(res.Rows) == 1 {
+			sro := res.Rows[0][0].ToString()
+			if sro == "0" || sro == "OFF" {
+				if err = c.WriteComQuery("SET GLOBAL super_read_only='ON'"); err != nil {
+					return nil, err
+				}
+			}
+		}
+	}
+
+	return result, err
+}


Same as above

GuptaManan100 · 2022-08-22T08:32:45Z

go/mysql/query.go

+	if err := c.WriteComQuery("SELECT @@global.read_only"); err != nil {
+		return nil, err
+	}
+	res, _, _, err := c.ReadQueryResult(1, false)
+	if err == nil && len(res.Rows) == 1 {
+		sro := res.Rows[0][0].ToString()
+		if sro == "1" || sro == "ON" {
+			if err = c.WriteComQuery("SET GLOBAL read_only='OFF'"); err != nil {
+				return nil, err
+			}
+		}
+	}
+
+	return result, err


Same as above

GuptaManan100 · 2022-08-22T08:59:55Z

go/vt/vtgate/endtoend/misc_test.go

@@ -51,7 +51,7 @@ func TestSysNumericPrecisionScale(t *testing.T) {

 func TestCreateAndDropDatabase(t *testing.T) {
 	// note that this is testing vttest and not vtgate
-	conn, err := mysql.Connect(ctx, &vtParams)
+	conn, err := mysql.Connect(ctx, &mysqlParams)


This doesn't seem correct. We should be connecting to vtgate and not mysql

GuptaManan100 · 2022-08-22T09:02:40Z

go/vt/vttest/environment.go

+	var initFile = path.Join(os.Getenv("VTROOT"), "config/init_db.sql")
+	if !env.IsSQLFlavor() {
+		initFile = path.Join(os.Getenv("VTROOT"), "config/init_maria_db.sql")
+	}


Instead of asking the environment, we could query the MySQL instance directly to know if it is mariadb distribution or not. This would prevent users from running into configuration issues.

GuptaManan100 · 2022-08-22T09:04:19Z

test/templates/unit_test.tpl

+        {{if (eq .Platform "mariadb103")}}
+        # mariadb103
+        export MYSQL_FLAVOR="MariaDB103"
+        {{end}}


If we decide to go with reading the MySQL version from the instance itself, this won't be required.

GuptaManan100 · 2022-08-22T09:04:44Z

go/vt/srvtopo/query.go

+	if *srvTopoNoCacheForGet {
+		entry := &queryEntry{
+			key: wkey,
+		}
+		result, err := q.query(ctx, entry)
+		if err != nil {
+			return nil, err
+		}
+		return result, nil
+	}


Is this change related to schema initialization?

GuptaManan100 · 2022-08-22T09:05:32Z

go/flags/endtoend/vtctld.txt

@@ -103,6 +103,7 @@ Usage of vtctld:
      --hot_row_protection_concurrent_transactions int                   Number of concurrent transactions let through to the txpool/MySQL for the same hot row. Should be > 1 to have enough 'ready' transactions in MySQL and benefit from a pipelining effect. (default 5)
      --hot_row_protection_max_global_queue_size int                     Global queue limit across all row (ranges). Useful to prevent that the queue can grow unbounded. (default 1000)
      --hot_row_protection_max_queue_size int                            Maximum number of BeginExecute RPCs which will be queued for the same row (range). (default 20)
+      --init_populate_metadata                                           (init parameter) populate metadata tables even if restore_from_backup is disabled. If restore_from_backup is enabled, metadata tables are always populated regardless of this flag.


This flag shouldn't be in vtctld

Signed-off-by: Rameez Sajwani <[email protected]>

github-actions · 2022-10-22T01:33:54Z

This PR is being marked as stale because it has been open for 30 days with no activity. To rectify, you may do any of the following:

Push additional commits to the associated branch.
Remove the stale label.
Add a comment indicating why it is not stale.

If no action is taken within 7 days, this PR will be closed.

github-actions · 2022-10-29T01:35:24Z

This PR was closed because it has been stale for 7 days with no activity.

Adding super user in config file

cfeb733

Signed-off-by: Rameez Sajwani <[email protected]>

deepthi changed the title ~~Draft: Non-Production: Experimenting with super-read-only user~~ Draft: Experimenting with super-read-only user Jun 22, 2022

rsajwani added 6 commits July 1, 2022 22:58

Consolidating all mutations

235944b

Signed-off-by: Rameez Sajwani <[email protected]>

Removing un-necessary code

10d736a

Signed-off-by: Rameez Sajwani <[email protected]>

Refactor InitSchemas to Init

0a5fb2b

Signed-off-by: Rameez Sajwani <[email protected]>

Cleanup: Remove un-wanted code

6d50661

Signed-off-by: Rameez Sajwani <[email protected]>

Merge remote-tracking branch 'upstream/main' into Issue#10363

d0b5f28

Signed-off-by: Rameez Sajwani <[email protected]>

Fixing test failures

e6dcba5

Signed-off-by: Rameez Sajwani <[email protected]>

rsajwani changed the title ~~Draft: Experimenting with super-read-only user~~ Draft: Tablet schema initialization refactoring Jul 8, 2022

rsajwani added 3 commits July 8, 2022 17:12

Merge remote-tracking branch 'upstream/main' into Issue#10363

6b31101

Signed-off-by: Rameez Sajwani <[email protected]>

Fixing test cases

8dda034

Signed-off-by: Rameez Sajwani <[email protected]>

Fixing health check test failures

01fe631

Signed-off-by: Rameez Sajwani <[email protected]>

rsajwani force-pushed the Issue#10363 branch from 87c7471 to 01fe631 Compare July 14, 2022 00:31

rsajwani added 17 commits July 14, 2022 17:43

Fixing test cases

c12124b

Signed-off-by: Rameez Sajwani <[email protected]>

Fixing more test cases

b7eb505

Signed-off-by: Rameez Sajwani <[email protected]>

Fixing vtgate end to end test cases

46cc9a0

Signed-off-by: Rameez Sajwani <[email protected]>

Merge remote-tracking branch 'origin/main' into Issue#10363

761569a

Signed-off-by: Rameez Sajwani <[email protected]>

Fixing end to end test

2f2424c

Signed-off-by: Rameez Sajwani <[email protected]>

Merge remote-tracking branch 'upstream/main' into Issue#10363

2c095a5

Signed-off-by: Rameez Sajwani <[email protected]>

FakeMysql fixes

9152b24

Signed-off-by: Rameez Sajwani <[email protected]>

Fix health_stream test

d5f4426

Signed-off-by: Rameez Sajwani <[email protected]>

Fixing reparent tests

2cec8db

Signed-off-by: Rameez Sajwani <[email protected]>

Fixing backup tests

d88be5a

Signed-off-by: Rameez Sajwani <[email protected]>

Fixing backup tests

2e97acd

Signed-off-by: Rameez Sajwani <[email protected]>

Merge remote-tracking branch 'upstream/main' into Issue#10363

7ae5ee4

Signed-off-by: Rameez Sajwani <[email protected]>

Fixing reparent tests

01f677e

Signed-off-by: Rameez Sajwani <[email protected]>

Setting super-read-only flag to true by default

8073fae

Signed-off-by: Rameez Sajwani <[email protected]>

Fixing test for ricebox and other test cases

e3fcec5

Signed-off-by: Rameez Sajwani <[email protected]>

Fix init_db.sql issue

2afb03f

Signed-off-by: Rameez Sajwani <[email protected]>

Fixing vreplication tests

20ea847

Signed-off-by: Rameez Sajwani <[email protected]>

rsajwani added the Component: Cluster management label Aug 19, 2022

deepthi requested a review from GuptaManan100 August 19, 2022 14:34

GuptaManan100 reviewed Aug 22, 2022

View reviewed changes

rsajwani added 20 commits August 25, 2022 17:15

remove 10 second delay from test.

817493d

Signed-off-by: Rameez Sajwani <[email protected]>

Merge remote-tracking branch 'upstream/main' into Issue#10363

4ad6160

Signed-off-by: Rameez Sajwani <[email protected]>

Adding create/alter reparent back to initPrimary

b189736

Signed-off-by: Rameez Sajwani <[email protected]>

Adding super-read-only tests

005214e

Signed-off-by: Rameez Sajwani <[email protected]>

Fixing some hacky solutions

fb5eed4

Signed-off-by: Rameez Sajwani <[email protected]>

Merge remote-tracking branch 'upstream/main' into Issue#10363

63f3c7a

Signed-off-by: Rameez Sajwani <[email protected]>

Fixing cache issue in test

0939da5

Signed-off-by: Rameez Sajwani <[email protected]>

fixing unit test cases

5bd4c89

Signed-off-by: Rameez Sajwani <[email protected]>

Merge remote-tracking branch 'upstream/main' into Issue#10363

d996c8a

Signed-off-by: Rameez Sajwani <[email protected]>

Fixing backup test

fa7c9a8

Signed-off-by: Rameez Sajwani <[email protected]>

Fix fake tablet

7861992

Signed-off-by: Rameez Sajwani <[email protected]>

fixing Vtorc test

ae4d344

Signed-off-by: Rameez Sajwani <[email protected]>

Fixing some code review feedback

48a84d8

Signed-off-by: Rameez Sajwani <[email protected]>

Fixing failed test cases

c11fe74

Signed-off-by: Rameez Sajwani <[email protected]>

Fixing helper strings

5d2617f

Signed-off-by: Rameez Sajwani <[email protected]>

Fixing vreplication test

2f4d794

Signed-off-by: Rameez Sajwani <[email protected]>

Fixing vtbackup failures

e80e365

Signed-off-by: Rameez Sajwani <[email protected]>

Code clean-up

ff58952

Signed-off-by: Rameez Sajwani <[email protected]>

Fixing upgrade/downgrade tests

76e13f2

Signed-off-by: Rameez Sajwani <[email protected]>

Merge remote-tracking branch 'upstream/main' into Issue#10363

fb938b2

Signed-off-by: Rameez Sajwani <[email protected]>

github-actions bot added the Stale Marks PRs as stale after a period of inactivity, which are then closed after a grace period. label Oct 22, 2022

github-actions bot closed this Oct 29, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Draft: Tablet schema initialization refactoring #10533

Draft: Tablet schema initialization refactoring #10533

rsajwani commented Jun 16, 2022 •

edited

Loading

GuptaManan100 Aug 22, 2022

GuptaManan100 Aug 22, 2022

GuptaManan100 Aug 22, 2022

GuptaManan100 Aug 22, 2022

GuptaManan100 Aug 22, 2022

GuptaManan100 Aug 22, 2022

GuptaManan100 Aug 22, 2022

GuptaManan100 Aug 22, 2022

GuptaManan100 Aug 22, 2022

GuptaManan100 Aug 22, 2022

github-actions bot commented Oct 22, 2022

github-actions bot commented Oct 29, 2022

		# This file is executed immediately after mysql_install_db,
		# to initialize a fresh data directory.

Draft: Tablet schema initialization refactoring #10533

Draft: Tablet schema initialization refactoring #10533

Conversation

rsajwani commented Jun 16, 2022 • edited Loading

Description

Details

For Reference

Unresolved Issued during code changes:

Related Issue(s)

Checklist

Deployment Notes

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Oct 22, 2022

github-actions bot commented Oct 29, 2022

rsajwani commented Jun 16, 2022 •

edited

Loading