From 42286cba70b1439cebd4d24835dfbc035b5540c8 Mon Sep 17 00:00:00 2001 From: Harshavardhana Date: Sat, 25 Jun 2016 14:51:06 -0700 Subject: [PATCH] XL: Implement new ReadAll API for files which are read in single call. (#1974) Add a unit test as well. --- format-config-v1.go | 10 ++--- fs-v1-metadata.go | 15 +++---- fs-v1.go | 13 ++---- posix.go | 65 +++++++++++++++++++++++++++ posix_test.go | 92 +++++++++++++++++++++++++++++++++++++++ rpc-client.go | 14 ++++++ rpc-server-datatypes.go | 9 ++++ rpc-server.go | 10 +++++ storage-interface.go | 3 ++ xl-v1-multipart-common.go | 13 ++---- xl-v1-utils.go | 15 ++----- 11 files changed, 211 insertions(+), 48 deletions(-) create mode 100644 posix_test.go diff --git a/format-config-v1.go b/format-config-v1.go index 32c0d9aa5..796293fed 100644 --- a/format-config-v1.go +++ b/format-config-v1.go @@ -17,7 +17,6 @@ package main import ( - "bytes" "encoding/json" "errors" "fmt" @@ -347,10 +346,8 @@ func reorderDisks(bootstrapDisks []StorageAPI, formatConfigs []*formatConfigV1) // loadFormat - loads format.json from disk. func loadFormat(disk StorageAPI) (format *formatConfigV1, err error) { - // Allocate staging buffer of 32KiB for copyBuffer. - buf := make([]byte, 32*1024) - var buffer = new(bytes.Buffer) - if err = copyBuffer(buffer, disk, minioMetaBucket, formatConfigFile, buf); err != nil { + buf, err := disk.ReadAll(minioMetaBucket, formatConfigFile) + if err != nil { // 'file not found' and 'volume not found' as // same. 'volume not found' usually means its a fresh disk. if err == errFileNotFound || err == errVolumeNotFound { @@ -371,8 +368,7 @@ func loadFormat(disk StorageAPI) (format *formatConfigV1, err error) { // Try to decode format json into formatConfigV1 struct. format = &formatConfigV1{} - d := json.NewDecoder(buffer) - if err = d.Decode(format); err != nil { + if err = json.Unmarshal(buf, format); err != nil { return nil, err } diff --git a/fs-v1-metadata.go b/fs-v1-metadata.go index 902b5fb1d..d8b799ca5 100644 --- a/fs-v1-metadata.go +++ b/fs-v1-metadata.go @@ -1,7 +1,6 @@ package main import ( - "bytes" "encoding/json" "path" "sort" @@ -59,18 +58,14 @@ func (m *fsMetaV1) AddObjectPart(partNumber int, partName string, partETag strin // readFSMetadata - returns the object metadata `fs.json` content. func readFSMetadata(disk StorageAPI, bucket, object string) (fsMeta fsMetaV1, err error) { - // 32KiB staging buffer for copying `fs.json`. - var buf = make([]byte, 32*1024) - - // `fs.json` writer. - var buffer = new(bytes.Buffer) - if err = copyBuffer(buffer, disk, bucket, path.Join(object, fsMetaJSONFile), buf); err != nil { + // Read all `fs.json`. + buf, err := disk.ReadAll(bucket, path.Join(object, fsMetaJSONFile)) + if err != nil { return fsMetaV1{}, err } - // Decode `fs.json` into fsMeta structure. - d := json.NewDecoder(buffer) - if err = d.Decode(&fsMeta); err != nil { + // Decode `fs.json` into fsMeta structure. + if err = json.Unmarshal(buf, &fsMeta); err != nil { return fsMetaV1{}, err } diff --git a/fs-v1.go b/fs-v1.go index 973251b05..26a8169bb 100644 --- a/fs-v1.go +++ b/fs-v1.go @@ -17,7 +17,6 @@ package main import ( - "bytes" "crypto/md5" "encoding/hex" "encoding/json" @@ -48,20 +47,14 @@ func initFormatFS(storageDisk StorageAPI) error { // loads format.json from minioMetaBucket if it exists. func loadFormatFS(storageDisk StorageAPI) (format formatConfigV1, err error) { - // Allocate 32k buffer, this is sufficient for the most of `format.json`. - buf := make([]byte, 32*1024) - - // Allocate a new `format.json` buffer writer. - var buffer = new(bytes.Buffer) - // Reads entire `format.json`. - if err = copyBuffer(buffer, storageDisk, minioMetaBucket, fsFormatJSONFile, buf); err != nil { + buf, err := storageDisk.ReadAll(minioMetaBucket, fsFormatJSONFile) + if err != nil { return formatConfigV1{}, err } // Unmarshal format config. - d := json.NewDecoder(buffer) - if err = d.Decode(&format); err != nil { + if err = json.Unmarshal(buf, &format); err != nil { return formatConfigV1{}, err } diff --git a/posix.go b/posix.go index d58652f98..20071be45 100644 --- a/posix.go +++ b/posix.go @@ -20,6 +20,7 @@ import ( "bytes" "errors" "io" + "io/ioutil" "os" slashpath "path" "path/filepath" @@ -352,6 +353,67 @@ func (s *posix) ListDir(volume, dirPath string) (entries []string, err error) { return readDir(pathJoin(volumeDir, dirPath)) } +// ReadAll reads from r until an error or EOF and returns the data it read. +// A successful call returns err == nil, not err == EOF. Because ReadAll is +// defined to read from src until EOF, it does not treat an EOF from Read +// as an error to be reported. +// This API is meant to be used on files which have small memory footprint, do +// not use this on large files as it would cause server to crash. +func (s *posix) ReadAll(volume, path string) (buf []byte, err error) { + defer func() { + if err == syscall.EIO { + atomic.AddInt32(&s.ioErrCount, 1) + } + }() + + if s.ioErrCount > maxAllowedIOError { + return nil, errFaultyDisk + } + // Validate if disk is free. + if err = checkDiskFree(s.diskPath, s.minFreeDisk); err != nil { + return nil, err + } + + volumeDir, err := s.getVolDir(volume) + if err != nil { + return nil, err + } + // Stat a volume entry. + _, err = os.Stat(preparePath(volumeDir)) + if err != nil { + if os.IsNotExist(err) { + return nil, errVolumeNotFound + } + return nil, err + } + + // Validate file path length, before reading. + filePath := pathJoin(volumeDir, path) + if err = checkPathLength(filePath); err != nil { + return nil, err + } + + // Open the file for reading. + buf, err = ioutil.ReadFile(preparePath(filePath)) + if err != nil { + if os.IsNotExist(err) { + return nil, errFileNotFound + } else if os.IsPermission(err) { + return nil, errFileAccessDenied + } else if pathErr, ok := err.(*os.PathError); ok { + if pathErr.Err == syscall.EISDIR { + return nil, errFileNotFound + } else if strings.Contains(pathErr.Err.Error(), "The handle is invalid") { + // This case is special and needs to be handled for windows. + return nil, errFileNotFound + } + return nil, pathErr + } + return nil, err + } + return buf, nil +} + // ReadFile reads exactly len(buf) bytes into buf. It returns the // number of bytes copied. The error is EOF only if no bytes were // read. On return, n == len(buf) if and only if err == nil. n == 0 @@ -386,10 +448,13 @@ func (s *posix) ReadFile(volume string, path string, offset int64, buf []byte) ( return 0, err } + // Validate effective path length before reading. filePath := pathJoin(volumeDir, path) if err = checkPathLength(filePath); err != nil { return 0, err } + + // Open the file for reading. file, err := os.Open(preparePath(filePath)) if err != nil { if os.IsNotExist(err) { diff --git a/posix_test.go b/posix_test.go new file mode 100644 index 000000000..7152b0a08 --- /dev/null +++ b/posix_test.go @@ -0,0 +1,92 @@ +/* + * Minio Cloud Storage, (C) 2016 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 main + +import ( + "io/ioutil" + "os" + "testing" +) + +// Tests the functionality implemented by ReadAll storage API. +func TestReadAll(t *testing.T) { + path, err := ioutil.TempDir(os.TempDir(), "minio-") + if err != nil { + t.Fatalf("Unable to create a temporary directory, %s", err) + } + defer removeAll(path) + + // Initialize posix storage layer. + posix, err := newPosix(path) + if err != nil { + t.Fatalf("Unable to initialize posix, %s", err) + } + + // Create files for the test cases. + if err = posix.MakeVol("exists"); err != nil { + t.Fatalf("Unable to create a volume \"exists\", %s", err) + } + if err = posix.AppendFile("exists", "as-directory/as-file", []byte("Hello, World")); err != nil { + t.Fatalf("Unable to create a file \"as-directory/as-file\", %s", err) + } + if err = posix.AppendFile("exists", "as-file", []byte("Hello, World")); err != nil { + t.Fatalf("Unable to create a file \"as-file\", %s", err) + } + + // Testcases to validate different conditions for ReadAll API. + testCases := []struct { + volume string + path string + err error + }{ + // Validate volume does not exist. + { + "i-dont-exist", + "", + errVolumeNotFound, + }, + // Validate bad condition file does not exist. + { + "exists", + "as-file-not-found", + errFileNotFound, + }, + // Validate bad condition file exists as prefix/directory and + // we are attempting to read it. + { + "exists", + "as-directory", + errFileNotFound, + }, + // Validate the good condition file exists and we are able to + // read it. + { + "exists", + "as-file", + nil, + }, + // Add more cases here. + } + + // Run through all the test cases and validate for ReadAll. + for i, testCase := range testCases { + _, err = posix.ReadAll(testCase.volume, testCase.path) + if err != testCase.err { + t.Errorf("Test %d expected err %s, got err %s", i+1, testCase.err, err) + } + } +} diff --git a/rpc-client.go b/rpc-client.go index 81b295592..5256952f9 100644 --- a/rpc-client.go +++ b/rpc-client.go @@ -168,6 +168,20 @@ func (n networkStorage) StatFile(volume, path string) (fileInfo FileInfo, err er return fileInfo, nil } +// ReadAll - reads entire contents of the file at path until EOF, retuns the +// contents in a byte slice. Returns buf == nil if err != nil. +// This API is meant to be used on files which have small memory footprint, do +// not use this on large files as it would cause server to crash. +func (n networkStorage) ReadAll(volume, path string) (buf []byte, err error) { + if err = n.rpcClient.Call("Storage.ReadAllHandler", ReadAllArgs{ + Vol: volume, + Path: path, + }, &buf); err != nil { + return nil, toStorageErr(err) + } + return buf, nil +} + // ReadFile - reads a file. func (n networkStorage) ReadFile(volume string, path string, offset int64, buffer []byte) (m int64, err error) { if err = n.rpcClient.Call("Storage.ReadFileHandler", ReadFileArgs{ diff --git a/rpc-server-datatypes.go b/rpc-server-datatypes.go index 779d17d9b..85b501901 100644 --- a/rpc-server-datatypes.go +++ b/rpc-server-datatypes.go @@ -28,6 +28,15 @@ type ListVolsReply struct { Vols []VolInfo } +// ReadAllArgs represents read all RPC arguments. +type ReadAllArgs struct { + // Name of the volume. + Vol string + + // Name of the path. + Path string +} + // ReadFileArgs represents read file RPC arguments. type ReadFileArgs struct { // Name of the volume. diff --git a/rpc-server.go b/rpc-server.go index 2df8cecc4..7550064a2 100644 --- a/rpc-server.go +++ b/rpc-server.go @@ -75,6 +75,16 @@ func (s *storageServer) ListDirHandler(arg *ListDirArgs, reply *[]string) error return nil } +// ReadAllHandler - read all handler is rpc wrapper to read all storage API. +func (s *storageServer) ReadAllHandler(arg *ReadFileArgs, reply *[]byte) error { + buf, err := s.storage.ReadAll(arg.Vol, arg.Path) + if err != nil { + return err + } + reply = &buf + return nil +} + // ReadFileHandler - read file handler is rpc wrapper to read file. func (s *storageServer) ReadFileHandler(arg *ReadFileArgs, reply *int64) error { n, err := s.storage.ReadFile(arg.Vol, arg.Path, arg.Offset, arg.Buffer) diff --git a/storage-interface.go b/storage-interface.go index da222c66c..f3f6fb113 100644 --- a/storage-interface.go +++ b/storage-interface.go @@ -31,4 +31,7 @@ type StorageAPI interface { RenameFile(srcVolume, srcPath, dstVolume, dstPath string) error StatFile(volume string, path string) (file FileInfo, err error) DeleteFile(volume string, path string) (err error) + + // Read all. + ReadAll(volume string, path string) (buf []byte, err error) } diff --git a/xl-v1-multipart-common.go b/xl-v1-multipart-common.go index 1ece15f80..b5e5cdb74 100644 --- a/xl-v1-multipart-common.go +++ b/xl-v1-multipart-common.go @@ -17,7 +17,6 @@ package main import ( - "bytes" "encoding/json" "path" "sort" @@ -70,21 +69,15 @@ func (u uploadsV1) Index(uploadID string) int { // readUploadsJSON - get all the saved uploads JSON. func readUploadsJSON(bucket, object string, disk StorageAPI) (uploadIDs uploadsV1, err error) { - // Staging buffer of 128KiB kept for reading `uploads.json`. - var buf = make([]byte, 128*1024) - - // Writer holding `uploads.json` content. - var buffer = new(bytes.Buffer) - uploadJSONPath := path.Join(mpartMetaPrefix, bucket, object, uploadsJSONFile) // Reads entire `uploads.json`. - if err = copyBuffer(buffer, disk, minioMetaBucket, uploadJSONPath, buf); err != nil { + buf, err := disk.ReadAll(minioMetaBucket, uploadJSONPath) + if err != nil { return uploadsV1{}, err } // Decode `uploads.json`. - d := json.NewDecoder(buffer) - if err = d.Decode(&uploadIDs); err != nil { + if err = json.Unmarshal(buf, &uploadIDs); err != nil { return uploadsV1{}, err } diff --git a/xl-v1-utils.go b/xl-v1-utils.go index 394eccfa9..deae95a42 100644 --- a/xl-v1-utils.go +++ b/xl-v1-utils.go @@ -17,7 +17,6 @@ package main import ( - "bytes" "encoding/json" "math/rand" "path" @@ -64,22 +63,16 @@ func randInts(count int) []int { return ints } -// readXLMeta reads `xl.json` returns contents as byte array. +// readXLMeta reads `xl.json` and returns back XL metadata structure. func readXLMeta(disk StorageAPI, bucket string, object string) (xlMeta xlMetaV1, err error) { - // Allocate 32k buffer, this is sufficient for the most of `xl.json`. - buf := make([]byte, 128*1024) - - // Allocate a new `xl.json` buffer writer. - var buffer = new(bytes.Buffer) - // Reads entire `xl.json`. - if err = copyBuffer(buffer, disk, bucket, path.Join(object, xlMetaJSONFile), buf); err != nil { + buf, err := disk.ReadAll(bucket, path.Join(object, xlMetaJSONFile)) + if err != nil { return xlMetaV1{}, err } // Unmarshal xl metadata. - d := json.NewDecoder(buffer) - if err = d.Decode(&xlMeta); err != nil { + if err = json.Unmarshal(buf, &xlMeta); err != nil { return xlMetaV1{}, err }