From 2c59e4ed40e4262ef333bda4b64c03002585bee2 Mon Sep 17 00:00:00 2001 From: Jaskanwal Pawar <32091345+jspawar@users.noreply.github.com> Date: Mon, 15 May 2023 17:07:01 -0700 Subject: [PATCH 1/2] Fix race in DBDDL plugin with vtcombo - Plugin previously would create tablets with the same UID - This change simply adds a lock so that only one `CreateDatabase` operation can happen at a time => UID should always be unique Signed-off-by: Jaskanwal Pawar <32091345+jspawar@users.noreply.github.com> --- go/cmd/vtcombo/plugin_dbddl.go | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/go/cmd/vtcombo/plugin_dbddl.go b/go/cmd/vtcombo/plugin_dbddl.go index 49a7a601fb1..bbd5d1f7404 100644 --- a/go/cmd/vtcombo/plugin_dbddl.go +++ b/go/cmd/vtcombo/plugin_dbddl.go @@ -18,6 +18,7 @@ package main import ( "context" + "sync" "vitess.io/vitess/go/vt/servenv" "vitess.io/vitess/go/vt/vtgate/engine" @@ -29,7 +30,9 @@ var globalCreateDb func(ctx context.Context, ks *vttestpb.Keyspace) error var globalDropDb func(ctx context.Context, ksName string) error // DBDDL doesn't need to store any state - we use the global variables above instead -type DBDDL struct{} +type DBDDL struct { + createDbLock *sync.Mutex +} // CreateDatabase implements the engine.DBDDLPlugin interface func (plugin *DBDDL) CreateDatabase(ctx context.Context, name string) error { @@ -39,6 +42,8 @@ func (plugin *DBDDL) CreateDatabase(ctx context.Context, name string) error { Name: "0", }}, } + plugin.createDbLock.Lock() + defer plugin.createDbLock.Unlock() return globalCreateDb(ctx, ks) } @@ -49,6 +54,6 @@ func (plugin *DBDDL) DropDatabase(ctx context.Context, name string) error { func init() { servenv.OnRun(func() { - engine.DBDDLRegister("vttest", &DBDDL{}) + engine.DBDDLRegister("vttest", &DBDDL{createDbLock: &sync.Mutex{}}) }) } From 7c5b840461614610fec2e02abc5562ae230ed1dc Mon Sep 17 00:00:00 2001 From: Jaskanwal Pawar <32091345+jspawar@users.noreply.github.com> Date: Wed, 17 May 2023 16:15:45 -0700 Subject: [PATCH 2/2] Apply suggestions from code review Co-authored-by: Arthur Schreiber Signed-off-by: Jaskanwal Pawar <32091345+jspawar@users.noreply.github.com> --- go/cmd/vtcombo/plugin_dbddl.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/go/cmd/vtcombo/plugin_dbddl.go b/go/cmd/vtcombo/plugin_dbddl.go index bbd5d1f7404..1a95e073308 100644 --- a/go/cmd/vtcombo/plugin_dbddl.go +++ b/go/cmd/vtcombo/plugin_dbddl.go @@ -31,7 +31,7 @@ var globalDropDb func(ctx context.Context, ksName string) error // DBDDL doesn't need to store any state - we use the global variables above instead type DBDDL struct { - createDbLock *sync.Mutex + mu sync.Mutex } // CreateDatabase implements the engine.DBDDLPlugin interface @@ -42,8 +42,8 @@ func (plugin *DBDDL) CreateDatabase(ctx context.Context, name string) error { Name: "0", }}, } - plugin.createDbLock.Lock() - defer plugin.createDbLock.Unlock() + plugin.mu.Lock() + defer plugin.mu.Unlock() return globalCreateDb(ctx, ks) } @@ -54,6 +54,6 @@ func (plugin *DBDDL) DropDatabase(ctx context.Context, name string) error { func init() { servenv.OnRun(func() { - engine.DBDDLRegister("vttest", &DBDDL{createDbLock: &sync.Mutex{}}) + engine.DBDDLRegister("vttest", &DBDDL{}) }) }