Skip to content

Commit

Permalink
Write to client store while holding lock
Browse files Browse the repository at this point in the history
Protect against a race where destroying and persist state goroutines
race.

The downside is that the database io operation will run while holding
the lock and may run indefinitely.  The risk of lock being long held is
slow destruction, but slow io has bigger problems.
  • Loading branch information
Mahmood Ali committed Aug 26, 2019
1 parent a80643e commit ff3dedd
Showing 1 changed file with 9 additions and 8 deletions.
17 changes: 9 additions & 8 deletions client/allocrunner/alloc_runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -765,14 +765,15 @@ func (ar *allocRunner) destroyImpl() {
// state if Run() ran at all.
<-ar.taskStateUpdateHandlerCh

// Cleanup state db
// Mark alloc as destroyed
ar.destroyedLock.Lock()

// Cleanup state db; while holding the lock to avoid
// a race periodic PersistState that may resurrect the alloc
if err := ar.stateDB.DeleteAllocationBucket(ar.id); err != nil {
ar.logger.Warn("failed to delete allocation state", "error", err)
}

// Mark alloc as destroyed
ar.destroyedLock.Lock()

if !ar.shutdown {
ar.shutdown = true
close(ar.shutdownCh)
Expand All @@ -785,10 +786,10 @@ func (ar *allocRunner) destroyImpl() {
}

func (ar *allocRunner) PersistState() error {
// note that a race exists where a goroutine attempts to persist state
// while another kicks off destruction process.
// Here, we attempt to reconcile by always deleting alloc bucket after alloc destruction
if ar.IsDestroyed() {
ar.destroyedLock.Lock()
defer ar.destroyedLock.Unlock()

if ar.destroyed {
err := ar.stateDB.DeleteAllocationBucket(ar.id)
if err != nil {
ar.logger.Warn("failed to delete allocation bucket", "error", err)
Expand Down

0 comments on commit ff3dedd

Please sign in to comment.