From 20a8a7da89a883390a96a91cac53ad83071934db Mon Sep 17 00:00:00 2001 From: bokket <3100563328@qq.com> Date: Mon, 6 Sep 2021 13:23:11 +0800 Subject: [PATCH 1/8] ci:Fix dependabot automatic update merge error Signed-off-by: bokket <3100563328@qq.com> --- .github/workflows/intergration-test.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/.github/workflows/intergration-test.yml b/.github/workflows/intergration-test.yml index 542c7bd..29fffe5 100644 --- a/.github/workflows/intergration-test.yml +++ b/.github/workflows/intergration-test.yml @@ -13,7 +13,6 @@ jobs: hdfs-version: - 3.2.2 - 3.3.0 - - 3.3.1 os: [ubuntu-latest] steps: From 3db1d78ae79606af2dd76d7ba161b6ddd42e5f27 Mon Sep 17 00:00:00 2001 From: bokket <3100563328@qq.com> Date: Mon, 6 Sep 2021 13:27:50 +0800 Subject: [PATCH 2/8] remove hdfs version 3.2.2 Signed-off-by: bokket <3100563328@qq.com> --- .github/workflows/intergration-test.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/intergration-test.yml b/.github/workflows/intergration-test.yml index 29fffe5..3bb1fb6 100644 --- a/.github/workflows/intergration-test.yml +++ b/.github/workflows/intergration-test.yml @@ -11,8 +11,8 @@ jobs: matrix: go: [ "1.15", "1.16" ] hdfs-version: - - 3.2.2 - 3.3.0 + - 3.3.1 os: [ubuntu-latest] steps: From 6cd5743744ac76f46279b16d117fdbf208c823ee Mon Sep 17 00:00:00 2001 From: bokket <3100563328@qq.com> Date: Mon, 6 Sep 2021 17:46:23 +0800 Subject: [PATCH 3/8] fix appender bug Signed-off-by: bokket <3100563328@qq.com> --- .github/workflows/intergration-test.yml | 1 + storage.go | 74 +++++++++++++------------ 2 files changed, 39 insertions(+), 36 deletions(-) diff --git a/.github/workflows/intergration-test.yml b/.github/workflows/intergration-test.yml index 3bb1fb6..1b3b7e1 100644 --- a/.github/workflows/intergration-test.yml +++ b/.github/workflows/intergration-test.yml @@ -13,6 +13,7 @@ jobs: hdfs-version: - 3.3.0 - 3.3.1 + - 3.2.2 os: [ubuntu-latest] steps: diff --git a/storage.go b/storage.go index c38e58e..367287c 100644 --- a/storage.go +++ b/storage.go @@ -34,29 +34,32 @@ func (s *Storage) create(path string, opt pairStorageCreate) (o *Object) { func (s *Storage) createAppend(ctx context.Context, path string, opt pairStorageCreateAppend) (o *Object, err error) { rp := s.getAbsPath(path) dir := filepath.Dir(rp) - err = s.hdfs.MkdirAll(dir, 0666) - if err != nil { - return - } + + // If dirname is already a directory, + // MkdirAll does nothing and returns nil. + err = s.hdfs.MkdirAll(dir, 0755) _, err = s.hdfs.Stat(rp) - if err == nil { - err = s.hdfs.Remove(rp) - if err != nil && errors.Is(err, os.ErrNotExist) { - // Omit `file not exist` error here - // ref: [GSP-46](https://github.com/beyondstorage/specs/blob/master/rfcs/46-idempotent-delete.md) - err = nil + if err != nil && errors.Is(err, os.ErrNotExist) { + f, err := s.hdfs.Create(rp) + + if err != nil { + return nil, err } + defer func() { + closeErr := f.Close() + if err == nil { + err = closeErr + } + }() } - f, err := s.hdfs.Create(rp) - if err != nil { - return + if err == nil { + // If the path does not exist, + // RemoveAll returns nil (no error). + err = s.hdfs.RemoveAll(rp) } - defer func() { - f.Close() - }() o = s.newObject(true) o.ID = rp @@ -69,10 +72,9 @@ func (s *Storage) createAppend(ctx context.Context, path string, opt pairStorage func (s *Storage) createDir(ctx context.Context, path string, opt pairStorageCreateDir) (o *Object, err error) { rp := s.getAbsPath(path) + // If dirname is already a directory, + // MkdirAll does nothing and returns nil. err = s.hdfs.MkdirAll(rp, 0755) - if err != nil { - return - } o = s.newObject(true) o.ID = rp @@ -83,12 +85,10 @@ func (s *Storage) createDir(ctx context.Context, path string, opt pairStorageCre func (s *Storage) delete(ctx context.Context, path string, opt pairStorageDelete) (err error) { rp := s.getAbsPath(path) - err = s.hdfs.Remove(rp) - if err != nil && errors.Is(err, os.ErrNotExist) { - // Omit `file not exist` error here - // ref: [GSP-46](https://github.com/beyondstorage/specs/blob/master/rfcs/46-idempotent-delete.md) - err = nil - } + + // If the path does not exist, + // RemoveAll returns nil (no error). + err = s.hdfs.RemoveAll(rp) return err } @@ -186,18 +186,16 @@ func (s *Storage) stat(ctx context.Context, path string, opt pairStorageStat) (o func (s *Storage) write(ctx context.Context, path string, r io.Reader, size int64, opt pairStorageWrite) (n int64, err error) { rp := s.getAbsPath(path) dir := filepath.Dir(rp) - err = s.hdfs.MkdirAll(dir, 0666) - if err != nil { - return 0, err - } + + // If dirname is already a directory, + // MkdirAll does nothing and returns nil. + err = s.hdfs.MkdirAll(dir, 0755) + _, err = s.hdfs.Stat(rp) if err == nil { - err = s.hdfs.Remove(rp) - if err != nil && errors.Is(err, os.ErrNotExist) { - // Omit `file not exist` error here - // ref: [GSP-46](https://github.com/beyondstorage/specs/blob/master/rfcs/46-idempotent-delete.md) - err = nil - } + // If the path does not exist, + // RemoveAll returns nil (no error). + err = s.hdfs.RemoveAll(rp) } f, err := s.hdfs.Create(rp) @@ -220,11 +218,15 @@ func (s *Storage) write(ctx context.Context, path string, r io.Reader, size int6 func (s *Storage) writeAppend(ctx context.Context, o *Object, r io.Reader, size int64, opt pairStorageWriteAppend) (n int64, err error) { f, err := s.hdfs.Append(o.ID) + if err != nil { return } defer func() { - f.Close() + closeErr := f.Close() + if err == nil { + err = closeErr + } }() return io.CopyN(f, r, size) From 0eab9b1a5ea50333dff592c5cb1a6ca1dd4284bd Mon Sep 17 00:00:00 2001 From: bokket <3100563328@qq.com> Date: Mon, 6 Sep 2021 17:50:20 +0800 Subject: [PATCH 4/8] remove version 3.2.2 Signed-off-by: bokket <3100563328@qq.com> --- .github/workflows/intergration-test.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/.github/workflows/intergration-test.yml b/.github/workflows/intergration-test.yml index 1b3b7e1..3bb1fb6 100644 --- a/.github/workflows/intergration-test.yml +++ b/.github/workflows/intergration-test.yml @@ -13,7 +13,6 @@ jobs: hdfs-version: - 3.3.0 - 3.3.1 - - 3.2.2 os: [ubuntu-latest] steps: From 976236acc19f68af154c6ff62cd5a7f935546529 Mon Sep 17 00:00:00 2001 From: bokket <3100563328@qq.com> Date: Mon, 6 Sep 2021 19:33:51 +0800 Subject: [PATCH 5/8] fix error Handling Signed-off-by: bokket <3100563328@qq.com> --- storage.go | 73 ++++++++++++++++++++++++++++++++++++++++-------------- 1 file changed, 54 insertions(+), 19 deletions(-) diff --git a/storage.go b/storage.go index 367287c..a2be5d6 100644 --- a/storage.go +++ b/storage.go @@ -38,29 +38,41 @@ func (s *Storage) createAppend(ctx context.Context, path string, opt pairStorage // If dirname is already a directory, // MkdirAll does nothing and returns nil. err = s.hdfs.MkdirAll(dir, 0755) + // IsNotExist will create a Mkdir rpc communication + // So we just need to catch other errors + if err != nil && !errors.Is(err, os.ErrNotExist) { + return nil, err + } _, err = s.hdfs.Stat(rp) - if err != nil && errors.Is(err, os.ErrNotExist) { - f, err := s.hdfs.Create(rp) - - if err != nil { - return nil, err - } - defer func() { - closeErr := f.Close() - if err == nil { - err = closeErr - } - }() - } - + // The error returned by Stat can only be nil or not os.ErrNotExist if err == nil { // If the path does not exist, // RemoveAll returns nil (no error). err = s.hdfs.RemoveAll(rp) + if err != nil && !errors.Is(err, os.ErrNotExist) { + return nil, err + } + } + + // This ensures that err can only be os.ErrNotExist + if err != nil && !errors.Is(err, os.ErrNotExist) { + return nil, err } + f, err := s.hdfs.Create(rp) + + if err != nil { + return nil, err + } + defer func() { + closeErr := f.Close() + if err == nil { + err = closeErr + } + }() + o = s.newObject(true) o.ID = rp o.Path = path @@ -75,6 +87,11 @@ func (s *Storage) createDir(ctx context.Context, path string, opt pairStorageCre // If dirname is already a directory, // MkdirAll does nothing and returns nil. err = s.hdfs.MkdirAll(rp, 0755) + // IsNotExist will create a Mkdir rpc communication + // So we just need to catch other errors + if err != nil && !errors.Is(err, os.ErrNotExist) { + return nil, err + } o = s.newObject(true) o.ID = rp @@ -86,8 +103,9 @@ func (s *Storage) createDir(ctx context.Context, path string, opt pairStorageCre func (s *Storage) delete(ctx context.Context, path string, opt pairStorageDelete) (err error) { rp := s.getAbsPath(path) - // If the path does not exist, - // RemoveAll returns nil (no error). + // If the path does not exist, + // RemoveAll returns nil (no error). + // If there is an error here other than os.ErrNotExist is also thrown directly err = s.hdfs.RemoveAll(rp) return err } @@ -120,12 +138,16 @@ func (s *Storage) move(ctx context.Context, src string, dst string, opt pairStor return services.ErrObjectModeInvalid } } + return s.hdfs.Rename(rs, rd) } func (s *Storage) read(ctx context.Context, path string, w io.Writer, opt pairStorageRead) (n int64, err error) { rp := s.getAbsPath(path) f, err := s.hdfs.Open(rp) + if err != nil { + return 0, err + } defer func() { closeErr := f.Close() @@ -134,9 +156,6 @@ func (s *Storage) read(ctx context.Context, path string, w io.Writer, opt pairSt } }() - if err != nil { - return 0, err - } if opt.HasOffset { _, err := f.Seek(opt.Offset, 0) if err != nil { @@ -190,18 +209,33 @@ func (s *Storage) write(ctx context.Context, path string, r io.Reader, size int6 // If dirname is already a directory, // MkdirAll does nothing and returns nil. err = s.hdfs.MkdirAll(dir, 0755) + // IsNotExist will create a Mkdir rpc communication + // So we just need to catch other errors + if err != nil && !errors.Is(err, os.ErrNotExist) { + return 0, err + } _, err = s.hdfs.Stat(rp) if err == nil { // If the path does not exist, // RemoveAll returns nil (no error). err = s.hdfs.RemoveAll(rp) + + if err != nil && !errors.Is(err, os.ErrNotExist) { + return 0, err + } + } + + // This ensures that err can only be os.ErrNotExist + if err != nil && !errors.Is(err, os.ErrNotExist) { + return 0, err } f, err := s.hdfs.Create(rp) if err != nil { return 0, err } + defer func() { closeErr := f.Close() if err == nil { @@ -222,6 +256,7 @@ func (s *Storage) writeAppend(ctx context.Context, o *Object, r io.Reader, size if err != nil { return } + defer func() { closeErr := f.Close() if err == nil { From 3a0e32a2e21fac341bae4388d0137be5e1b51e80 Mon Sep 17 00:00:00 2001 From: bokket <3100563328@qq.com> Date: Mon, 6 Sep 2021 22:16:31 +0800 Subject: [PATCH 6/8] remove some unwanted errors Signed-off-by: bokket <3100563328@qq.com> --- storage.go | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/storage.go b/storage.go index a2be5d6..54aae89 100644 --- a/storage.go +++ b/storage.go @@ -38,9 +38,9 @@ func (s *Storage) createAppend(ctx context.Context, path string, opt pairStorage // If dirname is already a directory, // MkdirAll does nothing and returns nil. err = s.hdfs.MkdirAll(dir, 0755) - // IsNotExist will create a Mkdir rpc communication + // If dirname is not exist ,it will create a Mkdir rpc communication // So we just need to catch other errors - if err != nil && !errors.Is(err, os.ErrNotExist) { + if err != nil { return nil, err } @@ -51,7 +51,7 @@ func (s *Storage) createAppend(ctx context.Context, path string, opt pairStorage // If the path does not exist, // RemoveAll returns nil (no error). err = s.hdfs.RemoveAll(rp) - if err != nil && !errors.Is(err, os.ErrNotExist) { + if err != nil { return nil, err } } @@ -87,9 +87,9 @@ func (s *Storage) createDir(ctx context.Context, path string, opt pairStorageCre // If dirname is already a directory, // MkdirAll does nothing and returns nil. err = s.hdfs.MkdirAll(rp, 0755) - // IsNotExist will create a Mkdir rpc communication + // If dirname is not exist ,it will create a Mkdir rpc communication // So we just need to catch other errors - if err != nil && !errors.Is(err, os.ErrNotExist) { + if err != nil { return nil, err } @@ -209,9 +209,9 @@ func (s *Storage) write(ctx context.Context, path string, r io.Reader, size int6 // If dirname is already a directory, // MkdirAll does nothing and returns nil. err = s.hdfs.MkdirAll(dir, 0755) - // IsNotExist will create a Mkdir rpc communication + // If dirname is not exist ,it will create a Mkdir rpc communication // So we just need to catch other errors - if err != nil && !errors.Is(err, os.ErrNotExist) { + if err != nil { return 0, err } @@ -221,7 +221,7 @@ func (s *Storage) write(ctx context.Context, path string, r io.Reader, size int6 // RemoveAll returns nil (no error). err = s.hdfs.RemoveAll(rp) - if err != nil && !errors.Is(err, os.ErrNotExist) { + if err != nil { return 0, err } } From 9a2572853644fa50627808aa75116eae44e288b4 Mon Sep 17 00:00:00 2001 From: bokket <3100563328@qq.com> Date: Tue, 7 Sep 2021 12:57:24 +0800 Subject: [PATCH 7/8] fix createAppend operator mistake Signed-off-by: bokket <3100563328@qq.com> --- storage.go | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/storage.go b/storage.go index 54aae89..d7d891a 100644 --- a/storage.go +++ b/storage.go @@ -48,11 +48,12 @@ func (s *Storage) createAppend(ctx context.Context, path string, opt pairStorage // The error returned by Stat can only be nil or not os.ErrNotExist if err == nil { - // If the path does not exist, - // RemoveAll returns nil (no error). - err = s.hdfs.RemoveAll(rp) - if err != nil { - return nil, err + // If the error returned by Stat is nil, the path must exist. + err = s.hdfs.Remove(rp) + if err != nil && errors.Is(err, os.ErrNotExist) { + // Omit `file not exist` error here + // ref: [GSP-46](https://github.com/beyondstorage/specs/blob/master/rfcs/46-idempotent-delete.md) + err = nil } } @@ -217,12 +218,13 @@ func (s *Storage) write(ctx context.Context, path string, r io.Reader, size int6 _, err = s.hdfs.Stat(rp) if err == nil { - // If the path does not exist, - // RemoveAll returns nil (no error). - err = s.hdfs.RemoveAll(rp) + // If the error returned by Stat is nil, the path must exist. + err = s.hdfs.Remove(rp) - if err != nil { - return 0, err + if err != nil && errors.Is(err, os.ErrNotExist) { + // Omit `file not exist` error here + // ref: [GSP-46](https://github.com/beyondstorage/specs/blob/master/rfcs/46-idempotent-delete.md) + err = nil } } From 20b91a6d3b0d613418e035b33ef2b950319d1d58 Mon Sep 17 00:00:00 2001 From: bokket <3100563328@qq.com> Date: Tue, 7 Sep 2021 13:19:56 +0800 Subject: [PATCH 8/8] modify delete operator Signed-off-by: bokket <3100563328@qq.com> --- storage.go | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/storage.go b/storage.go index d7d891a..9ebe87a 100644 --- a/storage.go +++ b/storage.go @@ -104,10 +104,13 @@ func (s *Storage) createDir(ctx context.Context, path string, opt pairStorageCre func (s *Storage) delete(ctx context.Context, path string, opt pairStorageDelete) (err error) { rp := s.getAbsPath(path) - // If the path does not exist, - // RemoveAll returns nil (no error). - // If there is an error here other than os.ErrNotExist is also thrown directly - err = s.hdfs.RemoveAll(rp) + err = s.hdfs.Remove(rp) + if err != nil && errors.Is(err, os.ErrNotExist) { + // Omit `file not exist` error here + // ref: [GSP-46](https://github.com/beyondstorage/specs/blob/master/rfcs/46-idempotent-delete.md) + err = nil + } + return err }