Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: because lock is not release, drop cutover sentry table is hanged #1180

Merged
merged 4 commits into from
Feb 6, 2023
Merged

fix: because lock is not release, drop cutover sentry table is hanged #1180

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
321e584
fix: because lock is not release, drop cutover sentry table is hanged
lmtwga Sep 16, 2022
659e440
Merge branch 'master' into master
dm-2 Oct 21, 2022
b538733
Merge branch 'master' into master
dm-2 Oct 21, 2022
bea5323
Merge branch 'master' into master
timvaillancourt Jan 27, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 6 additions & 8 deletions go/logic/applier.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import (
gosql "database/sql"
"fmt"
"strings"
"sync"
"sync/atomic"
"time"

Expand Down Expand Up @@ -935,7 +934,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) error {
tx, err := this.db.Begin()
if err != nil {
tableLocked <- err
Expand All @@ -946,6 +945,7 @@ func (this *Applier) AtomicCutOverMagicLock(sessionIdChan chan int64, tableLocke
tableLocked <- fmt.Errorf("Unexpected error in AtomicCutOverMagicLock(), injected to release blocking channel reads")
tableUnlocked <- fmt.Errorf("Unexpected error in AtomicCutOverMagicLock(), injected to release blocking channel reads")
tx.Rollback()
this.DropAtomicCutOverSentryTableIfExists()
}()

var sessionId int64
Expand Down Expand Up @@ -1014,12 +1014,10 @@ func (this *Applier) AtomicCutOverMagicLock(sessionIdChan chan int64, tableLocke
sql.EscapeName(this.migrationContext.GetOldTableName()),
)

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`!
}
})
if _, err := tx.Exec(query); err != nil {
this.migrationContext.Log.Errore(err)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be better to execute this query with a timeout and exit the program if it fails. While this PR should change ghost so that it will always successfully unlock the tables, in the event that there is some other unforseen race condition or deadlock here, then stopping program execution would be a guaranteed way to at least release all locks held by this program.

It would have the unfortunate side effect of leaving the _del table in existence which would need to be cleaned up later, but IMO that's operationally better than holding table locks which will cause any application actively trying to use the tables to stall.

// We DO NOT return here because we must `UNLOCK TABLES`!
}

// Tables still locked
this.migrationContext.Log.Infof("Releasing lock from %s.%s, %s.%s",
Expand Down
7 changes: 1 addition & 6 deletions go/logic/migrator.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import (
"math"
"os"
"strings"
"sync"
"sync/atomic"
"time"

Expand Down Expand Up @@ -639,12 +638,8 @@ 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
dropCutOverSentryTableOnce.Do(func() {
this.applier.DropAtomicCutOverSentryTableIfExists()
})
}()

atomic.StoreInt64(&this.migrationContext.AllEventsUpToLockProcessedInjectedFlag, 0)
Expand All @@ -653,7 +648,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); err != nil {
this.migrationContext.Log.Errore(err)
}
}()
Expand Down