From 49262c523a81b231d032aa3da6a83fec672eab64 Mon Sep 17 00:00:00 2001 From: CLJ Date: Tue, 11 Jun 2019 12:41:31 +0800 Subject: [PATCH 1/5] use mutex to avoid to send drop cutover sentry table to mysql twice --- go/logic/applier.go | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/go/logic/applier.go b/go/logic/applier.go index 5fb795bac..9ad1d052d 100644 --- a/go/logic/applier.go +++ b/go/logic/applier.go @@ -18,6 +18,7 @@ import ( "github.com/outbrain/golib/log" "github.com/outbrain/golib/sqlutils" + "sync" ) const ( @@ -52,11 +53,12 @@ func newDmlBuildResultError(err error) *dmlBuildResult { // Applier is the one to actually write row data and apply binlog events onto the ghost table. // It is where the ghost & changelog tables get created. It is where the cut-over phase happens. type Applier struct { - connectionConfig *mysql.ConnectionConfig - db *gosql.DB - singletonDB *gosql.DB - migrationContext *base.MigrationContext - finishedMigrating int64 + connectionConfig *mysql.ConnectionConfig + db *gosql.DB + singletonDB *gosql.DB + migrationContext *base.MigrationContext + finishedMigrating int64 + dropAtomicCutOverSentryTableMutex sync.Mutex } func NewApplier(migrationContext *base.MigrationContext) *Applier { @@ -738,6 +740,9 @@ func (this *Applier) ExpectProcess(sessionId int64, stateHint, infoHint string) // DropAtomicCutOverSentryTableIfExists checks if the "old" table name // happens to be a cut-over magic table; if so, it drops it. func (this *Applier) DropAtomicCutOverSentryTableIfExists() error { + this.dropAtomicCutOverSentryTableMutex.Lock() + defer this.dropAtomicCutOverSentryTableMutex.Unlock() + log.Infof("Looking for magic cut-over table") tableName := this.migrationContext.GetOldTableName() rowMap := this.showTableStatus(tableName) @@ -854,6 +859,7 @@ func (this *Applier) AtomicCutOverMagicLock(sessionIdChan chan int64, tableLocke // The magic table is here because we locked it. And we are the only ones allowed to drop it. // And in fact, we will: + this.dropAtomicCutOverSentryTableMutex.Lock() log.Infof("Dropping magic cut-over table") query = fmt.Sprintf(`drop /* gh-ost */ table if exists %s.%s`, sql.EscapeName(this.migrationContext.DatabaseName), @@ -863,6 +869,7 @@ func (this *Applier) AtomicCutOverMagicLock(sessionIdChan chan int64, tableLocke log.Errore(err) // We DO NOT return here because we must `UNLOCK TABLES`! } + this.dropAtomicCutOverSentryTableMutex.Unlock() // Tables still locked log.Infof("Releasing lock from %s.%s, %s.%s", From 46211ee18d74b951df58571b2f0885e7165630d5 Mon Sep 17 00:00:00 2001 From: MOON_CLJ Date: Wed, 12 Jun 2019 00:21:20 +0800 Subject: [PATCH 2/5] Revert "use mutex to avoid to send drop cutover sentry table to mysql twice" This reverts commit 49262c523a81b231d032aa3da6a83fec672eab64. --- go/logic/applier.go | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/go/logic/applier.go b/go/logic/applier.go index 9ad1d052d..5fb795bac 100644 --- a/go/logic/applier.go +++ b/go/logic/applier.go @@ -18,7 +18,6 @@ import ( "github.com/outbrain/golib/log" "github.com/outbrain/golib/sqlutils" - "sync" ) const ( @@ -53,12 +52,11 @@ func newDmlBuildResultError(err error) *dmlBuildResult { // Applier is the one to actually write row data and apply binlog events onto the ghost table. // It is where the ghost & changelog tables get created. It is where the cut-over phase happens. type Applier struct { - connectionConfig *mysql.ConnectionConfig - db *gosql.DB - singletonDB *gosql.DB - migrationContext *base.MigrationContext - finishedMigrating int64 - dropAtomicCutOverSentryTableMutex sync.Mutex + connectionConfig *mysql.ConnectionConfig + db *gosql.DB + singletonDB *gosql.DB + migrationContext *base.MigrationContext + finishedMigrating int64 } func NewApplier(migrationContext *base.MigrationContext) *Applier { @@ -740,9 +738,6 @@ func (this *Applier) ExpectProcess(sessionId int64, stateHint, infoHint string) // DropAtomicCutOverSentryTableIfExists checks if the "old" table name // happens to be a cut-over magic table; if so, it drops it. func (this *Applier) DropAtomicCutOverSentryTableIfExists() error { - this.dropAtomicCutOverSentryTableMutex.Lock() - defer this.dropAtomicCutOverSentryTableMutex.Unlock() - log.Infof("Looking for magic cut-over table") tableName := this.migrationContext.GetOldTableName() rowMap := this.showTableStatus(tableName) @@ -859,7 +854,6 @@ func (this *Applier) AtomicCutOverMagicLock(sessionIdChan chan int64, tableLocke // The magic table is here because we locked it. And we are the only ones allowed to drop it. // And in fact, we will: - this.dropAtomicCutOverSentryTableMutex.Lock() log.Infof("Dropping magic cut-over table") query = fmt.Sprintf(`drop /* gh-ost */ table if exists %s.%s`, sql.EscapeName(this.migrationContext.DatabaseName), @@ -869,7 +863,6 @@ func (this *Applier) AtomicCutOverMagicLock(sessionIdChan chan int64, tableLocke log.Errore(err) // We DO NOT return here because we must `UNLOCK TABLES`! } - this.dropAtomicCutOverSentryTableMutex.Unlock() // Tables still locked log.Infof("Releasing lock from %s.%s, %s.%s", From 8405a71c1a62c4a3a7912169cb649fb68d09ad02 Mon Sep 17 00:00:00 2001 From: MOON_CLJ Date: Wed, 12 Jun 2019 00:41:14 +0800 Subject: [PATCH 3/5] use sync.Once to ensure drop cutover sentry table once --- go/logic/applier.go | 14 +++++++++----- go/logic/migrator.go | 8 ++++++-- 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/go/logic/applier.go b/go/logic/applier.go index 5fb795bac..67f5ecb8d 100644 --- a/go/logic/applier.go +++ b/go/logic/applier.go @@ -18,6 +18,7 @@ import ( "github.com/outbrain/golib/log" "github.com/outbrain/golib/sqlutils" + "sync" ) const ( @@ -781,7 +782,7 @@ func (this *Applier) CreateAtomicCutOverSentryTable() error { } // AtomicCutOverMagicLock -func (this *Applier) AtomicCutOverMagicLock(sessionIdChan chan int64, tableLocked chan<- error, okToUnlockTable <-chan bool, tableUnlocked chan<- error) error { +func (this *Applier) AtomicCutOverMagicLock(sessionIdChan chan int64, tableLocked chan<- error, okToUnlockTable <-chan bool, tableUnlocked chan<- error, dropCutOverSentryTableOnce sync.Once) error { tx, err := this.db.Begin() if err != nil { tableLocked <- err @@ -859,10 +860,13 @@ func (this *Applier) AtomicCutOverMagicLock(sessionIdChan chan int64, tableLocke sql.EscapeName(this.migrationContext.DatabaseName), sql.EscapeName(this.migrationContext.GetOldTableName()), ) - if _, err := tx.Exec(query); err != nil { - log.Errore(err) - // We DO NOT return here because we must `UNLOCK TABLES`! - } + + dropCutOverSentryTableOnce.Do(func() { + if _, err := tx.Exec(query); err != nil { + log.Errore(err) + // We DO NOT return here because we must `UNLOCK TABLES`! + } + }) // Tables still locked log.Infof("Releasing lock from %s.%s, %s.%s", diff --git a/go/logic/migrator.go b/go/logic/migrator.go index 86de5ac15..da48c2f80 100644 --- a/go/logic/migrator.go +++ b/go/logic/migrator.go @@ -11,6 +11,7 @@ import ( "math" "os" "strings" + "sync" "sync/atomic" "time" @@ -608,9 +609,12 @@ func (this *Migrator) atomicCutOver() (err error) { defer atomic.StoreInt64(&this.migrationContext.InCutOverCriticalSectionFlag, 0) okToUnlockTable := make(chan bool, 4) + var dropCutOverSentryTableOnce sync.Once defer func() { okToUnlockTable <- true - this.applier.DropAtomicCutOverSentryTableIfExists() + dropCutOverSentryTableOnce.Do(func() { + this.applier.DropAtomicCutOverSentryTableIfExists() + }) }() atomic.StoreInt64(&this.migrationContext.AllEventsUpToLockProcessedInjectedFlag, 0) @@ -619,7 +623,7 @@ func (this *Migrator) atomicCutOver() (err error) { tableLocked := make(chan error, 2) tableUnlocked := make(chan error, 2) go func() { - if err := this.applier.AtomicCutOverMagicLock(lockOriginalSessionIdChan, tableLocked, okToUnlockTable, tableUnlocked); err != nil { + if err := this.applier.AtomicCutOverMagicLock(lockOriginalSessionIdChan, tableLocked, okToUnlockTable, tableUnlocked, dropCutOverSentryTableOnce); err != nil { log.Errore(err) } }() From 43a3497a1857f5bdfe0db9126fd6665232b74021 Mon Sep 17 00:00:00 2001 From: CLJ Date: Wed, 12 Jun 2019 12:14:54 +0800 Subject: [PATCH 4/5] fix use *sync.Once --- go/logic/applier.go | 2 +- go/logic/migrator.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/go/logic/applier.go b/go/logic/applier.go index 67f5ecb8d..9e6ded655 100644 --- a/go/logic/applier.go +++ b/go/logic/applier.go @@ -782,7 +782,7 @@ func (this *Applier) CreateAtomicCutOverSentryTable() error { } // AtomicCutOverMagicLock -func (this *Applier) AtomicCutOverMagicLock(sessionIdChan chan int64, tableLocked chan<- error, okToUnlockTable <-chan bool, tableUnlocked chan<- error, dropCutOverSentryTableOnce sync.Once) error { +func (this *Applier) AtomicCutOverMagicLock(sessionIdChan chan int64, tableLocked chan<- error, okToUnlockTable <-chan bool, tableUnlocked chan<- error, dropCutOverSentryTableOnce *sync.Once) error { tx, err := this.db.Begin() if err != nil { tableLocked <- err diff --git a/go/logic/migrator.go b/go/logic/migrator.go index da48c2f80..cbb78efc7 100644 --- a/go/logic/migrator.go +++ b/go/logic/migrator.go @@ -623,7 +623,7 @@ func (this *Migrator) atomicCutOver() (err error) { tableLocked := make(chan error, 2) tableUnlocked := make(chan error, 2) go func() { - if err := this.applier.AtomicCutOverMagicLock(lockOriginalSessionIdChan, tableLocked, okToUnlockTable, tableUnlocked, dropCutOverSentryTableOnce); err != nil { + if err := this.applier.AtomicCutOverMagicLock(lockOriginalSessionIdChan, tableLocked, okToUnlockTable, tableUnlocked, &dropCutOverSentryTableOnce); err != nil { log.Errore(err) } }() From ad79e1bc12082ac47638c256caadf0ca9de014aa Mon Sep 17 00:00:00 2001 From: Tim Vaillancourt Date: Wed, 19 Aug 2020 22:24:07 +0200 Subject: [PATCH 5/5] Fix ci --- go/logic/applier.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/go/logic/applier.go b/go/logic/applier.go index 2960b3444..166d43d01 100644 --- a/go/logic/applier.go +++ b/go/logic/applier.go @@ -862,8 +862,8 @@ func (this *Applier) AtomicCutOverMagicLock(sessionIdChan chan int64, tableLocke dropCutOverSentryTableOnce.Do(func() { if _, err := tx.Exec(query); err != nil { - this.migrationContext.Log.Errore(err) - // We DO NOT return here because we must `UNLOCK TABLES`! + this.migrationContext.Log.Errore(err) + // We DO NOT return here because we must `UNLOCK TABLES`! } })