Bug Report: sidecar DB init unnecessarily issues CREATE DATABASE when the DB exists but some table(s) need to be created/altered

[mattlord]

Overview of the Issue

Early on in the tablet's sidecar DB init process we check to see if the the DB exists and if not we create it:

vitess/go/vt/sidecardb/sidecardb.go

Lines 255 to 264 in fd0ffeb

	dbExists, err := si.doesSidecarDBExist()
	if err != nil {
	return err
	}
	if !dbExists {
	if err := si.createSidecarDB(); err != nil {
	return err
	}
	si.dbCreated = true
	}

But we do NOT set si.dbCreated to true when it was previously created. So if there are any tables that we need to CREATE/ALTER then we get in here and because we did not set si.dbCreated previously, we create it again (which does not fail because we use create database if not exists):

vitess/go/vt/sidecardb/sidecardb.go

Lines 434 to 451 in fd0ffeb

	ddl, err = si.findTableSchemaDiff(table.name, currentTableSchema, desiredTableSchema)
	if err != nil {
	return err
	}
	if ddl != "" {
	if !si.dbCreated {
	// We use createSidecarDB to also create the
	// first binlog entry when a primary comes up.
	// That statement doesn't make it to the
	// replicas, so we run the query again so that
	// it is replicated to the replicas so that the
	// replicas can create the sidecar database.
	if err := si.createSidecarDB(); err != nil {
	return err
	}
	si.dbCreated = true
	}

This does not cause any problems, but it does perform unnecessary work. The fix should be something like this:

diff --git a/go/vt/sidecardb/sidecardb.go b/go/vt/sidecardb/sidecardb.go
index 9cbe054003..036681f895 100644
--- a/go/vt/sidecardb/sidecardb.go
+++ b/go/vt/sidecardb/sidecardb.go
@@ -252,11 +252,12 @@ func Init(ctx context.Context, env *vtenv.Environment, exec Exec) error {
 	// Hence, we should not always try to CREATE the
 	// database, since it will then error out as the instance
 	// is read-only.
-	dbExists, err := si.doesSidecarDBExist()
+	var err error
+	si.dbCreated, err = si.doesSidecarDBExist()
 	if err != nil {
 		return err
 	}
-	if !dbExists {
+	if !si.dbCreated {
 		if err := si.createSidecarDB(); err != nil {
 			return err
 		}

OR we could remove that second creation spot entirely. It does not seem necessary to do the create again, especially w/o another check to see if it exists.

Reproduction Steps

git checkout main

make build && cd examples/local

alias vtctldclient='command vtctldclient --server=localhost:15999'

./101_initial_cluster.sh

for uid in $(vtctldclient GetTablets | awk '{print $1}' | cut -d- -f2 | bc); do
  command mysql -u root --socket "${VTDATAROOT}/vt_0000000${uid}/mysql.sock" -e "set @@global.general_log=ON"
done

commerce_primary_uid=$(vtctldclient GetTablets --keyspace commerce --tablet-type primary --shard "0" | awk '{print $1}' | cut -d- -f2 | bc)

command mysql -u root --socket "${VTDATAROOT}/vt_0000000${commerce_primary_uid}/mysql.sock" -e "drop table _vt.vdiff"

vtctldclient PlannedReparentShard commerce/0

sleep 5

grep -i "create database" "${VTDATAROOT}"/vt_000000010*/data/$(hostname -s).log

You'll see grep output like this:

/opt/vtdataroot/vt_0000000100/data/pslord.log:2025-01-14T21:01:41.433730Z         68 Query     create database if not exists _vt
/opt/vtdataroot/vt_0000000101/data/pslord.log:2025-01-14T21:01:41.432689Z          78 Query     create database if not exists _vt
/opt/vtdataroot/vt_0000000102/data/pslord.log:2025-01-14T21:01:41.433068Z          38 Query     create database if not exists _vt

Binary Version

❯ vtgate --version
vtgate version Version: 22.0.0-SNAPSHOT (Git revision 06def1405671266d64ac93832a755dc2c245fe32 branch 'main') built on Tue Jan 14 15:55:55 EST 2025 by [email protected] using go1.23.4 darwin/arm64

Operating System and Environment details

N/A

Log Fragments

No response

[mattlord]

I wanted to note that the current behavior may be necessary to deal with some specific cases:

vitess/go/vt/sidecardb/sidecardb.go

Lines 441 to 446 in fd0ffeb

	// We use createSidecarDB to also create the
	// first binlog entry when a primary comes up.
	// That statement doesn't make it to the
	// replicas, so we run the query again so that
	// it is replicated to the replicas so that the
	// replicas can create the sidecar database.