From 6a8f2198c7b84a52fdb52e971203168610cd8cd6 Mon Sep 17 00:00:00 2001
From: Ishan Tyagi <42602577+ishan16696@users.noreply.github.com>
Date: Mon, 15 Jul 2024 19:02:36 +0530
Subject: [PATCH] Do not check SSR State when Snapshotter should be stopped
 (#680) (#750)

* Specify send/recv

Allows type checking of channel interactions

* Close instead of sending on channel

I think that closing the channel makes it clearer that this channels purpose has been fulfilled and it no longer needs to be considered. All the places that use the channel are recieving all values from the channel so they will continue to function in the same way

* Do not check ssr state in handleSsrStopRequest

The snapshotter is not set to active state until an initial delta
snapshot or a fullsnapshot has been taken. If a leadership election
takes place while the snapshot is being taken and the current leader
loses that election, then handleSsrRequest does not send on the
ssrStopCh. This in turn causes ssr.snapshotEventHandler to never return
and snapshots would continue to be taken.

Co-authored-by: Alex Vest <avestuk@gmail.com>
---
 pkg/server/backuprestoreserver.go | 16 +++++-----------
 1 file changed, 5 insertions(+), 11 deletions(-)

diff --git a/pkg/server/backuprestoreserver.go b/pkg/server/backuprestoreserver.go
index 0bd5001f8..a32bd8fa7 100644
--- a/pkg/server/backuprestoreserver.go
+++ b/pkg/server/backuprestoreserver.go
@@ -340,7 +340,7 @@ func (b *BackupRestoreServer) runServer(ctx context.Context, restoreOpts *brtype
 
 // runEtcdProbeLoopWithSnapshotter runs the etcd probe loop
 // for the case when backup-restore becomes leading sidecar.
-func (b *BackupRestoreServer) runEtcdProbeLoopWithSnapshotter(ctx context.Context, handler *HTTPHandler, ssr *snapshotter.Snapshotter, ss brtypes.SnapStore, ssrStopCh chan struct{}, ackCh chan struct{}) {
+func (b *BackupRestoreServer) runEtcdProbeLoopWithSnapshotter(ctx context.Context, handler *HTTPHandler, ssr *snapshotter.Snapshotter, ss brtypes.SnapStore, ssrStopCh <-chan struct{}, ackCh chan<- struct{}) {
 	var (
 		err                       error
 		initialDeltaSnapshotTaken bool
@@ -523,7 +523,7 @@ func handleAckState(handler *HTTPHandler, ackCh chan struct{}) {
 }
 
 // handleSsrStopRequest responds to handlers request and stop interrupt.
-func handleSsrStopRequest(ctx context.Context, handler *HTTPHandler, ssr *snapshotter.Snapshotter, ackCh, ssrStopCh chan struct{}, logger *logrus.Entry) {
+func handleSsrStopRequest(ctx context.Context, handler *HTTPHandler, ssr *snapshotter.Snapshotter, ackCh chan<- struct{}, ssrStopCh chan<- struct{}, logger *logrus.Entry) {
 	logger.Info("Starting the handleSsrStopRequest handler...")
 	for {
 		var ok bool
@@ -533,15 +533,9 @@ func handleSsrStopRequest(ctx context.Context, handler *HTTPHandler, ssr *snapsh
 			logger.Infof("handleSsrStopRequest: %v", ctx.Err())
 		}
 
-		ssr.SsrStateMutex.Lock()
-		if ssr.SsrState == brtypes.SnapshotterActive {
-			ssr.SsrStateMutex.Unlock()
-			ssrStopCh <- emptyStruct
-		} else {
-			ssr.SsrState = brtypes.SnapshotterInactive
-			ssr.SsrStateMutex.Unlock()
-			ackCh <- emptyStruct
-		}
+		ackCh <- emptyStruct
+		close(ssrStopCh)
+
 		if !ok {
 			logger.Info("Stopping handleSsrStopRequest handler...")
 			return