From ca64b86112384f6c40d0ee0a8b774ebbcf348449 Mon Sep 17 00:00:00 2001 From: Krishnan Parthasarathi Date: Fri, 14 Apr 2017 22:58:35 +0530 Subject: [PATCH] Return possible states a heal operation (#4045) --- cmd/admin-handlers.go | 47 ++++++++++++++++++++++++++++--------- cmd/admin-handlers_test.go | 26 ++++++++++++++++++++ cmd/xl-v1-healing.go | 13 ++++++++-- pkg/madmin/API.md | 23 ++++++++++++++++++ pkg/madmin/heal-commands.go | 15 ++++++++++-- 5 files changed, 109 insertions(+), 15 deletions(-) diff --git a/cmd/admin-handlers.go b/cmd/admin-handlers.go index 3ad163db6..b3773bdd3 100644 --- a/cmd/admin-handlers.go +++ b/cmd/admin-handlers.go @@ -630,9 +630,40 @@ func isDryRun(qval url.Values) bool { return false } -type healObjectResult struct { - HealedCount int - OfflineCount int +// healResult - represents result of a heal operation like +// heal-object, heal-upload. +type healResult struct { + State healState `json:"state"` +} + +// healState - different states of heal operation +type healState int + +const ( + // healNone - none of the disks healed + healNone healState = iota + // healPartial - some disks were healed, others were offline + healPartial + // healOK - all disks were healed + healOK +) + +// newHealResult - returns healResult given number of disks healed and +// number of disks offline +func newHealResult(numHealedDisks, numOfflineDisks int) healResult { + var state healState + switch { + case numHealedDisks == 0: + state = healNone + + case numOfflineDisks > 0: + state = healPartial + + default: + state = healOK + } + + return healResult{State: state} } // HealObjectHandler - POST /?heal&bucket=mybucket&object=myobject&dry-run @@ -683,10 +714,7 @@ func (adminAPI adminAPIHandlers) HealObjectHandler(w http.ResponseWriter, r *htt return } - jsonBytes, err := json.Marshal(healObjectResult{ - HealedCount: numHealedDisks, - OfflineCount: numOfflineDisks, - }) + jsonBytes, err := json.Marshal(newHealResult(numHealedDisks, numOfflineDisks)) if err != nil { writeErrorResponse(w, toAPIErrorCode(err), r.URL) return @@ -761,10 +789,7 @@ func (adminAPI adminAPIHandlers) HealUploadHandler(w http.ResponseWriter, r *htt return } - jsonBytes, err := json.Marshal(healObjectResult{ - HealedCount: numHealedDisks, - OfflineCount: numOfflineDisks, - }) + jsonBytes, err := json.Marshal(newHealResult(numHealedDisks, numOfflineDisks)) if err != nil { writeErrorResponse(w, toAPIErrorCode(err), r.URL) return diff --git a/cmd/admin-handlers_test.go b/cmd/admin-handlers_test.go index 4e9fa4373..4a6ad4d36 100644 --- a/cmd/admin-handlers_test.go +++ b/cmd/admin-handlers_test.go @@ -1586,3 +1586,29 @@ func TestListHealUploadsHandler(t *testing.T) { } } + +// Test for newHealResult helper function. +func TestNewHealResult(t *testing.T) { + testCases := []struct { + healedDisks int + offlineDisks int + state healState + }{ + // 1. No disks healed, no disks offline. + {0, 0, healNone}, + // 2. No disks healed, non-zero disks offline. + {0, 1, healNone}, + // 3. Non-zero disks healed, no disks offline. + {1, 0, healOK}, + // 4. Non-zero disks healed, non-zero disks offline. + {1, 1, healPartial}, + } + + for i, test := range testCases { + actual := newHealResult(test.healedDisks, test.offlineDisks) + if actual.State != test.state { + t.Errorf("Test %d: Expected %v but received %v", i+1, + test.state, actual.State) + } + } +} diff --git a/cmd/xl-v1-healing.go b/cmd/xl-v1-healing.go index 70af99872..dcfe213e9 100644 --- a/cmd/xl-v1-healing.go +++ b/cmd/xl-v1-healing.go @@ -336,12 +336,19 @@ func healObject(storageDisks []StorageAPI, bucket string, object string, quorum return 0, 0, toObjectErr(aErr, bucket, object) } - numAvailableDisks := 0 + // Number of disks which don't serve data. numOfflineDisks := 0 - for index, disk := range availableDisks { + for index, disk := range storageDisks { switch { case disk == nil, errs[index] == errDiskNotFound: numOfflineDisks++ + } + } + + // Number of disks which have all parts of the given object. + numAvailableDisks := 0 + for _, disk := range availableDisks { + switch { case disk != nil: numAvailableDisks++ } @@ -357,6 +364,8 @@ func healObject(storageDisks []StorageAPI, bucket string, object string, quorum outDatedDisks := outDatedDisks(storageDisks, availableDisks, errs, partsMetadata, bucket, object) + // Number of disks that had outdated content of the given + // object and are online to be healed. numHealedDisks := 0 for _, disk := range outDatedDisks { if disk != nil { diff --git a/pkg/madmin/API.md b/pkg/madmin/API.md index 0698c97aa..dc09b9978 100644 --- a/pkg/madmin/API.md +++ b/pkg/madmin/API.md @@ -245,6 +245,18 @@ __Example__ ### HealObject(bucket, object string, isDryRun bool) (HealResult, error) If object is successfully healed returns nil, otherwise returns error indicating the reason for failure. If isDryRun is true, then the object is not healed, but heal object request is validated by the server. e.g, if the object exists, if object name is valid etc. +| Param | Type | Description | +|---|---|---| +|`h.State` | _HealState_ | Represents the result of heal operation. It could be one of `HealNone`, `HealPartial` or `HealOK`. | + + +| Value | Description | +|---|---| +|`HealNone` | Object/Upload wasn't healed on any of the disks | +|`HealPartial` | Object/Upload was healed on some of the disks needing heal | +| `HealOK` | Object/Upload was healed on all the disks needing heal | + + __Example__ ``` go @@ -327,6 +339,17 @@ __Example__ ### HealUpload(bucket, object, uploadID string, isDryRun bool) (HealResult, error) If upload is successfully healed returns nil, otherwise returns error indicating the reason for failure. If isDryRun is true, then the upload is not healed, but heal upload request is validated by the server. e.g, if the upload exists, if upload name is valid etc. +| Param | Type | Description | +|---|---|---| +|`h.State` | _HealState_ | Represents the result of heal operation. It could be one of `HealNone`, `HealPartial` or `HealOK`. | + + +| Value | Description | +|---|---| +| `HealNone` | Object/Upload wasn't healed on any of the disks | +| `HealPartial` | Object/Upload was healed on some of the disks needing heal | +| `HealOK` | Object/Upload was healed on all the disks needing heal | + ``` go isDryRun = false healResult, err := madmClnt.HealUpload("mybucket", "myobject", "myUploadID", isDryRun) diff --git a/pkg/madmin/heal-commands.go b/pkg/madmin/heal-commands.go index 8b69236f5..4bb2cf6c2 100644 --- a/pkg/madmin/heal-commands.go +++ b/pkg/madmin/heal-commands.go @@ -494,10 +494,21 @@ func (adm *AdminClient) HealUpload(bucket, object, uploadID string, dryrun bool) // HealResult - represents result of heal-object admin API. type HealResult struct { - HealedCount int // number of disks that were healed. - OfflineCount int // number of disks that needed healing but were offline. + State HealState `json:"state"` } +// HealState - different states of heal operation +type HealState int + +const ( + // HealNone - none of the disks healed + HealNone HealState = iota + // HealPartial - some disks were healed, others were offline + HealPartial + // HealOK - all disks were healed + HealOK +) + // HealObject - Heal the given object. func (adm *AdminClient) HealObject(bucket, object string, dryrun bool) (HealResult, error) { // Construct query params.