From fb43d64dc320e6d585fb8c548b99f25f029a7ec2 Mon Sep 17 00:00:00 2001 From: Harshavardhana Date: Thu, 21 Nov 2019 13:18:32 -0800 Subject: [PATCH] Fix healing on multiple zones (#8555) It is expected in zone healing underlying callers should return appropriate errors --- cmd/bucket-handlers.go | 10 ++---- cmd/notification.go | 45 +++++++++++++++++++------ cmd/object-lock.go | 5 +-- cmd/peer-rest-client.go | 12 +++++++ cmd/peer-rest-common.go | 71 ++++++++++++++++++++------------------- cmd/peer-rest-server.go | 25 ++++++++++++-- cmd/xl-v1-healing.go | 9 ++--- cmd/xl-v1-healing_test.go | 6 ++-- 8 files changed, 120 insertions(+), 63 deletions(-) diff --git a/cmd/bucket-handlers.go b/cmd/bucket-handlers.go index a5270e314..cf6e7026f 100644 --- a/cmd/bucket-handlers.go +++ b/cmd/bucket-handlers.go @@ -564,6 +564,7 @@ func (api objectAPIHandlers) PutBucketHandler(w http.ResponseWriter, r *http.Req return } globalBucketObjectLockConfig.Set(bucket, Retention{}) + globalNotificationSys.PutBucketObjectLockConfig(ctx, bucket, Retention{}) } // Make sure to add Location information here only for bucket @@ -896,12 +897,7 @@ func (api objectAPIHandlers) DeleteBucketHandler(w http.ResponseWriter, r *http. } } - globalBucketObjectLockConfig.Delete(bucket) - globalNotificationSys.RemoveNotification(bucket) - globalPolicySys.Remove(bucket) globalNotificationSys.DeleteBucket(ctx, bucket) - globalLifecycleSys.Remove(bucket) - globalNotificationSys.RemoveBucketLifecycle(ctx, bucket) // Write success response. writeSuccessNoContent(w) @@ -1026,8 +1022,8 @@ func (api objectAPIHandlers) PutBucketObjectLockConfigHandler(w http.ResponseWri globalBucketObjectLockConfig.Set(bucket, retention) globalNotificationSys.PutBucketObjectLockConfig(ctx, bucket, retention) } else { - globalBucketObjectLockConfig.Set(bucket, Retention{}) - globalBucketObjectLockConfig.Delete(bucket) + globalBucketObjectLockConfig.Remove(bucket) + globalNotificationSys.RemoveBucketObjectLockConfig(ctx, bucket) } // Write success response. diff --git a/cmd/notification.go b/cmd/notification.go index 994cc9f2f..195ce0362 100644 --- a/cmd/notification.go +++ b/cmd/notification.go @@ -525,6 +525,11 @@ func (sys *NotificationSys) SetBucketPolicy(ctx context.Context, bucketName stri // DeleteBucket - calls DeleteBucket RPC call on all peers. func (sys *NotificationSys) DeleteBucket(ctx context.Context, bucketName string) { + globalNotificationSys.RemoveNotification(bucketName) + globalBucketObjectLockConfig.Remove(bucketName) + globalPolicySys.Remove(bucketName) + globalLifecycleSys.Remove(bucketName) + go func() { ng := WithNPeers(len(sys.peerClients)) for idx, client := range sys.peerClients { @@ -557,6 +562,23 @@ func (sys *NotificationSys) RemoveBucketPolicy(ctx context.Context, bucketName s }() } +// RemoveBucketObjectLockConfig - calls RemoveBucketObjectLockConfig RPC call on all peers. +func (sys *NotificationSys) RemoveBucketObjectLockConfig(ctx context.Context, bucketName string) { + go func() { + ng := WithNPeers(len(sys.peerClients)) + for idx, client := range sys.peerClients { + if client == nil { + continue + } + client := client + ng.Go(ctx, func() error { + return client.RemoveBucketObjectLockConfig(bucketName) + }, idx, *client.host) + } + ng.Wait() + }() +} + // SetBucketLifecycle - calls SetBucketLifecycle on all peers. func (sys *NotificationSys) SetBucketLifecycle(ctx context.Context, bucketName string, bucketLifecycle *lifecycle.Lifecycle) { @@ -984,21 +1006,22 @@ func (sys *NotificationSys) Send(args eventArgs) []event.TargetIDErr { // PutBucketObjectLockConfig - put bucket object lock configuration to all peers. func (sys *NotificationSys) PutBucketObjectLockConfig(ctx context.Context, bucketName string, retention Retention) { - var wg sync.WaitGroup - for _, client := range sys.peerClients { + g := errgroup.WithNErrs(len(sys.peerClients)) + for index, client := range sys.peerClients { if client == nil { continue } - wg.Add(1) - go func(client *peerRESTClient) { - defer wg.Done() - if err := client.PutBucketObjectLockConfig(bucketName, retention); err != nil { - logger.GetReqInfo(ctx).AppendTags("remotePeer", client.host.Name) - logger.LogIf(ctx, err) - } - }(client) + index := index + g.Go(func() error { + return sys.peerClients[index].PutBucketObjectLockConfig(bucketName, retention) + }, index) + } + for i, err := range g.Wait() { + if err != nil { + logger.GetReqInfo(ctx).AppendTags("remotePeer", sys.peerClients[i].host.String()) + logger.LogIf(ctx, err) + } } - wg.Wait() } // NetReadPerfInfo - Network read performance information. diff --git a/cmd/object-lock.go b/cmd/object-lock.go index 80a68dcb8..cc0aefe11 100644 --- a/cmd/object-lock.go +++ b/cmd/object-lock.go @@ -118,8 +118,8 @@ func (config *BucketObjectLockConfig) Get(bucketName string) (r Retention, ok bo return r, ok } -// Delete - delete retention configuration. -func (config *BucketObjectLockConfig) Delete(bucketName string) { +// Remove - removes retention configuration. +func (config *BucketObjectLockConfig) Remove(bucketName string) { config.Lock() delete(config.retentionMap, bucketName) config.Unlock() @@ -414,6 +414,7 @@ func checkGovernanceBypassAllowed(ctx context.Context, r *http.Request, bucket, if err != nil { // ignore case where object no longer exists if toAPIError(ctx, err).Code == "NoSuchKey" { + oi.UserDefined = map[string]string{} return oi, ErrNone } return oi, toAPIErrorCode(ctx, err) diff --git a/cmd/peer-rest-client.go b/cmd/peer-rest-client.go index 851a5f3e8..37085354d 100644 --- a/cmd/peer-rest-client.go +++ b/cmd/peer-rest-client.go @@ -361,6 +361,18 @@ func (client *peerRESTClient) RemoveBucketPolicy(bucket string) error { return nil } +// RemoveBucketObjectLockConfig - Remove bucket object lock config on the peer node. +func (client *peerRESTClient) RemoveBucketObjectLockConfig(bucket string) error { + values := make(url.Values) + values.Set(peerRESTBucket, bucket) + respBody, err := client.call(peerRESTMethodBucketObjectLockConfigRemove, values, nil, -1) + if err != nil { + return err + } + defer http.DrainBody(respBody) + return nil +} + // SetBucketPolicy - Set bucket policy on the peer node. func (client *peerRESTClient) SetBucketPolicy(bucket string, bucketPolicy *policy.Policy) error { values := make(url.Values) diff --git a/cmd/peer-rest-common.go b/cmd/peer-rest-common.go index 574e9ebe5..071a752f6 100644 --- a/cmd/peer-rest-common.go +++ b/cmd/peer-rest-common.go @@ -24,41 +24,42 @@ const ( ) const ( - peerRESTMethodNetReadPerfInfo = "/netreadperfinfo" - peerRESTMethodCollectNetPerfInfo = "/collectnetperfinfo" - peerRESTMethodServerInfo = "/serverinfo" - peerRESTMethodCPULoadInfo = "/cpuloadinfo" - peerRESTMethodMemUsageInfo = "/memusageinfo" - peerRESTMethodDrivePerfInfo = "/driveperfinfo" - peerRESTMethodDeleteBucket = "/deletebucket" - peerRESTMethodServerUpdate = "/serverupdate" - peerRESTMethodSignalService = "/signalservice" - peerRESTMethodBackgroundHealStatus = "/backgroundhealstatus" - peerRESTMethodBackgroundOpsStatus = "/backgroundopsstatus" - peerRESTMethodGetLocks = "/getlocks" - peerRESTMethodBucketPolicyRemove = "/removebucketpolicy" - peerRESTMethodLoadUser = "/loaduser" - peerRESTMethodDeleteUser = "/deleteuser" - peerRESTMethodLoadPolicy = "/loadpolicy" - peerRESTMethodLoadPolicyMapping = "/loadpolicymapping" - peerRESTMethodDeletePolicy = "/deletepolicy" - peerRESTMethodLoadUsers = "/loadusers" - peerRESTMethodLoadGroup = "/loadgroup" - peerRESTMethodStartProfiling = "/startprofiling" - peerRESTMethodDownloadProfilingData = "/downloadprofilingdata" - peerRESTMethodBucketPolicySet = "/setbucketpolicy" - peerRESTMethodBucketNotificationPut = "/putbucketnotification" - peerRESTMethodBucketNotificationListen = "/listenbucketnotification" - peerRESTMethodReloadFormat = "/reloadformat" - peerRESTMethodTargetExists = "/targetexists" - peerRESTMethodSendEvent = "/sendevent" - peerRESTMethodTrace = "/trace" - peerRESTMethodBucketLifecycleSet = "/setbucketlifecycle" - peerRESTMethodBucketLifecycleRemove = "/removebucketlifecycle" - peerRESTMethodLog = "/log" - peerRESTMethodHardwareCPUInfo = "/cpuhardwareinfo" - peerRESTMethodHardwareNetworkInfo = "/networkhardwareinfo" - peerRESTMethodPutBucketObjectLockConfig = "/putbucketobjectlockconfig" + peerRESTMethodNetReadPerfInfo = "/netreadperfinfo" + peerRESTMethodCollectNetPerfInfo = "/collectnetperfinfo" + peerRESTMethodServerInfo = "/serverinfo" + peerRESTMethodCPULoadInfo = "/cpuloadinfo" + peerRESTMethodMemUsageInfo = "/memusageinfo" + peerRESTMethodDrivePerfInfo = "/driveperfinfo" + peerRESTMethodDeleteBucket = "/deletebucket" + peerRESTMethodServerUpdate = "/serverupdate" + peerRESTMethodSignalService = "/signalservice" + peerRESTMethodBackgroundHealStatus = "/backgroundhealstatus" + peerRESTMethodBackgroundOpsStatus = "/backgroundopsstatus" + peerRESTMethodGetLocks = "/getlocks" + peerRESTMethodBucketPolicyRemove = "/removebucketpolicy" + peerRESTMethodLoadUser = "/loaduser" + peerRESTMethodDeleteUser = "/deleteuser" + peerRESTMethodLoadPolicy = "/loadpolicy" + peerRESTMethodLoadPolicyMapping = "/loadpolicymapping" + peerRESTMethodDeletePolicy = "/deletepolicy" + peerRESTMethodLoadUsers = "/loadusers" + peerRESTMethodLoadGroup = "/loadgroup" + peerRESTMethodStartProfiling = "/startprofiling" + peerRESTMethodDownloadProfilingData = "/downloadprofilingdata" + peerRESTMethodBucketPolicySet = "/setbucketpolicy" + peerRESTMethodBucketNotificationPut = "/putbucketnotification" + peerRESTMethodBucketNotificationListen = "/listenbucketnotification" + peerRESTMethodReloadFormat = "/reloadformat" + peerRESTMethodTargetExists = "/targetexists" + peerRESTMethodSendEvent = "/sendevent" + peerRESTMethodTrace = "/trace" + peerRESTMethodBucketLifecycleSet = "/setbucketlifecycle" + peerRESTMethodBucketLifecycleRemove = "/removebucketlifecycle" + peerRESTMethodLog = "/log" + peerRESTMethodHardwareCPUInfo = "/cpuhardwareinfo" + peerRESTMethodHardwareNetworkInfo = "/networkhardwareinfo" + peerRESTMethodPutBucketObjectLockConfig = "/putbucketobjectlockconfig" + peerRESTMethodBucketObjectLockConfigRemove = "/removebucketobjectlockconfig" ) const ( diff --git a/cmd/peer-rest-server.go b/cmd/peer-rest-server.go index b74a69e79..0e417c977 100644 --- a/cmd/peer-rest-server.go +++ b/cmd/peer-rest-server.go @@ -510,7 +510,8 @@ func (s *peerRESTServer) DeleteBucketHandler(w http.ResponseWriter, r *http.Requ globalNotificationSys.RemoveNotification(bucketName) globalPolicySys.Remove(bucketName) - globalBucketObjectLockConfig.Delete(bucketName) + globalBucketObjectLockConfig.Remove(bucketName) + globalLifecycleSys.Remove(bucketName) w.(http.Flusher).Flush() } @@ -761,6 +762,24 @@ func (s *peerRESTServer) PutBucketNotificationHandler(w http.ResponseWriter, r * w.(http.Flusher).Flush() } +// RemoveBucketObjectLockConfigHandler - handles DELETE bucket object lock configuration. +func (s *peerRESTServer) RemoveBucketObjectLockConfigHandler(w http.ResponseWriter, r *http.Request) { + if !s.IsValid(w, r) { + s.writeErrorResponse(w, errors.New("Invalid request")) + return + } + + vars := mux.Vars(r) + bucketName := vars[peerRESTBucket] + if bucketName == "" { + s.writeErrorResponse(w, errors.New("Bucket name is missing")) + return + } + + globalBucketObjectLockConfig.Remove(bucketName) + w.(http.Flusher).Flush() +} + // PutBucketObjectLockConfigHandler - handles PUT bucket object lock configuration. func (s *peerRESTServer) PutBucketObjectLockConfigHandler(w http.ResponseWriter, r *http.Request) { if !s.IsValid(w, r) { @@ -1027,7 +1046,6 @@ func (s *peerRESTServer) IsValid(w http.ResponseWriter, r *http.Request) bool { func registerPeerRESTHandlers(router *mux.Router) { server := &peerRESTServer{} subrouter := router.PathPrefix(peerRESTPrefix).Subrouter() - subrouter.Methods(http.MethodPost).Path(peerRESTVersionPrefix + peerRESTMethodPutBucketObjectLockConfig).HandlerFunc(httpTraceHdrs(server.PutBucketObjectLockConfigHandler)).Queries(restQueries(peerRESTBucket)...) subrouter.Methods(http.MethodPost).Path(peerRESTVersionPrefix + peerRESTMethodNetReadPerfInfo).HandlerFunc(httpTraceHdrs(server.NetReadPerfInfoHandler)).Queries(restQueries(peerRESTNetPerfSize)...) subrouter.Methods(http.MethodPost).Path(peerRESTVersionPrefix + peerRESTMethodCollectNetPerfInfo).HandlerFunc(httpTraceHdrs(server.CollectNetPerfInfoHandler)).Queries(restQueries(peerRESTNetPerfSize)...) subrouter.Methods(http.MethodPost).Path(peerRESTVersionPrefix + peerRESTMethodGetLocks).HandlerFunc(httpTraceHdrs(server.GetLocksHandler)) @@ -1069,6 +1087,9 @@ func registerPeerRESTHandlers(router *mux.Router) { subrouter.Methods(http.MethodPost).Path(peerRESTVersionPrefix + peerRESTMethodBackgroundHealStatus).HandlerFunc(server.BackgroundHealStatusHandler) subrouter.Methods(http.MethodPost).Path(peerRESTVersionPrefix + peerRESTMethodLog).HandlerFunc(server.ConsoleLogHandler) + subrouter.Methods(http.MethodPost).Path(peerRESTVersionPrefix + peerRESTMethodPutBucketObjectLockConfig).HandlerFunc(httpTraceHdrs(server.PutBucketObjectLockConfigHandler)).Queries(restQueries(peerRESTBucket)...) + subrouter.Methods(http.MethodPost).Path(peerRESTVersionPrefix + peerRESTMethodBucketObjectLockConfigRemove).HandlerFunc(httpTraceHdrs(server.RemoveBucketObjectLockConfigHandler)).Queries(restQueries(peerRESTBucket)...) + // If none of the routes match add default error handler routes router.NotFoundHandler = http.HandlerFunc(httpTraceAll(errorResponseHandler)) router.MethodNotAllowedHandler = http.HandlerFunc(httpTraceAll(errorResponseHandler)) diff --git a/cmd/xl-v1-healing.go b/cmd/xl-v1-healing.go index f49e4c147..f01d7a4bb 100644 --- a/cmd/xl-v1-healing.go +++ b/cmd/xl-v1-healing.go @@ -697,9 +697,10 @@ func (xl xlObjects) HealObject(ctx context.Context, bucket, object string, dryRu writeQuorum = len(xl.getDisks())/2 + 1 } if !dryRun && remove { - err = xl.deleteObject(healCtx, bucket, object, writeQuorum, false) + xl.deleteObject(healCtx, bucket, object, writeQuorum, false) } - return defaultHealResult(xlMetaV1{}, storageDisks, errs, bucket, object), err + err = reduceReadQuorumErrs(ctx, errs, nil, writeQuorum-1) + return defaultHealResult(xlMetaV1{}, storageDisks, errs, bucket, object), toObjectErr(err, bucket, object) } latestXLMeta, err := getLatestXLMeta(healCtx, partsMetadata, errs) @@ -718,7 +719,7 @@ func (xl xlObjects) HealObject(ctx context.Context, bucket, object string, dryRu // Only if we get errors from all the disks we return error. Else we need to // continue to return filled madmin.HealResultItem struct which includes info // on what disks the file is available etc. - if reducedErr := reduceReadQuorumErrs(ctx, errs, nil, latestXLMeta.Erasure.DataBlocks); reducedErr != nil { + if err = reduceReadQuorumErrs(ctx, errs, nil, latestXLMeta.Erasure.DataBlocks); err != nil { if m, ok := isObjectDangling(partsMetadata, errs, []error{}); ok { writeQuorum := m.Erasure.DataBlocks + 1 if m.Erasure.DataBlocks == 0 { @@ -728,7 +729,7 @@ func (xl xlObjects) HealObject(ctx context.Context, bucket, object string, dryRu xl.deleteObject(ctx, bucket, object, writeQuorum, false) } } - return defaultHealResult(latestXLMeta, storageDisks, errs, bucket, object), toObjectErr(reducedErr, bucket, object) + return defaultHealResult(latestXLMeta, storageDisks, errs, bucket, object), toObjectErr(err, bucket, object) } } diff --git a/cmd/xl-v1-healing_test.go b/cmd/xl-v1-healing_test.go index 2dea22c35..fafbaed18 100644 --- a/cmd/xl-v1-healing_test.go +++ b/cmd/xl-v1-healing_test.go @@ -185,10 +185,12 @@ func TestHealObjectCorrupted(t *testing.T) { xl.getDisks()[i].DeleteFile(bucket, filepath.Join(object, xlMetaJSONFile)) } - // Try healing now, expect to receive errDiskNotFound. + // Try healing now, expect to receive errFileNotFound. _, err = objLayer.HealObject(context.Background(), bucket, object, false, true, madmin.HealDeepScan) if err != nil { - t.Errorf("Expected nil but received %v", err) + if _, ok := err.(ObjectNotFound); !ok { + t.Errorf("Expect %v but received %v", ObjectNotFound{Bucket: bucket, Object: object}, err) + } } // since majority of xl.jsons are not available, object should be successfully deleted.