From 55032ffdf9df1f9f5288a61b22d638a09ced21b0 Mon Sep 17 00:00:00 2001 From: Harshavardhana Date: Mon, 25 Apr 2016 00:27:04 -0700 Subject: [PATCH] xl: Simplify blockingWriter and its usage. (#1373) This removes odd races since we don't need to track errors and avoids locking. All we need is a Wait() and Done() waitgroup. --- xl-v1-blockingwriter.go | 24 +++--------------------- xl-v1-createfile.go | 13 +++---------- 2 files changed, 6 insertions(+), 31 deletions(-) diff --git a/xl-v1-blockingwriter.go b/xl-v1-blockingwriter.go index e92c00316..d5b4097ab 100644 --- a/xl-v1-blockingwriter.go +++ b/xl-v1-blockingwriter.go @@ -25,43 +25,26 @@ import ( type blockingWriteCloser struct { writer io.WriteCloser // Embedded writer. release *sync.WaitGroup // Waitgroup for atomicity. - mutex *sync.Mutex // Mutex for thread safety. err error } // Write to the underlying writer. func (b *blockingWriteCloser) Write(data []byte) (int, error) { - n, err := b.writer.Write(data) - if err != nil { - b.mutex.Lock() - b.err = err - b.mutex.Unlock() - } - return n, b.err + return b.writer.Write(data) } // Close blocks until another goroutine calls Release(error). Returns // error code if either writer fails or Release is called with an error. func (b *blockingWriteCloser) Close() error { err := b.writer.Close() - if err != nil { - b.mutex.Lock() - b.err = err - b.mutex.Unlock() - } b.release.Wait() - return b.err + return err } // Release the Close, causing it to unblock. Only call this // once. Calling it multiple times results in a panic. -func (b *blockingWriteCloser) Release(err error) { +func (b *blockingWriteCloser) Release() { b.release.Done() - if err != nil { - b.mutex.Lock() - b.err = err - b.mutex.Unlock() - } return } @@ -74,7 +57,6 @@ func newBlockingWriteCloser(writer io.WriteCloser) *blockingWriteCloser { wg.Add(1) return &blockingWriteCloser{ writer: writer, - mutex: &sync.Mutex{}, release: wg, } } diff --git a/xl-v1-createfile.go b/xl-v1-createfile.go index 1726b9704..003a2bb15 100644 --- a/xl-v1-createfile.go +++ b/xl-v1-createfile.go @@ -129,6 +129,9 @@ func (xl XL) getFileQuorumVersionMap(volume, path string) map[int]int64 { // WriteErasure reads predefined blocks, encodes them and writes to // configured storage disks. func (xl XL) writeErasure(volume, path string, reader *io.PipeReader, bwriter *blockingWriteCloser) { + // Release the block writer upon function return. + defer bwriter.Release() + // Get available quorum for existing file path. _, higherVersion := xl.getQuorumDisks(volume, path) // Increment to have next higher version. @@ -163,7 +166,6 @@ func (xl XL) writeErasure(volume, path string, reader *io.PipeReader, bwriter *b // Remove previous temp writers for any failure. xl.cleanupCreateFileOps(volume, path, append(writers, metadataWriters...)...) reader.CloseWithError(errWriteQuorum) - bwriter.Release(errWriteQuorum) return } @@ -186,7 +188,6 @@ func (xl XL) writeErasure(volume, path string, reader *io.PipeReader, bwriter *b // Remove previous temp writers for any failure. xl.cleanupCreateFileOps(volume, path, append(writers, metadataWriters...)...) reader.CloseWithError(errWriteQuorum) - bwriter.Release(errWriteQuorum) return } @@ -207,7 +208,6 @@ func (xl XL) writeErasure(volume, path string, reader *io.PipeReader, bwriter *b // Remove all temp writers. xl.cleanupCreateFileOps(volume, path, append(writers, metadataWriters...)...) reader.CloseWithError(err) - bwriter.Release(err) return } } @@ -223,7 +223,6 @@ func (xl XL) writeErasure(volume, path string, reader *io.PipeReader, bwriter *b // Remove all temp writers. xl.cleanupCreateFileOps(volume, path, append(writers, metadataWriters...)...) reader.CloseWithError(err) - bwriter.Release(err) return } @@ -233,7 +232,6 @@ func (xl XL) writeErasure(volume, path string, reader *io.PipeReader, bwriter *b // Remove all temp writers upon error. xl.cleanupCreateFileOps(volume, path, append(writers, metadataWriters...)...) reader.CloseWithError(err) - bwriter.Release(err) return } @@ -248,7 +246,6 @@ func (xl XL) writeErasure(volume, path string, reader *io.PipeReader, bwriter *b // Remove all temp writers upon error. xl.cleanupCreateFileOps(volume, path, append(writers, metadataWriters...)...) reader.CloseWithError(err) - bwriter.Release(err) return } if sha512Writers[index] != nil { @@ -298,7 +295,6 @@ func (xl XL) writeErasure(volume, path string, reader *io.PipeReader, bwriter *b // Remove temporary files. xl.cleanupCreateFileOps(volume, path, append(writers, metadataWriters...)...) reader.CloseWithError(err) - bwriter.Release(err) return } } @@ -322,9 +318,6 @@ func (xl XL) writeErasure(volume, path string, reader *io.PipeReader, bwriter *b metadataWriters[index].Close() } - // Release the blocking writer. - bwriter.Release(nil) - // Close the pipe reader and return. reader.Close() return