From f5fb13579309d5e55feecfda8e592cc38d73439a Mon Sep 17 00:00:00 2001 From: James Shubin Date: Tue, 30 Aug 2016 03:44:02 -0400 Subject: [PATCH] converger: Update the API for errors and naming We updated the API and behaviour to make two changes: 1) We remove the log.* stuff, and replace the Fatal calls with straight calls to panic(). These are meant to be like an assert, and shouldn't happen unless there is a user error or a bug. 2) We made the !uuid.IsValid() checks return an error instead of causing a panic. These returns errors instead, and makes the process safer for users who are okay calling a function that won't have an effect. --- converger.go | 49 ++++++++++++++++++++++++++++++++----------------- etcd.go | 2 ++ 2 files changed, 34 insertions(+), 17 deletions(-) diff --git a/converger.go b/converger.go index a475dc5e..ecbf54b2 100644 --- a/converger.go +++ b/converger.go @@ -18,7 +18,7 @@ package main import ( - "log" + "fmt" "sync" "time" ) @@ -26,8 +26,8 @@ import ( // Converger is the general interface for implementing a convergence watcher type Converger interface { // TODO: need a better name Register() ConvergerUUID - IsConverged(ConvergerUUID) bool // is the UUID converged ? - SetConverged(ConvergerUUID, bool) // set the converged state of the UUID + IsConverged(ConvergerUUID) bool // is the UUID converged ? + SetConverged(ConvergerUUID, bool) error // set the converged state of the UUID Unregister(ConvergerUUID) Start() Pause() @@ -38,11 +38,13 @@ type Converger interface { // TODO: need a better name // ConvergerUUID is the interface resources can use to notify with if converged // you'll need to use part of the Converger interface to Register initially too type ConvergerUUID interface { - ID() uint64 // get Id + ID() uint64 // get Id + Name() string // get a friendly name + SetName(string) IsValid() bool // has Id been initialized ? InvalidateID() // set Id to nil IsConverged() bool - SetConverged(bool) + SetConverged(bool) error Unregister() ConvergedTimer() <-chan time.Time } @@ -62,6 +64,7 @@ type converger struct { type convergerUUID struct { converger Converger id uint64 + name string // user defined, friendly name } // NewConverger builds a new converger struct @@ -85,31 +88,32 @@ func (obj *converger) Register() ConvergerUUID { return &convergerUUID{ converger: obj, id: obj.lastid, + name: fmt.Sprintf("%d", obj.lastid), // some default } } // IsConverged gets the converged status of a uuid func (obj *converger) IsConverged(uuid ConvergerUUID) bool { if !uuid.IsValid() { - log.Fatal("Id of ConvergerUUID is nil!") + panic(fmt.Sprintf("Id of ConvergerUUID(%s) is nil!", uuid.Name())) } obj.mutex.RLock() isConverged, found := obj.status[uuid.ID()] // lookup obj.mutex.RUnlock() if !found { - log.Fatal("Id of ConvergerUUID is unregistered!") + panic("Id of ConvergerUUID is unregistered!") } return isConverged } // SetConverged updates the converger with the converged state of the UUID -func (obj *converger) SetConverged(uuid ConvergerUUID, isConverged bool) { +func (obj *converger) SetConverged(uuid ConvergerUUID, isConverged bool) error { if !uuid.IsValid() { - log.Fatal("Id of ConvergerUUID is nil!") + return fmt.Errorf("Id of ConvergerUUID(%s) is nil!", uuid.Name()) } obj.mutex.Lock() if _, found := obj.status[uuid.ID()]; !found { - log.Fatal("Id of ConvergerUUID is unregistered!") + panic("Id of ConvergerUUID is unregistered!") } obj.status[uuid.ID()] = isConverged // set obj.mutex.Unlock() // unlock *before* poke or deadlock! @@ -118,6 +122,7 @@ func (obj *converger) SetConverged(uuid ConvergerUUID, isConverged bool) { // this allows us to send events, even if we haven't started... go func() { obj.channel <- struct{}{} }() } + return nil } // isConverged returns true if *every* registered uuid has converged @@ -135,7 +140,7 @@ func (obj *converger) isConverged() bool { // Unregister dissociates the ConvergedUUID from the converged checking func (obj *converger) Unregister(uuid ConvergerUUID) { if !uuid.IsValid() { - log.Fatal("Id of ConvergerUUID is nil!") + panic(fmt.Sprintf("Id of ConvergerUUID(%s) is nil!", uuid.Name())) } obj.mutex.Lock() delete(obj.status, uuid.ID()) @@ -160,13 +165,13 @@ func (obj *converger) Pause() { // FIXME: add a sync ACK on pause before return // have joined the map, then it might appears as if we converged before we did! func (obj *converger) Loop(startPaused bool) { if obj.control == nil { - log.Fatal("Converger not initialized correctly") + panic("Converger not initialized correctly") } if startPaused { // start paused without racing select { case e := <-obj.control: if !e { - log.Fatal("Converger expected true!") + panic("Converger expected true!") } } } @@ -174,13 +179,13 @@ func (obj *converger) Loop(startPaused bool) { select { case e := <-obj.control: // expecting "false" which means pause! if e { - log.Fatal("Converger expected false!") + panic("Converger expected false!") } // now i'm paused... select { case e := <-obj.control: if !e { - log.Fatal("Converger expected true!") + panic("Converger expected true!") } // restart // kick once to refresh the check... @@ -230,6 +235,16 @@ func (obj *convergerUUID) ID() uint64 { return obj.id } +// Name returns a user defined name for the specific convergerUUID. +func (obj *convergerUUID) Name() string { + return obj.name +} + +// SetName sets a user defined name for the specific convergerUUID. +func (obj *convergerUUID) SetName(name string) { + obj.name = name +} + // IsValid tells us if the id is valid or has already been destroyed func (obj *convergerUUID) IsValid() bool { return obj.id != 0 // an id of 0 is invalid @@ -246,8 +261,8 @@ func (obj *convergerUUID) IsConverged() bool { } // SetConverged is a helper function to the regular SetConverged notification -func (obj *convergerUUID) SetConverged(isConverged bool) { - obj.converger.SetConverged(obj, isConverged) +func (obj *convergerUUID) SetConverged(isConverged bool) error { + return obj.converger.SetConverged(obj, isConverged) } // Unregister is a helper function to unregister myself diff --git a/etcd.go b/etcd.go index 4b855ba1..1e593fc2 100644 --- a/etcd.go +++ b/etcd.go @@ -678,6 +678,7 @@ func (obj *EmbdEtcd) CtxError(ctx context.Context, err error) (context.Context, // CbLoop is the loop where callback execution is serialized func (obj *EmbdEtcd) CbLoop() { cuuid := obj.converger.Register() + cuuid.SetName("Etcd: CbLoop") defer cuuid.Unregister() if e := obj.Connect(false); e != nil { return // fatal @@ -731,6 +732,7 @@ func (obj *EmbdEtcd) CbLoop() { // Loop is the main loop where everything is serialized func (obj *EmbdEtcd) Loop() { cuuid := obj.converger.Register() + cuuid.SetName("Etcd: Loop") defer cuuid.Unregister() if e := obj.Connect(false); e != nil { return // fatal