From ef8dd91077f05cabaef0e40f6d53e318fb1b88ae Mon Sep 17 00:00:00 2001
From: Seebs <seebs@seebs.net>
Date: Wed, 10 Mar 2021 16:43:02 -0600
Subject: [PATCH] report errors more clearly when trying to recreate leases

There's some loose ends here because really we probably want to be
using the top-level server logger, and we should fix that, but in the
mean time, let's not swallow the errors as much, because the last
line printed doesn't actually show what the error was, but it could.

To do this, we distinguish between the current error (which might
be a wrapper around DeadlineExceeded) and a previous error which
we might prefer to return, if one exists, since it's more likely
the "real" cause.
---
 etcd/leasedkv.go | 31 ++++++++++++++++---------------
 1 file changed, 16 insertions(+), 15 deletions(-)

diff --git a/etcd/leasedkv.go b/etcd/leasedkv.go
index 3f0703b64..858c000d8 100644
--- a/etcd/leasedkv.go
+++ b/etcd/leasedkv.go
@@ -100,7 +100,7 @@ func (l *leasedKV) consumeLease(ch <-chan *clientv3.LeaseKeepAliveResponse) {
 				return
 			}
 
-			if ok := retry(1*time.Second, func() error {
+			if e := retry("consumeLease", 1*time.Second, func() error {
 				kaChann, err := l.create(l.value)
 				if err != nil {
 					return err
@@ -108,13 +108,13 @@ func (l *leasedKV) consumeLease(ch <-chan *clientv3.LeaseKeepAliveResponse) {
 
 				go l.consumeLease(kaChann)
 				return nil
-			}); !ok {
-				log.Println("lease cannot be recreated. Key:", l.key)
+			}); e != nil {
+				log.Printf("lease %q cannot be recreated: %v", l.key, e)
 				l.mu.Unlock()
 				return
 			}
 
-			log.Println("lease recreated after a problem. Key:", l.key)
+			log.Printf("lease %q recreated after a problem", l.key)
 			l.mu.Unlock()
 			return
 		}
@@ -172,20 +172,21 @@ func (l *leasedKV) Get(ctx context.Context) (string, error) {
 	return l.value, nil
 }
 
-func retry(sleep time.Duration, f func() error) bool {
+func retry(desc string, sleep time.Duration, f func() error) (err error) {
 	for {
-		err := f()
-		if err == nil {
-			return true
+		lastErr := f()
+		if lastErr == nil {
+			return lastErr
 		}
-
-		// sometimes the element in charge of stopping the lease renewal doesn't do it, causing context errors.
-		if errors.Is(err, context.DeadlineExceeded) {
-			return false
+		if errors.Is(lastErr, context.DeadlineExceeded) {
+			if err != nil {
+				return err
+			} else {
+				return lastErr
+			}
 		}
-
+		log.Printf("%s: got error %v, retrying", desc, lastErr)
+		err = lastErr
 		time.Sleep(sleep)
-
-		log.Println("retrying after error:", err)
 	}
 }