diff --git a/internal/storage/local_chunk_manager.go b/internal/storage/local_chunk_manager.go index 2a37ed40fd..1d9ba04600 100644 --- a/internal/storage/local_chunk_manager.go +++ b/internal/storage/local_chunk_manager.go @@ -33,7 +33,7 @@ import ( "golang.org/x/exp/mmap" "github.com/milvus-io/milvus/internal/log" - "github.com/milvus-io/milvus/internal/util/errorutil" + "github.com/milvus-io/milvus/internal/util/errutil" ) // LocalChunkManager is responsible for read and write local file. @@ -103,16 +103,13 @@ func (lcm *LocalChunkManager) Write(ctx context.Context, filePath string, conten // MultiWrite writes the data to local storage. func (lcm *LocalChunkManager) MultiWrite(ctx context.Context, contents map[string][]byte) error { - var el errorutil.ErrorList + var el error for filePath, content := range contents { err := lcm.Write(ctx, filePath, content) if err != nil { - el = append(el, err) + el = errutil.Combine(el, errors.Wrapf(err, "write %s failed", filePath)) } } - if len(el) == 0 { - return nil - } return el } @@ -144,17 +141,14 @@ func (lcm *LocalChunkManager) Read(ctx context.Context, filePath string) ([]byte // MultiRead reads the local storage data if exists. func (lcm *LocalChunkManager) MultiRead(ctx context.Context, filePaths []string) ([][]byte, error) { results := make([][]byte, len(filePaths)) - var el errorutil.ErrorList + var el error for i, filePath := range filePaths { content, err := lcm.Read(ctx, filePath) if err != nil { - el = append(el, err) + el = errutil.Combine(el, errors.Wrapf(err, "failed to read %s", filePath)) } results[i] = content } - if len(el) == 0 { - return results, nil - } return results, el } @@ -254,16 +248,13 @@ func (lcm *LocalChunkManager) Remove(ctx context.Context, filePath string) error } func (lcm *LocalChunkManager) MultiRemove(ctx context.Context, filePaths []string) error { - var el errorutil.ErrorList + var el error for _, filePath := range filePaths { err := lcm.Remove(ctx, filePath) if err != nil { - el = append(el, err) + el = errutil.Combine(err, errors.Wrapf(err, "failed to remove %s", filePath)) } } - if len(el) == 0 { - return nil - } return el } diff --git a/internal/storage/minio_chunk_manager.go b/internal/storage/minio_chunk_manager.go index 9c1a9ed3a7..a44001cfe4 100644 --- a/internal/storage/minio_chunk_manager.go +++ b/internal/storage/minio_chunk_manager.go @@ -29,7 +29,7 @@ import ( "github.com/milvus-io/milvus/internal/log" "github.com/milvus-io/milvus/internal/storage/gcp" - "github.com/milvus-io/milvus/internal/util/errorutil" + "github.com/milvus-io/milvus/internal/util/errutil" "github.com/milvus-io/milvus/internal/util/retry" "github.com/minio/minio-go/v7" "github.com/minio/minio-go/v7/pkg/credentials" @@ -201,16 +201,13 @@ func (mcm *MinioChunkManager) Write(ctx context.Context, filePath string, conten // MultiWrite saves multiple objects, the path is the key of @kvs. // The object value is the value of @kvs. func (mcm *MinioChunkManager) MultiWrite(ctx context.Context, kvs map[string][]byte) error { - var el errorutil.ErrorList + var el error for key, value := range kvs { err := mcm.Write(ctx, key, value) if err != nil { - el = append(el, err) + el = errutil.Combine(el, errors.Wrapf(err, "failed to write %s", key)) } } - if len(el) == 0 { - return nil - } return el } @@ -272,19 +269,16 @@ func (mcm *MinioChunkManager) Read(ctx context.Context, filePath string) ([]byte } func (mcm *MinioChunkManager) MultiRead(ctx context.Context, keys []string) ([][]byte, error) { - var el errorutil.ErrorList + var el error var objectsValues [][]byte for _, key := range keys { objectValue, err := mcm.Read(ctx, key) if err != nil { - el = append(el, err) + el = errutil.Combine(el, errors.Wrapf(err, "failed to read %s", key)) } objectsValues = append(objectsValues, objectValue) } - if len(el) == 0 { - return objectsValues, nil - } return objectsValues, el } @@ -349,16 +343,13 @@ func (mcm *MinioChunkManager) Remove(ctx context.Context, filePath string) error // MultiRemove deletes a objects with @keys. func (mcm *MinioChunkManager) MultiRemove(ctx context.Context, keys []string) error { - var el errorutil.ErrorList + var el error for _, key := range keys { err := mcm.Remove(ctx, key) if err != nil { - el = append(el, err) + el = errutil.Combine(el, errors.Wrapf(err, "failed to remove %s", key)) } } - if len(el) == 0 { - return nil - } return el } diff --git a/internal/util/errorutil/util.go b/internal/util/errorutil/util.go index d61c60c2d6..ce0c639747 100644 --- a/internal/util/errorutil/util.go +++ b/internal/util/errorutil/util.go @@ -2,7 +2,6 @@ package errorutil import ( "fmt" - "strings" "github.com/cockroachdb/errors" @@ -11,27 +10,6 @@ import ( "github.com/milvus-io/milvus/internal/util/typeutil" ) -// ErrorList for print error log -type ErrorList []error - -// Error method return an string representation of retry error list. -func (el ErrorList) Error() string { - limit := 10 - var builder strings.Builder - builder.WriteString("All attempts results:\n") - for index, err := range el { - // if early termination happens - if err == nil { - break - } - if index > limit { - break - } - builder.WriteString(fmt.Sprintf("attempt #%d:%s\n", index+1, err.Error())) - } - return builder.String() -} - func UnhealthyStatus(code commonpb.StateCode) *commonpb.Status { return &commonpb.Status{ ErrorCode: commonpb.ErrorCode_UnexpectedError, diff --git a/internal/util/errorutil/util_test.go b/internal/util/errorutil/util_test.go index a1a2c8c935..b21a29e2e4 100644 --- a/internal/util/errorutil/util_test.go +++ b/internal/util/errorutil/util_test.go @@ -1,29 +1 @@ package errorutil - -import ( - "testing" - - "github.com/cockroachdb/errors" - - "github.com/milvus-io/milvus/internal/log" - "go.uber.org/zap" -) - -func TestErrorList_Error(t *testing.T) { - var el ErrorList - for i := 0; i < 5; i++ { - el = append(el, errors.New("error occur")) - } - for i := 0; i < 5; i++ { - el = append(el, nil) - } - log.Debug("all errors are", zap.Error(el)) -} - -func TestErrorList_Error_Limit(t *testing.T) { - var el ErrorList - for i := 0; i < 15; i++ { - el = append(el, errors.New("error occur")) - } - log.Debug("all errors are", zap.Error(el)) -} diff --git a/internal/util/errutil/errors.go b/internal/util/errutil/errors.go index b4ac3d532c..04def61e6a 100644 --- a/internal/util/errutil/errors.go +++ b/internal/util/errutil/errors.go @@ -1,3 +1,20 @@ +// Licensed to the LF AI & Data foundation under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you 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. + +// errutil package provides utility for errors handling. package errutil import ( diff --git a/internal/util/errutil/errors_test.go b/internal/util/errutil/errors_test.go index 6df9266ff0..e0d28397e7 100644 --- a/internal/util/errutil/errors_test.go +++ b/internal/util/errutil/errors_test.go @@ -1,3 +1,19 @@ +// Licensed to the LF AI & Data foundation under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you 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 errutil import (