From e0bed3f93e941c4fcec7028a45b709bbf873b1ad Mon Sep 17 00:00:00 2001
From: Periyasamy Palanisamy <pepalani@redhat.com>
Date: Tue, 26 Sep 2023 12:31:26 +0200
Subject: [PATCH] Do not send echo when inactivity check in progress

This commit skips spawning another goroutine to send echo when another
echo check is already in progress. It also throws error if inactivity
timeout value is lesser than reconnect timeout value while initializing
client with WithInactivityCheck option.

Signed-off-by: Periyasamy Palanisamy <pepalani@redhat.com>
---
 .github/workflows/ci.yml | 12 ++++++------
 client/client.go         | 11 ++++++++++-
 client/options.go        |  4 ++++
 go.mod                   |  2 +-
 4 files changed, 21 insertions(+), 8 deletions(-)

diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index bdbac266..251d7cfd 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -12,10 +12,10 @@ jobs:
     runs-on: ubuntu-latest
     steps:
 
-    - name: Set up Go 1.18
-      uses: actions/setup-go@v2
+    - name: Set up Go 1.19.6
+      uses: actions/setup-go@v3
       with:
-        go-version: 1.18
+        go-version: 1.19.6
       id: go
 
     - name: Install benchstat
@@ -82,10 +82,10 @@ jobs:
     runs-on: ubuntu-latest
     
     steps:
-    - name: Set up Go 1.18
-      uses: actions/setup-go@v1
+    - name: Set up Go 1.19.6
+      uses: actions/setup-go@v3
       with:
-        go-version: 1.18
+        go-version: 1.19.6
       id: go
 
     - name: Check out code into the Go module directory
diff --git a/client/client.go b/client/client.go
index 8c9a1051..56b43f15 100644
--- a/client/client.go
+++ b/client/client.go
@@ -13,6 +13,7 @@ import (
 	"reflect"
 	"strings"
 	"sync"
+	"sync/atomic"
 	"time"
 
 	"github.com/cenkalti/backoff/v4"
@@ -1202,6 +1203,7 @@ func (o *ovsdbClient) handleInactivityProbes() {
 	stopCh := o.stopCh
 	trafficSeen := o.trafficSeen
 	timer := time.NewTimer(o.options.inactivityTimeout)
+	var echoInProgress atomic.Bool
 	for {
 		select {
 		case <-stopCh:
@@ -1212,8 +1214,14 @@ func (o *ovsdbClient) handleInactivityProbes() {
 				<-timer.C
 			}
 		case <-timer.C:
-			// Otherwise send an echo in a goroutine so that transactions don't block
+			// When echo request is already in the fly, ignore sending another
+			// echo message
+			if echoInProgress.Load() {
+				continue
+			}
+			// Send an echo in a goroutine so that transactions don't block
 			go func() {
+				echoInProgress.Store(true)
 				ctx, cancel := context.WithTimeout(context.Background(), o.options.timeout)
 				err := o.Echo(ctx)
 				if err != nil {
@@ -1221,6 +1229,7 @@ func (o *ovsdbClient) handleInactivityProbes() {
 					o.Disconnect()
 				}
 				cancel()
+				echoInProgress.Store(false)
 			}()
 		}
 		timer.Reset(o.options.inactivityTimeout)
diff --git a/client/options.go b/client/options.go
index 81ccffe2..6f9b7b17 100644
--- a/client/options.go
+++ b/client/options.go
@@ -2,6 +2,7 @@ package client
 
 import (
 	"crypto/tls"
+	"errors"
 	"net/url"
 	"time"
 
@@ -120,6 +121,9 @@ func WithReconnect(timeout time.Duration, backoff backoff.BackOff) Option {
 func WithInactivityCheck(inactivityTimeout, reconnectTimeout time.Duration,
 	reconnectBackoff backoff.BackOff) Option {
 	return func(o *options) error {
+		if reconnectTimeout >= inactivityTimeout {
+			return errors.New("inactivity timeout value should be greater than reconnect timeout value")
+		}
 		o.reconnect = true
 		o.timeout = reconnectTimeout
 		o.backoff = reconnectBackoff
diff --git a/go.mod b/go.mod
index 1fd49840..a9c12881 100644
--- a/go.mod
+++ b/go.mod
@@ -1,6 +1,6 @@
 module github.com/ovn-org/libovsdb
 
-go 1.18
+go 1.19
 
 require (
 	github.com/cenkalti/backoff/v4 v4.1.3