From 950c67ee45e363477a81f830cc7eb3132737c0a0 Mon Sep 17 00:00:00 2001 From: glorv Date: Mon, 17 Aug 2020 11:54:09 +0800 Subject: [PATCH 1/6] use path in WalkDir and Open Signed-off-by: glorv --- pkg/storage/local.go | 11 +++++++---- pkg/storage/s3.go | 8 ++++++-- pkg/storage/storage.go | 4 ++-- 3 files changed, 15 insertions(+), 8 deletions(-) diff --git a/pkg/storage/local.go b/pkg/storage/local.go index 06eae91f9..29dd51777 100644 --- a/pkg/storage/local.go +++ b/pkg/storage/local.go @@ -52,13 +52,16 @@ func (l *LocalStorage) WalkDir(ctx context.Context, fn func(string, int64) error return nil } - return fn(f.Name(), f.Size()) + return fn(path, f.Size()) }) } -// Open a Reader by file name. -func (l *LocalStorage) Open(ctx context.Context, name string) (ReadSeekCloser, error) { - return os.Open(path.Join(l.base, name)) +// Open a Reader by file path. +func (l *LocalStorage) Open(ctx context.Context, p string) (ReadSeekCloser, error) { + if !filepath.IsAbs(p) { + p = path.Join(l.base, p) + } + return os.Open(p) } func pathExists(_path string) (bool, error) { diff --git a/pkg/storage/s3.go b/pkg/storage/s3.go index 429e8b8d8..3e2c4c283 100644 --- a/pkg/storage/s3.go +++ b/pkg/storage/s3.go @@ -9,6 +9,7 @@ import ( "io" "io/ioutil" "net/url" + "strings" "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/aws/awserr" @@ -349,10 +350,13 @@ func (rs *S3Storage) Open(ctx context.Context, name string) (ReadSeekCloser, err }, nil } -func (rs *S3Storage) open(ctx context.Context, name string, startOffset int64, endOffset int64) (io.ReadCloser, error) { +func (rs *S3Storage) open(ctx context.Context, path string, startOffset int64, endOffset int64) (io.ReadCloser, error) { + if strings.Index(path, rs.options.Prefix) != 0 { + path = rs.options.Prefix + path + } input := &s3.GetObjectInput{ Bucket: aws.String(rs.options.Bucket), - Key: aws.String(rs.options.Prefix + name), + Key: aws.String(path), } var rangeOffset *string diff --git a/pkg/storage/storage.go b/pkg/storage/storage.go index 4e6ad6ca1..37f1e10c0 100644 --- a/pkg/storage/storage.go +++ b/pkg/storage/storage.go @@ -25,8 +25,8 @@ type ExternalStorage interface { Read(ctx context.Context, name string) ([]byte, error) // FileExists return true if file exists FileExists(ctx context.Context, name string) (bool, error) - // Open a Reader by file name. - Open(ctx context.Context, name string) (ReadSeekCloser, error) + // Open a Reader by file path. path can be either an absolute path or relative path to storage base path + Open(ctx context.Context, path string) (ReadSeekCloser, error) // WalkDir traverse all the files in a dir. // // fn is the function called for each regular file visited by WalkDir. From eee3d6d7e038c2ef84178d8f4b1c2f649698fea0 Mon Sep 17 00:00:00 2001 From: glorv Date: Mon, 17 Aug 2020 11:56:47 +0800 Subject: [PATCH 2/6] update comments Signed-off-by: glorv --- pkg/storage/gcs.go | 4 ++-- pkg/storage/noop.go | 4 ++-- pkg/storage/s3.go | 8 ++++---- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/pkg/storage/gcs.go b/pkg/storage/gcs.go index c41fc2e09..56c59d4e1 100644 --- a/pkg/storage/gcs.go +++ b/pkg/storage/gcs.go @@ -123,8 +123,8 @@ func (s *gcsStorage) FileExists(ctx context.Context, name string) (bool, error) return true, nil } -// Open a Reader by file name. -func (s *gcsStorage) Open(ctx context.Context, name string) (ReadSeekCloser, error) { +// Open a Reader by file path. +func (s *gcsStorage) Open(ctx context.Context, path string) (ReadSeekCloser, error) { // TODO, implement this if needed panic("Unsupported Operation") } diff --git a/pkg/storage/noop.go b/pkg/storage/noop.go index 83c7384ed..375c48d45 100644 --- a/pkg/storage/noop.go +++ b/pkg/storage/noop.go @@ -23,8 +23,8 @@ func (*noopStorage) FileExists(ctx context.Context, name string) (bool, error) { return false, nil } -// Open a Reader by file name. -func (*noopStorage) Open(ctx context.Context, name string) (ReadSeekCloser, error) { +// Open a Reader by file path. +func (*noopStorage) Open(ctx context.Context, path string) (ReadSeekCloser, error) { return noopReader{}, nil } diff --git a/pkg/storage/s3.go b/pkg/storage/s3.go index 3e2c4c283..e0ae3baa2 100644 --- a/pkg/storage/s3.go +++ b/pkg/storage/s3.go @@ -337,15 +337,15 @@ func (rs *S3Storage) WalkDir(ctx context.Context, fn func(string, int64) error) return nil } -// Open a Reader by file name. -func (rs *S3Storage) Open(ctx context.Context, name string) (ReadSeekCloser, error) { - reader, err := rs.open(ctx, name, 0, 0) +// Open a Reader by file path. +func (rs *S3Storage) Open(ctx context.Context, path string) (ReadSeekCloser, error) { + reader, err := rs.open(ctx, path, 0, 0) if err != nil { return nil, err } return &s3ObjectReader{ storage: rs, - name: name, + name: path, reader: reader, }, nil } From 6f18bb03f2276842041ac2893b9307e24e10a471 Mon Sep 17 00:00:00 2001 From: glorv Date: Mon, 17 Aug 2020 15:16:22 +0800 Subject: [PATCH 3/6] use filepath instead of path --- pkg/storage/local.go | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/pkg/storage/local.go b/pkg/storage/local.go index 29dd51777..3cf3de008 100644 --- a/pkg/storage/local.go +++ b/pkg/storage/local.go @@ -6,7 +6,6 @@ import ( "context" "io/ioutil" "os" - "path" "path/filepath" "github.com/pingcap/errors" @@ -20,19 +19,19 @@ type LocalStorage struct { } func (l *LocalStorage) Write(ctx context.Context, name string, data []byte) error { - filepath := path.Join(l.base, name) + filepath := filepath.Join(l.base, name) return ioutil.WriteFile(filepath, data, 0644) // nolint:gosec // the backupmeta file _is_ intended to be world-readable. } func (l *LocalStorage) Read(ctx context.Context, name string) ([]byte, error) { - filepath := path.Join(l.base, name) + filepath := filepath.Join(l.base, name) return ioutil.ReadFile(filepath) } // FileExists implement ExternalStorage.FileExists. func (l *LocalStorage) FileExists(ctx context.Context, name string) (bool, error) { - filepath := path.Join(l.base, name) + filepath := filepath.Join(l.base, name) return pathExists(filepath) } @@ -59,7 +58,7 @@ func (l *LocalStorage) WalkDir(ctx context.Context, fn func(string, int64) error // Open a Reader by file path. func (l *LocalStorage) Open(ctx context.Context, p string) (ReadSeekCloser, error) { if !filepath.IsAbs(p) { - p = path.Join(l.base, p) + p = filepath.Join(l.base, p) } return os.Open(p) } From a9a3cfe4f0d3325d2b8a890a8a5148dbdc9b1eaf Mon Sep 17 00:00:00 2001 From: glorv Date: Mon, 17 Aug 2020 17:52:17 +0800 Subject: [PATCH 4/6] force convert to relative path --- pkg/storage/local.go | 11 +++++------ pkg/storage/s3.go | 13 ++++--------- pkg/storage/storage.go | 2 +- 3 files changed, 10 insertions(+), 16 deletions(-) diff --git a/pkg/storage/local.go b/pkg/storage/local.go index 3cf3de008..6422c27d0 100644 --- a/pkg/storage/local.go +++ b/pkg/storage/local.go @@ -50,17 +50,16 @@ func (l *LocalStorage) WalkDir(ctx context.Context, fn func(string, int64) error if f == nil || f.IsDir() { return nil } - + // in mac osx, the path parameter is absolute path; in linux, the path is relative path to execution base dir, + // so use Rel to convert to relative path to l.base + path, _ = filepath.Rel(l.base, path) return fn(path, f.Size()) }) } -// Open a Reader by file path. +// Open a Reader by file path. path is a relative path to base path func (l *LocalStorage) Open(ctx context.Context, p string) (ReadSeekCloser, error) { - if !filepath.IsAbs(p) { - p = filepath.Join(l.base, p) - } - return os.Open(p) + return os.Open(filepath.Join(l.base, p)) } func pathExists(_path string) (bool, error) { diff --git a/pkg/storage/s3.go b/pkg/storage/s3.go index e0ae3baa2..c7759959f 100644 --- a/pkg/storage/s3.go +++ b/pkg/storage/s3.go @@ -6,11 +6,6 @@ import ( "bytes" "context" "fmt" - "io" - "io/ioutil" - "net/url" - "strings" - "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/aws/awserr" "github.com/aws/aws-sdk-go/aws/credentials" @@ -18,6 +13,9 @@ import ( "github.com/aws/aws-sdk-go/aws/session" "github.com/aws/aws-sdk-go/service/s3" "github.com/spf13/pflag" + "io" + "io/ioutil" + "net/url" "github.com/pingcap/errors" "github.com/pingcap/kvproto/pkg/backup" @@ -351,12 +349,9 @@ func (rs *S3Storage) Open(ctx context.Context, path string) (ReadSeekCloser, err } func (rs *S3Storage) open(ctx context.Context, path string, startOffset int64, endOffset int64) (io.ReadCloser, error) { - if strings.Index(path, rs.options.Prefix) != 0 { - path = rs.options.Prefix + path - } input := &s3.GetObjectInput{ Bucket: aws.String(rs.options.Bucket), - Key: aws.String(path), + Key: aws.String(rs.options.Prefix + path), } var rangeOffset *string diff --git a/pkg/storage/storage.go b/pkg/storage/storage.go index 37f1e10c0..7602e12f1 100644 --- a/pkg/storage/storage.go +++ b/pkg/storage/storage.go @@ -25,7 +25,7 @@ type ExternalStorage interface { Read(ctx context.Context, name string) ([]byte, error) // FileExists return true if file exists FileExists(ctx context.Context, name string) (bool, error) - // Open a Reader by file path. path can be either an absolute path or relative path to storage base path + // Open a Reader by file path. path is relative path to storage base path Open(ctx context.Context, path string) (ReadSeekCloser, error) // WalkDir traverse all the files in a dir. // From e4a61c8d6aa7bbb50883554e96e7edb431629361 Mon Sep 17 00:00:00 2001 From: glorv Date: Mon, 17 Aug 2020 17:55:02 +0800 Subject: [PATCH 5/6] fix import --- pkg/storage/s3.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/pkg/storage/s3.go b/pkg/storage/s3.go index c7759959f..dcbdc5cd7 100644 --- a/pkg/storage/s3.go +++ b/pkg/storage/s3.go @@ -6,6 +6,10 @@ import ( "bytes" "context" "fmt" + "io" + "io/ioutil" + "net/url" + "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/aws/awserr" "github.com/aws/aws-sdk-go/aws/credentials" @@ -13,9 +17,6 @@ import ( "github.com/aws/aws-sdk-go/aws/session" "github.com/aws/aws-sdk-go/service/s3" "github.com/spf13/pflag" - "io" - "io/ioutil" - "net/url" "github.com/pingcap/errors" "github.com/pingcap/kvproto/pkg/backup" From 0ad9cfdbf1622d8bf399d87e406f4ddc1ccfcfc3 Mon Sep 17 00:00:00 2001 From: glorv Date: Mon, 17 Aug 2020 17:57:08 +0800 Subject: [PATCH 6/6] fix go doc --- pkg/storage/local.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/storage/local.go b/pkg/storage/local.go index 6422c27d0..27f8ed53e 100644 --- a/pkg/storage/local.go +++ b/pkg/storage/local.go @@ -57,9 +57,9 @@ func (l *LocalStorage) WalkDir(ctx context.Context, fn func(string, int64) error }) } -// Open a Reader by file path. path is a relative path to base path -func (l *LocalStorage) Open(ctx context.Context, p string) (ReadSeekCloser, error) { - return os.Open(filepath.Join(l.base, p)) +// Open a Reader by file path, path is a relative path to base path. +func (l *LocalStorage) Open(ctx context.Context, path string) (ReadSeekCloser, error) { + return os.Open(filepath.Join(l.base, path)) } func pathExists(_path string) (bool, error) {