From 5e5cdc581da91a7e25f90e902fe6e4f5f392b4fd Mon Sep 17 00:00:00 2001 From: Harshavardhana Date: Fri, 30 Oct 2020 14:55:50 -0700 Subject: [PATCH] remove unnecessary logging and move to log once (#10798) the current master logs way too much when a node is down, instead log once and move on. --- cmd/erasure-server-sets.go | 13 ++++++++++--- cmd/logger/logger.go | 13 +++++++++++-- cmd/logger/logonce.go | 16 ++++++++++++++++ cmd/metacache-bucket.go | 4 ++-- cmd/rest/client.go | 4 ---- 5 files changed, 39 insertions(+), 11 deletions(-) diff --git a/cmd/erasure-server-sets.go b/cmd/erasure-server-sets.go index 3e2b926bb..64be83470 100644 --- a/cmd/erasure-server-sets.go +++ b/cmd/erasure-server-sets.go @@ -1191,7 +1191,7 @@ func (z *erasureServerSets) DeleteBucket(ctx context.Context, bucket string, for // Note that set distribution is ignored so it should only be used in cases where // data is not distributed across sets. // Errors are logged but individual disk failures are not returned. -func (z *erasureServerSets) deleteAll(ctx context.Context, bucket, prefix string) error { +func (z *erasureServerSets) deleteAll(ctx context.Context, bucket, prefix string) { var wg sync.WaitGroup for _, servers := range z.serverSets { for _, set := range servers.sets { @@ -1202,13 +1202,20 @@ func (z *erasureServerSets) deleteAll(ctx context.Context, bucket, prefix string wg.Add(1) go func(disk StorageAPI) { defer wg.Done() - logger.LogIf(ctx, disk.Delete(ctx, bucket, prefix, true)) + if err := disk.Delete(ctx, bucket, prefix, true); err != nil { + if !IsErrIgnored(err, []error{ + errDiskNotFound, + errVolumeNotFound, + errFileNotFound, + }...) { + logger.LogOnceIf(ctx, err, disk.String()) + } + } }(disk) } } } wg.Wait() - return nil } // This function is used to undo a successful DeleteBucket operation. diff --git a/cmd/logger/logger.go b/cmd/logger/logger.go index 7011f514e..1e2bf704c 100644 --- a/cmd/logger/logger.go +++ b/cmd/logger/logger.go @@ -23,6 +23,7 @@ import ( "fmt" "go/build" "hash" + "net/http" "path/filepath" "reflect" "runtime" @@ -299,9 +300,17 @@ func LogIf(ctx context.Context, err error, errKind ...interface{}) { return } - if !errors.Is(err, context.Canceled) { - logIf(ctx, err, errKind...) + if errors.Is(err, context.Canceled) || errors.Is(err, http.ErrServerClosed) { + return } + + if e := errors.Unwrap(err); e != nil { + if e.Error() == "disk not found" { + return + } + } + + logIf(ctx, err, errKind...) } // logIf prints a detailed error message during diff --git a/cmd/logger/logonce.go b/cmd/logger/logonce.go index f6355c14a..dbac4310c 100644 --- a/cmd/logger/logonce.go +++ b/cmd/logger/logonce.go @@ -18,6 +18,8 @@ package logger import ( "context" + "errors" + "net/http" "sync" "time" @@ -77,5 +79,19 @@ var logOnce = newLogOnceType() // id is a unique identifier for related log messages, refer to cmd/notification.go // on how it is used. func LogOnceIf(ctx context.Context, err error, id interface{}, errKind ...interface{}) { + if err == nil { + return + } + + if errors.Is(err, context.Canceled) || errors.Is(err, http.ErrServerClosed) { + return + } + + if e := errors.Unwrap(err); e != nil { + if e.Error() == "disk not found" { + return + } + } + logOnce.logOnceIf(ctx, err, id, errKind...) } diff --git a/cmd/metacache-bucket.go b/cmd/metacache-bucket.go index 7501be4f5..334d4637a 100644 --- a/cmd/metacache-bucket.go +++ b/cmd/metacache-bucket.go @@ -398,7 +398,7 @@ func (b *bucketMetacache) deleteAll() { wg.Add(1) go func(cache metacache) { defer wg.Done() - logger.LogIf(ctx, ez.deleteAll(ctx, minioMetaBucket, metacachePrefixForID(cache.bucket, cache.id))) + ez.deleteAll(ctx, minioMetaBucket, metacachePrefixForID(cache.bucket, cache.id)) }(b.caches[id]) delete(b.caches, id) } @@ -426,6 +426,6 @@ func (b *bucketMetacache) deleteCache(id string) { logger.LogIf(ctx, errors.New("bucketMetacache: expected objAPI to be *erasureServerSets")) return } - logger.LogIf(ctx, ez.deleteAll(ctx, minioMetaBucket, metacachePrefixForID(c.bucket, c.id))) + ez.deleteAll(ctx, minioMetaBucket, metacachePrefixForID(c.bucket, c.id)) } } diff --git a/cmd/rest/client.go b/cmd/rest/client.go index f74284145..8b9539837 100644 --- a/cmd/rest/client.go +++ b/cmd/rest/client.go @@ -27,8 +27,6 @@ import ( "sync/atomic" "time" - "github.com/minio/minio/cmd/logger" - xhttp "github.com/minio/minio/cmd/http" xnet "github.com/minio/minio/pkg/net" ) @@ -119,7 +117,6 @@ func (c *Client) Call(ctx context.Context, method string, values url.Values, bod resp, err := c.httpClient.Do(req) if err != nil { if xnet.IsNetworkOrHostDown(err, c.ExpectTimeouts) { - logger.LogIf(ctx, err, "marking disk offline") c.MarkOffline() } return nil, &NetworkError{err} @@ -149,7 +146,6 @@ func (c *Client) Call(ctx context.Context, method string, values url.Values, bod b, err := ioutil.ReadAll(io.LimitReader(resp.Body, c.MaxErrResponseSize)) if err != nil { if xnet.IsNetworkOrHostDown(err, c.ExpectTimeouts) { - logger.LogIf(ctx, err, "marking disk offline") c.MarkOffline() } return nil, err