From 52cd23ad9f0ceb5e58223a237c08dc40aac94d7c Mon Sep 17 00:00:00 2001 From: Harshavardhana Date: Tue, 7 Jul 2015 12:29:08 -0700 Subject: [PATCH] Move atomic file writes into its own package, use them inside quick and disk packages --- pkg/donut/disk/disk.go | 43 +++---------------- pkg/quick/quick.go | 8 +++- pkg/utils/atomic/atomic.go | 74 +++++++++++++++++++++++++++++++++ pkg/utils/atomic/atomic_test.go | 58 ++++++++++++++++++++++++++ 4 files changed, 143 insertions(+), 40 deletions(-) create mode 100644 pkg/utils/atomic/atomic.go create mode 100644 pkg/utils/atomic/atomic_test.go diff --git a/pkg/donut/disk/disk.go b/pkg/donut/disk/disk.go index 6db2bc962..f899f8ed0 100644 --- a/pkg/donut/disk/disk.go +++ b/pkg/donut/disk/disk.go @@ -18,7 +18,6 @@ package disk import ( "fmt" - "io/ioutil" "os" "path/filepath" "strconv" @@ -27,6 +26,7 @@ import ( "syscall" "github.com/minio/minio/pkg/iodine" + "github.com/minio/minio/pkg/utils/atomic" ) // Disk container for disk parameters @@ -36,36 +36,6 @@ type Disk struct { fsInfo map[string]string } -// File container provided by disk for atomic file writes -type File struct { - *os.File - path string -} - -// Close the file replacing the configured file. -func (f *File) Close() error { - if err := f.File.Close(); err != nil { - return err - } - if err := os.Rename(f.Name(), f.path); err != nil { - return err - } - return nil -} - -// Abort closes the file and removes it instead of replacing the configured -// file. This is useful if after starting to write to the file you decide you -// don't want it anymore. -func (f *File) Abort() error { - if err := f.File.Close(); err != nil { - return err - } - if err := os.Remove(f.Name()); err != nil { - return err - } - return nil -} - // New - instantiate new disk func New(diskPath string) (Disk, error) { if diskPath == "" { @@ -175,7 +145,7 @@ func (disk Disk) ListFiles(dirname string) ([]os.FileInfo, error) { } // CreateFile - create a file inside disk root path, replies with custome disk.File which provides atomic writes -func (disk Disk) CreateFile(filename string) (*File, error) { +func (disk Disk) CreateFile(filename string) (*atomic.File, error) { disk.lock.Lock() defer disk.lock.Unlock() @@ -188,16 +158,13 @@ func (disk Disk) CreateFile(filename string) (*File, error) { if err := os.MkdirAll(filepath.Dir(filePath), 0700); err != nil { return nil, iodine.New(err, nil) } - f, err := ioutil.TempFile(filepath.Dir(filePath), filepath.Base(filePath)) + + f, err := atomic.FileCreate(filePath) if err != nil { return nil, iodine.New(err, nil) } - if err := os.Chmod(f.Name(), 0600); err != nil { - os.Remove(f.Name()) - return nil, iodine.New(err, nil) - } - return &File{File: f, path: filePath}, nil + return f, nil } // OpenFile - read a file inside disk root path diff --git a/pkg/quick/quick.go b/pkg/quick/quick.go index 70760c7c6..ad77f255e 100644 --- a/pkg/quick/quick.go +++ b/pkg/quick/quick.go @@ -31,6 +31,7 @@ import ( "github.com/fatih/structs" "github.com/minio/minio/pkg/iodine" + "github.com/minio/minio/pkg/utils/atomic" ) // Config - generic config interface functions @@ -115,11 +116,10 @@ func (d config) Save(filename string) (err error) { return iodine.New(err, nil) } - file, err := os.OpenFile(filename, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0600) + file, err := atomic.FileCreate(filename) if err != nil { return iodine.New(err, nil) } - defer file.Close() if runtime.GOOS == "windows" { jsonData = []byte(strings.Replace(string(jsonData), "\n", "\r\n", -1)) @@ -128,6 +128,10 @@ func (d config) Save(filename string) (err error) { if err != nil { return iodine.New(err, nil) } + + if err := file.Close(); err != nil { + return iodine.New(err, nil) + } return nil } diff --git a/pkg/utils/atomic/atomic.go b/pkg/utils/atomic/atomic.go new file mode 100644 index 000000000..6b6368aa5 --- /dev/null +++ b/pkg/utils/atomic/atomic.go @@ -0,0 +1,74 @@ +/* + * Minio Client (C) 2015 Minio, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +// NOTE - Rename() not guaranteed to be atomic on all filesystems which are not fully POSIX compatible + +// Package atomic provides atomic file write semantics by leveraging Rename's() atomicity. +package atomic + +import ( + "io/ioutil" + "os" + "path/filepath" + + "github.com/minio/minio/pkg/iodine" +) + +// File container provided for atomic file writes +type File struct { + *os.File + file string +} + +// Close the file replacing, returns an error if any +func (f *File) Close() error { + // close the embedded fd + if err := f.File.Close(); err != nil { + return iodine.New(err, nil) + } + // atomic rename to final destination + if err := os.Rename(f.Name(), f.file); err != nil { + return iodine.New(err, nil) + } + return nil +} + +// CloseAndPurge removes the temp file, closes the transaction and returns an error if any +func (f *File) CloseAndPurge() error { + // close the embedded fd + if err := f.File.Close(); err != nil { + return iodine.New(err, nil) + } + if err := os.Remove(f.Name()); err != nil { + return err + } + return nil +} + +// FileCreate creates a new file at filePath for atomic writes +func FileCreate(filePath string) (*File, error) { + f, err := ioutil.TempFile(filepath.Dir(filePath), filepath.Base(filePath)) + if err != nil { + return nil, iodine.New(err, nil) + } + if err := os.Chmod(f.Name(), 0600); err != nil { + if err := os.Remove(f.Name()); err != nil { + return nil, iodine.New(err, nil) + } + return nil, iodine.New(err, nil) + } + return &File{File: f, file: filePath}, nil +} diff --git a/pkg/utils/atomic/atomic_test.go b/pkg/utils/atomic/atomic_test.go new file mode 100644 index 000000000..31871b75c --- /dev/null +++ b/pkg/utils/atomic/atomic_test.go @@ -0,0 +1,58 @@ +/* + * Minio Client (C) 2015 Minio, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package atomic + +import ( + "io/ioutil" + "os" + "path/filepath" + "testing" + + . "github.com/minio/check" +) + +func Test(t *testing.T) { TestingT(t) } + +type MySuite struct{} + +var _ = Suite(&MySuite{}) + +func (s *MySuite) TestAtomic(c *C) { + root, err := ioutil.TempDir("/tmp", "atomic-") + c.Assert(err, IsNil) + f, err := FileCreate(filepath.Join(root, "testfile")) + c.Assert(err, IsNil) + _, err = os.Stat(filepath.Join(root, "testfile")) + c.Assert(err, Not(IsNil)) + err = f.Close() + c.Assert(err, IsNil) + _, err = os.Stat(filepath.Join(root, "testfile")) + c.Assert(err, IsNil) +} + +func (s *MySuite) TestAtomicPurge(c *C) { + root, err := ioutil.TempDir("/tmp", "atomic-") + c.Assert(err, IsNil) + f, err := FileCreate(filepath.Join(root, "testfile")) + c.Assert(err, IsNil) + _, err = os.Stat(filepath.Join(root, "testfile")) + c.Assert(err, Not(IsNil)) + err = f.CloseAndPurge() + c.Assert(err, IsNil) + err = f.Close() + c.Assert(err, Not(IsNil)) +}