From 6c914d62e7f3755f826db88e3d82fb17e9b2b4c2 Mon Sep 17 00:00:00 2001 From: Payes Date: Wed, 2 Aug 2017 11:40:35 +0530 Subject: [PATCH 1/3] Fix issues with registration of replicas at controller. ref - openebs/openebs#176 --- backend/remote/remote.go | 6 ++++-- controller/client/controller_client.go | 13 +++++++------ controller/rest/model.go | 2 +- controller/rest/replica.go | 9 +++++++-- replica/rest/model.go | 10 +++++----- replica/rest/replica.go | 5 +++-- 6 files changed, 27 insertions(+), 18 deletions(-) diff --git a/backend/remote/remote.go b/backend/remote/remote.go index 2b0b856c..f262d4aa 100644 --- a/backend/remote/remote.go +++ b/backend/remote/remote.go @@ -143,7 +143,8 @@ func (r *Remote) GetRevisionCounter() (int64, error) { if replica.State != "open" && replica.State != "dirty" { return 0, fmt.Errorf("Invalid state %v for getting revision counter", replica.State) } - return replica.RevisionCounter, nil + counter, _ := strconv.ParseInt(replica.RevisionCounter, 10, 64) + return counter, nil } func (r *Remote) GetVolUsage() (types.VolUsage, error) { @@ -175,7 +176,8 @@ func (r *Remote) GetVolUsage() (types.VolUsage, error) { func (r *Remote) SetRevisionCounter(counter int64) error { logrus.Infof("Set revision counter of %s to : %v", r.name, counter) - return r.doAction("setrevisioncounter", &map[string]int64{"counter": counter}) + localRevCount := strconv.FormatInt(counter, 10) + return r.doAction("setrevisioncounter", &map[string]string{"counter": localRevCount}) } func (r *Remote) UpdatePeerDetails(replicaCount int64, quorumReplicaCount int64) error { diff --git a/controller/client/controller_client.go b/controller/client/controller_client.go index 95f6932c..12f82796 100644 --- a/controller/client/controller_client.go +++ b/controller/client/controller_client.go @@ -7,6 +7,7 @@ import ( "fmt" "io/ioutil" "net/http" + "strconv" "strings" "time" @@ -188,12 +189,12 @@ func (c *ControllerClient) GetVolume() (*rest.Volume, error) { } func (c *ControllerClient) Register(address string, revisionCount int64, peerDetails types.PeerDetails, replicaType string, upTime time.Duration) error { - err := c.post("/register", &types.RegReplica{ - Address: address, - RevCount: revisionCount, - PeerDetail: peerDetails, - RepType: replicaType, - UpTime: upTime, + err := c.post("/register", &rest.RegReplica{ + Address: address, + RevCount: strconv.FormatInt(revisionCount, 10), + PeerDetails: peerDetails, + RepType: replicaType, + UpTime: upTime, }, nil) return err } diff --git a/controller/rest/model.go b/controller/rest/model.go index f73ba25a..2ad43f5f 100644 --- a/controller/rest/model.go +++ b/controller/rest/model.go @@ -94,7 +94,7 @@ type PrepareRebuildOutput struct { type RegReplica struct { client.Resource Address string `json:"Address"` - RevCount int64 `json:"RevCount"` + RevCount string `json:"RevCount"` PeerDetails types.PeerDetails `json:"PeerDetails"` RepType string `json:"RepType"` UpTime time.Duration `json:"UpTime"` diff --git a/controller/rest/replica.go b/controller/rest/replica.go index 85548383..89663f09 100644 --- a/controller/rest/replica.go +++ b/controller/rest/replica.go @@ -2,6 +2,7 @@ package rest import ( "net/http" + "strconv" "github.com/gorilla/mux" "github.com/openebs/jiva/types" @@ -39,13 +40,17 @@ func (s *Server) GetReplica(rw http.ResponseWriter, req *http.Request) error { } func (s *Server) RegisterReplica(rw http.ResponseWriter, req *http.Request) error { - var regReplica RegReplica + var ( + regReplica RegReplica + localRevCount int64 + ) apiContext := api.GetApiContext(req) if err := apiContext.Read(®Replica); err != nil { return err } - local := types.RegReplica{Address: regReplica.Address, RevCount: regReplica.RevCount, PeerDetail: regReplica.PeerDetails, RepType: regReplica.RepType, UpTime: regReplica.UpTime} + localRevCount, _ = strconv.ParseInt(regReplica.RevCount, 10, 64) + local := types.RegReplica{Address: regReplica.Address, RevCount: localRevCount, PeerDetail: regReplica.PeerDetails, RepType: regReplica.RepType, UpTime: regReplica.UpTime} if err := s.c.RegisterReplica(local); err != nil { return err } diff --git a/replica/rest/model.go b/replica/rest/model.go index e7fb2f2e..62c55dca 100644 --- a/replica/rest/model.go +++ b/replica/rest/model.go @@ -20,7 +20,7 @@ type Replica struct { Chain []string `json:"chain"` Disks map[string]replica.DiskInfo `json:"disks"` RemainSnapshots int `json:"remainsnapshots"` - RevisionCounter int64 `json:"revisioncounter"` + RevisionCounter string `json:"revisioncounter"` ReplicaCounter int64 `json:"replicacounter"` UsedLogicalBlocks string `json:"usedlogicalblocks"` UsedBlocks string `json:"usedblocks"` @@ -28,8 +28,8 @@ type Replica struct { type Stats struct { client.Resource - ReplicaCounter int64 `json:"replicacounter"` - RevisionCounter int64 `json:"revisioncounter"` + ReplicaCounter int64 `json:"replicacounter"` + RevisionCounter string `json:"revisioncounter"` } type CreateInput struct { @@ -91,7 +91,7 @@ type PrepareRemoveDiskOutput struct { type RevisionCounter struct { client.Resource - Counter int64 `json:"counter"` + Counter string `json:"counter"` } type ReplicaCounter struct { @@ -193,7 +193,7 @@ func NewReplica(context *api.ApiContext, state replica.State, info replica.Info, r.Chain, _ = rep.DisplayChain() r.Disks = rep.ListDisks() r.RemainSnapshots = rep.GetRemainSnapshotCounts() - r.RevisionCounter = rep.GetRevisionCounter() + r.RevisionCounter = strconv.FormatInt(rep.GetRevisionCounter(), 10) } return r diff --git a/replica/rest/replica.go b/replica/rest/replica.go index 4e46a3e0..4c90e893 100644 --- a/replica/rest/replica.go +++ b/replica/rest/replica.go @@ -57,7 +57,7 @@ func (s *Server) GetStats(rw http.ResponseWriter, req *http.Request) error { Actions: map[string]string{}, Links: map[string]string{}, }, - RevisionCounter: stats.RevisionCounter, + RevisionCounter: strconv.FormatInt(stats.RevisionCounter, 10), ReplicaCounter: stats.ReplicaCounter, } apiContext.Write(resp) @@ -239,7 +239,8 @@ func (s *Server) SetRevisionCounter(rw http.ResponseWriter, req *http.Request) e if err := apiContext.Read(&input); err != nil && err != io.EOF { return err } - return s.doOp(req, s.s.SetRevisionCounter(input.Counter)) + counter, _ := strconv.ParseInt(input.Counter, 10, 64) + return s.doOp(req, s.s.SetRevisionCounter(counter)) } func (s *Server) UpdatePeerDetails(rw http.ResponseWriter, req *http.Request) error { From 7943ab8746ac243cbd8e46d5b655072c0737c23c Mon Sep 17 00:00:00 2001 From: payes Date: Wed, 13 Sep 2017 17:16:19 +0530 Subject: [PATCH 2/3] Fix to avoid launching target with a replica in rebuilding state --- backend/file/file.go | 2 +- backend/remote/remote.go | 2 +- controller/client/controller_client.go | 3 ++- controller/control.go | 13 ++++++++----- controller/rebuild.go | 7 +++++++ controller/replicator.go | 2 +- controller/rest/model.go | 1 + controller/rest/replica.go | 2 +- replica/replica.go | 6 ++++++ replica/rest/model.go | 4 ++-- replica/server.go | 20 +++++++++++++++++++- sync/sync.go | 10 +++++++--- types/types.go | 7 ++++--- 13 files changed, 60 insertions(+), 19 deletions(-) diff --git a/backend/file/file.go b/backend/file/file.go index 8059b1f1..319ba739 100644 --- a/backend/file/file.go +++ b/backend/file/file.go @@ -59,7 +59,7 @@ func (f *Wrapper) SetRevisionCounter(counter int64) error { return nil } -func (f *Wrapper) UpdatePeerDetails(replicaCount int64, quorumReplicaCount int64) error { +func (f *Wrapper) UpdatePeerDetails(replicaCount int, quorumReplicaCount int) error { return nil } diff --git a/backend/remote/remote.go b/backend/remote/remote.go index f262d4aa..9c4420d2 100644 --- a/backend/remote/remote.go +++ b/backend/remote/remote.go @@ -180,7 +180,7 @@ func (r *Remote) SetRevisionCounter(counter int64) error { return r.doAction("setrevisioncounter", &map[string]string{"counter": localRevCount}) } -func (r *Remote) UpdatePeerDetails(replicaCount int64, quorumReplicaCount int64) error { +func (r *Remote) UpdatePeerDetails(replicaCount int, quorumReplicaCount int) error { logrus.Infof("Update peer details of %s ", r.name) return r.doAction("updatepeerdetails", &map[string]interface{}{ diff --git a/controller/client/controller_client.go b/controller/client/controller_client.go index 12f82796..1150dd22 100644 --- a/controller/client/controller_client.go +++ b/controller/client/controller_client.go @@ -188,13 +188,14 @@ func (c *ControllerClient) GetVolume() (*rest.Volume, error) { return &volumes.Data[0], nil } -func (c *ControllerClient) Register(address string, revisionCount int64, peerDetails types.PeerDetails, replicaType string, upTime time.Duration) error { +func (c *ControllerClient) Register(address string, revisionCount int64, peerDetails types.PeerDetails, replicaType string, upTime time.Duration, state string) error { err := c.post("/register", &rest.RegReplica{ Address: address, RevCount: strconv.FormatInt(revisionCount, 10), PeerDetails: peerDetails, RepType: replicaType, UpTime: upTime, + RepState: state, }, nil) return err } diff --git a/controller/control.go b/controller/control.go index 29d40a89..499fd871 100644 --- a/controller/control.go +++ b/controller/control.go @@ -20,9 +20,9 @@ type Controller struct { size int64 sectorSize int64 replicas []types.Replica - replicaCount int64 + replicaCount int quorumReplicas []types.Replica - quorumReplicaCount int64 + quorumReplicaCount int factory types.BackendFactory backend *replicator frontend types.Frontend @@ -220,6 +220,10 @@ func (c *Controller) registerReplica(register types.RegReplica) error { return nil } } + fmt.Println("STATE =", register.RepState) + if register.RepState == "rebuilding" { + return nil + } if c.MaxRevReplica == "" { c.MaxRevReplica = register.Address @@ -229,13 +233,13 @@ func (c *Controller) registerReplica(register types.RegReplica) error { c.MaxRevReplica = register.Address } - if ((int64)(len(c.RegisteredReplicas)) >= c.replicaCount/2) && ((int64)(len(c.RegisteredReplicas)+len(c.RegisteredQuorumReplicas)) > (c.quorumReplicaCount+c.replicaCount)/2) { + if ((len(c.RegisteredReplicas)) >= c.replicaCount/2) && ((len(c.RegisteredReplicas) + len(c.RegisteredQuorumReplicas)) > (c.quorumReplicaCount+c.replicaCount)/2) { c.signalToAdd() c.StartSignalled = true return nil } - if c.RegisteredReplicas[c.MaxRevReplica].PeerDetail.ReplicaCount == 0 { + if c.RegisteredReplicas[c.MaxRevReplica].PeerDetail.ReplicaCount <= 1 { c.signalToAdd() c.StartSignalled = true return nil @@ -392,7 +396,6 @@ func (c *Controller) addReplicaNoLock(newBackend types.Backend, address string, Address: address, Mode: types.WO, }) - c.replicaCount++ c.backend.AddBackend(address, newBackend) diff --git a/controller/rebuild.go b/controller/rebuild.go index 2a61a644..f967539f 100644 --- a/controller/rebuild.go +++ b/controller/rebuild.go @@ -93,6 +93,13 @@ func (c *Controller) VerifyRebuildReplica(address string) error { } logrus.Debugf("WO replica %v's chain verified, update mode to RW", address) c.setReplicaModeNoLock(address, types.RW) + if len(c.replicas) > c.replicaCount { + c.replicaCount = len(c.replicas) + } + if len(c.quorumReplicas) > c.quorumReplicaCount { + c.quorumReplicaCount = len(c.quorumReplicas) + } + c.backend.UpdatePeerDetails(c.replicaCount, c.quorumReplicaCount) return nil } diff --git a/controller/replicator.go b/controller/replicator.go index 707e03f9..e1fab2fc 100644 --- a/controller/replicator.go +++ b/controller/replicator.go @@ -382,7 +382,7 @@ func (r *replicator) SetQuorumRevisionCounter(address string, counter int64) err return nil } -func (r *replicator) UpdatePeerDetails(replicaCount int64, quorumReplicaCount int64) error { +func (r *replicator) UpdatePeerDetails(replicaCount int, quorumReplicaCount int) error { for _, backend := range r.backends { if err := backend.backend.UpdatePeerDetails(replicaCount, quorumReplicaCount); err != nil { diff --git a/controller/rest/model.go b/controller/rest/model.go index 2ad43f5f..80cbea98 100644 --- a/controller/rest/model.go +++ b/controller/rest/model.go @@ -97,6 +97,7 @@ type RegReplica struct { RevCount string `json:"RevCount"` PeerDetails types.PeerDetails `json:"PeerDetails"` RepType string `json:"RepType"` + RepState string `json:"RepState"` UpTime time.Duration `json:"UpTime"` } diff --git a/controller/rest/replica.go b/controller/rest/replica.go index 89663f09..b417a948 100644 --- a/controller/rest/replica.go +++ b/controller/rest/replica.go @@ -50,7 +50,7 @@ func (s *Server) RegisterReplica(rw http.ResponseWriter, req *http.Request) erro } localRevCount, _ = strconv.ParseInt(regReplica.RevCount, 10, 64) - local := types.RegReplica{Address: regReplica.Address, RevCount: localRevCount, PeerDetail: regReplica.PeerDetails, RepType: regReplica.RepType, UpTime: regReplica.UpTime} + local := types.RegReplica{Address: regReplica.Address, RevCount: localRevCount, PeerDetail: regReplica.PeerDetails, RepType: regReplica.RepType, UpTime: regReplica.UpTime, RepState: regReplica.RepState} if err := s.c.RegisterReplica(local); err != nil { return err } diff --git a/replica/replica.go b/replica/replica.go index afae6bf3..2c860cea 100644 --- a/replica/replica.go +++ b/replica/replica.go @@ -126,6 +126,12 @@ func CreateTempReplica() (*Replica, error) { return r, nil } +func CreateTempServer() (*Server, error) { + return &Server{ + dir: Dir, + }, nil +} + func ReadInfo(dir string) (Info, error) { var info Info err := (&Replica{dir: dir}).unmarshalFile(volumeMetaData, &info) diff --git a/replica/rest/model.go b/replica/rest/model.go index 62c55dca..8879150b 100644 --- a/replica/rest/model.go +++ b/replica/rest/model.go @@ -101,8 +101,8 @@ type ReplicaCounter struct { type PeerDetails struct { client.Resource - ReplicaCount int64 `json:"replicacount"` - QuorumReplicaCount int64 `json:"quorumreplicacount"` + ReplicaCount int `json:"replicacount"` + QuorumReplicaCount int `json:"quorumreplicacount"` } type Action struct { diff --git a/replica/server.go b/replica/server.go index d0e7b044..b84a826b 100644 --- a/replica/server.go +++ b/replica/server.go @@ -152,11 +152,29 @@ func (s *Server) Status() (State, Info) { return Open, info } } + +func (s *Server) PrevStatus() (State, Info) { + info, err := ReadInfo(s.dir) + if os.IsNotExist(err) { + return Initial, Info{} + } else if err != nil { + return Error, Info{} + } + switch { + case info.Rebuilding: + return Rebuilding, info + case info.Dirty: + return Dirty, info + } + + return Closed, info +} + func (s *Server) Stats() *types.Stats { r := s.r return &types.Stats{ RevisionCounter: r.revisionCache, - ReplicaCounter: r.peerCache.ReplicaCount, + ReplicaCounter: int64(r.peerCache.ReplicaCount), } } diff --git a/sync/sync.go b/sync/sync.go index eef72743..18bbf8b6 100644 --- a/sync/sync.go +++ b/sync/sync.go @@ -182,13 +182,15 @@ Register: addr := strings.Split(replicaAddress, "://") parts := strings.Split(addr[1], ":") Replica, _ := replica.CreateTempReplica() + server, _ := replica.CreateTempServer() if volume.ReplicaCount == 0 { revisionCount := Replica.GetRevisionCounter() peerDetails, _ := Replica.GetPeerDetails() replicaType := "quorum" upTime := time.Since(Replica.ReplicaStartTime) - _ = t.client.Register(parts[0], revisionCount, peerDetails, replicaType, upTime) + state, _ := server.PrevStatus() + _ = t.client.Register(parts[0], revisionCount, peerDetails, replicaType, upTime, string(state)) select { case <-ticker.C: goto Register @@ -207,6 +209,7 @@ Register: func (t *Task) AddReplica(replicaAddress string) error { var action string + ticker := time.NewTicker(5 * time.Second) defer ticker.Stop() Register: @@ -217,13 +220,14 @@ Register: addr := strings.Split(replicaAddress, "://") parts := strings.Split(addr[1], ":") Replica, _ := replica.CreateTempReplica() - + server, _ := replica.CreateTempServer() if volume.ReplicaCount == 0 { revisionCount := Replica.GetRevisionCounter() peerDetails, _ := Replica.GetPeerDetails() replicaType := "Backend" upTime := time.Since(Replica.ReplicaStartTime) - t.client.Register(parts[0], revisionCount, peerDetails, replicaType, upTime) + state, _ := server.PrevStatus() + t.client.Register(parts[0], revisionCount, peerDetails, replicaType, upTime, string(state)) select { case <-ticker.C: goto Register diff --git a/types/types.go b/types/types.go index 57b47c99..1591c0d1 100644 --- a/types/types.go +++ b/types/types.go @@ -38,7 +38,7 @@ type Backend interface { GetRevisionCounter() (int64, error) GetVolUsage() (VolUsage, error) SetRevisionCounter(counter int64) error - UpdatePeerDetails(replicaCount int64, quorumReplicaCount int64) error + UpdatePeerDetails(replicaCount int, quorumReplicaCount int) error SetRebuilding(rebuilding bool) error GetMonitorChannel() MonitorChannel StopMonitoring() @@ -83,6 +83,7 @@ type RegReplica struct { UpTime time.Duration RevCount int64 RepType string + RepState string PeerDetail PeerDetails } @@ -114,8 +115,8 @@ type Stats struct { type Interface interface{} type PeerDetails struct { - ReplicaCount int64 - QuorumReplicaCount int64 + ReplicaCount int + QuorumReplicaCount int } type Frontend interface { From 0c4f4461feaf2a59382c8260997e2967c304a1a0 Mon Sep 17 00:00:00 2001 From: Payes Anand Date: Wed, 27 Sep 2017 15:43:15 +0530 Subject: [PATCH 3/3] Resolve golint issues --- app/backup.go | 12 ++---------- backup/main.go | 10 ++-------- controller/rest/replica.go | 5 +---- replica/peer_details.go | 5 +---- replica/replica.go | 11 ++--------- sync/backup.go | 5 +---- sync/sync.go | 10 ++-------- 7 files changed, 11 insertions(+), 47 deletions(-) diff --git a/app/backup.go b/app/backup.go index a4286789..3c2d3677 100644 --- a/app/backup.go +++ b/app/backup.go @@ -121,11 +121,7 @@ func rmBackup(c *cli.Context) error { return fmt.Errorf("Missing required parameter backup") } - if err := task.RmBackup(backup); err != nil { - return err - } - - return nil + return task.RmBackup(backup) } func restoreBackup(c *cli.Context) error { @@ -137,11 +133,7 @@ func restoreBackup(c *cli.Context) error { return fmt.Errorf("Missing required parameter backup") } - if err := task.RestoreBackup(backup); err != nil { - return err - } - - return nil + return task.RestoreBackup(backup) } func inspectBackup(c *cli.Context) error { diff --git a/backup/main.go b/backup/main.go index 2f33ce36..51c6ba59 100644 --- a/backup/main.go +++ b/backup/main.go @@ -238,10 +238,7 @@ func doBackupDelete(c *cli.Context) error { } backupURL = UnescapeURL(backupURL) - if err := backupstore.DeleteDeltaBlockBackup(backupURL); err != nil { - return err - } - return nil + return backupstore.DeleteDeltaBlockBackup(backupURL) } func cmdBackupRestore(c *cli.Context) { @@ -269,10 +266,7 @@ func doBackupRestore(c *cli.Context) error { return err } - if err := createNewSnapshotMetafile(toFile + ".meta"); err != nil { - return err - } - return nil + return createNewSnapshotMetafile(toFile + ".meta") } func createNewSnapshotMetafile(file string) error { diff --git a/controller/rest/replica.go b/controller/rest/replica.go index b417a948..ef709da1 100644 --- a/controller/rest/replica.go +++ b/controller/rest/replica.go @@ -51,11 +51,8 @@ func (s *Server) RegisterReplica(rw http.ResponseWriter, req *http.Request) erro localRevCount, _ = strconv.ParseInt(regReplica.RevCount, 10, 64) local := types.RegReplica{Address: regReplica.Address, RevCount: localRevCount, PeerDetail: regReplica.PeerDetails, RepType: regReplica.RepType, UpTime: regReplica.UpTime, RepState: regReplica.RepState} - if err := s.c.RegisterReplica(local); err != nil { - return err - } + return s.c.RegisterReplica(local) - return nil } func (s *Server) CreateReplica(rw http.ResponseWriter, req *http.Request) error { diff --git a/replica/peer_details.go b/replica/peer_details.go index f890e1ad..6b214189 100644 --- a/replica/peer_details.go +++ b/replica/peer_details.go @@ -31,10 +31,7 @@ func (r *Replica) writePeerDetails(peerDetails types.PeerDetails) error { if r.peerFile == nil { return fmt.Errorf("BUG: peer file wasn't initialized") } - if err := r.encodeToFile(&peerDetails, peerDetailsFile); err != nil { - return err - } - return nil + return r.encodeToFile(&peerDetails, peerDetailsFile) } func (r *Replica) openPeerFile(isCreate bool) error { diff --git a/replica/replica.go b/replica/replica.go index 2c860cea..4fbc75af 100644 --- a/replica/replica.go +++ b/replica/replica.go @@ -303,10 +303,7 @@ func (r *Replica) Resize(obj interface{}) error { } } r.info.Size = sizeInBytes - if err := r.encodeToFile(&r.info, volumeMetaData); err != nil { - return err - } - return nil + return r.encodeToFile(&r.info, volumeMetaData) } func (r *Replica) Reload() (*Replica, error) { @@ -342,11 +339,7 @@ func (r *Replica) RemoveDiffDisk(name string) error { return err } - if err := r.rmDisk(name); err != nil { - return err - } - - return nil + return r.rmDisk(name) } func (r *Replica) hardlinkDisk(target, source string) error { diff --git a/sync/backup.go b/sync/backup.go index 524ead55..43c65539 100644 --- a/sync/backup.go +++ b/sync/backup.go @@ -93,10 +93,7 @@ func (t *Task) RmBackup(backup string) error { return fmt.Errorf("Cannot find a suitable replica for remove backup") } - if err := t.rmBackup(replica, backup); err != nil { - return err - } - return nil + return t.rmBackup(replica, backup) } func (t *Task) rmBackup(replicaInController *rest.Replica, backup string) error { diff --git a/sync/sync.go b/sync/sync.go index 18bbf8b6..d7088529 100644 --- a/sync/sync.go +++ b/sync/sync.go @@ -265,11 +265,8 @@ Register: return err } - if err := t.reloadAndVerify(replicaAddress, toClient); err != nil { - return err - } + return t.reloadAndVerify(replicaAddress, toClient) - return nil } func (t *Task) checkAndResetFailedRebuild(address string) error { @@ -308,10 +305,7 @@ func (t *Task) reloadAndVerify(address string, repClient *replicaClient.ReplicaC return err } - if err := repClient.SetRebuilding(false); err != nil { - return err - } - return nil + return repClient.SetRebuilding(false) } func (t *Task) syncFiles(fromClient *replicaClient.ReplicaClient, toClient *replicaClient.ReplicaClient, disks []string) error {