retry: Execute on error callbacks for retryable errors only

All of our error callbacks are used to log the error and indicate that
we are retrying. Previously, in the case of context errors or
non-retryable errors, we would have called these too, which would have
resulted in misleading log messages.

pkg/config/redis.go

@@ -86,7 +86,7 @@ func dialWithLogging(dialer ctxDialerFunc, logger *logging.Logger) ctxDialerFunc
 			backoff.NewExponentialWithJitter(1*time.Millisecond, 1*time.Second),
 			retry.Settings{
 				Timeout: retry.DefaultTimeout,
-				OnError: func(_ time.Duration, _ uint64, err, lastErr error) {
+				OnRetryableError: func(_ time.Duration, _ uint64, err, lastErr error) {
 					if lastErr == nil || err.Error() != lastErr.Error() {
 						logger.Warnw("Can't connect to Redis. Retrying", zap.Error(err))
 					}

pkg/icingadb/db.go

@@ -674,7 +674,7 @@ func (db *DB) GetSemaphoreForTable(table string) *semaphore.Weighted {
 func (db *DB) getDefaultRetrySettings() retry.Settings {
 	return retry.Settings{
 		Timeout: retry.DefaultTimeout,
-		OnError: func(_ time.Duration, _ uint64, err, lastErr error) {
+		OnRetryableError: func(_ time.Duration, _ uint64, err, lastErr error) {
 			if lastErr == nil || err.Error() != lastErr.Error() {
 				db.logger.Warnw("Can't execute query. Retrying", zap.Error(err))
 			}

pkg/icingadb/driver.go

@@ -56,7 +56,7 @@ func (c RetryConnector) Connect(ctx context.Context) (driver.Conn, error) {
 		backoff.NewExponentialWithJitter(time.Millisecond*128, time.Minute*1),
 		retry.Settings{
 			Timeout: retry.DefaultTimeout,
-			OnError: func(_ time.Duration, _ uint64, err, lastErr error) {
+			OnRetryableError: func(_ time.Duration, _ uint64, err, lastErr error) {
 				telemetry.UpdateCurrentDbConnErr(err)
 
 				if lastErr == nil || err.Error() != lastErr.Error() {

pkg/icingadb/ha.go

@@ -387,7 +387,7 @@ func (h *HA) realize(
 		backoff.NewExponentialWithJitter(time.Millisecond*256, time.Second*3),
 		retry.Settings{
 			// Intentionally no timeout is set, as we use a context with a deadline.
-			OnError: func(_ time.Duration, attempt uint64, err, lastErr error) {
+			OnRetryableError: func(_ time.Duration, attempt uint64, err, lastErr error) {
 				if lastErr == nil || err.Error() != lastErr.Error() {
 					log := h.logger.Debugw
 					if attempt > 2 {

pkg/retry/retry.go

@@ -28,9 +28,9 @@ type Settings struct {
 	// This means that WithBackoff may not stop exactly after Timeout expires,
 	// or may not retry at all if the first execution of RetryableFunc already takes longer than Timeout.
 	Timeout time.Duration
-	// OnError is called if an error occurs.
-	OnError func(elapsed time.Duration, attempt uint64, err, lastErr error)
-	// OnSuccess is called once the operation succeeds.
+	// OnRetryableError is called if a retryable error occurs.
+	OnRetryableError func(elapsed time.Duration, attempt uint64, err, lastErr error)
+	// OnSuccess is called once the operation succeeds after a retryable error.
 	OnSuccess func(elapsed time.Duration, attempt uint64, lastErr error)
 }
 
@@ -61,10 +61,6 @@ func WithBackoff(
 			return
 		}
 
-		if settings.OnError != nil {
-			settings.OnError(time.Since(start), attempt, err, prevErr)
-		}
-
 		if errors.Is(err, context.DeadlineExceeded) || errors.Is(err, context.Canceled) {
 			if prevErr != nil {
 				err = errors.Wrap(err, prevErr.Error())
@@ -79,6 +75,10 @@ func WithBackoff(
 			return
 		}
 
+		if settings.OnRetryableError != nil {
+			settings.OnRetryableError(time.Since(start), attempt, err, prevErr)
+		}
+
 		select {
 		case <-time.After(b(attempt)):
 		case <-timeout: