Remove isDone field from subtransaction type

isDone field in the subtransaction indicates whether the subtransaction
is already finished or not. It's set always when closing the doneCh, which
is used to signal to voters waiting for a subtransaction that it's finished.
Given the above, isDone really just duplicates the state of whether the doneCh
has been closed or not. This commit replaces isDone with a method that checks
doneCh's state to remove the duplication.

2 files changed, 19 insertions, 12 deletions

diff --git a/internal/praefect/transactions/subtransaction.go b/internal/praefect/transactions/subtransaction.go
index 4193ea1f2..981746ad7 100644
--- a/internal/praefect/transactions/subtransaction.go
+++ b/internal/praefect/transactions/subtransaction.go
@@ -35,7 +35,6 @@ type subtransaction struct {
 	lock         sync.RWMutex
 	votersByNode map[string]*Voter
 	voteCounts   map[voting.Vote]uint
-	isDone       bool
 }
 
 func newSubtransaction(voters []Voter, threshold uint) (*subtransaction, error) {
@@ -66,8 +65,7 @@ func (t *subtransaction) cancel() {
 		}
 	}
 
-	if !t.isDone {
-		t.isDone = true
+	if !t.isDone() {
 		close(t.doneCh)
 	}
 }
@@ -93,8 +91,7 @@ func (t *subtransaction) stop() error {
 		}
 	}
 
-	if !t.isDone {
-		t.isDone = true
+	if !t.isDone() {
 		close(t.doneCh)
 	}
 
@@ -152,7 +149,6 @@ func (t *subtransaction) vote(node string, vote voting.Vote) error {
 	t.updateVoterStates()
 
 	if t.mustSignalVoters() {
-		t.isDone = true
 		close(t.doneCh)
 	}
 
@@ -216,7 +212,7 @@ func (t *subtransaction) updateVoterStates() {
 func (t *subtransaction) mustSignalVoters() bool {
 	// If somebody else already notified voters, then we mustn't do so
 	// again.
-	if t.isDone {
+	if t.isDone() {
 		return false
 	}
 
@@ -286,6 +282,15 @@ func (t *subtransaction) collectVotes(ctx context.Context, node string) error {
 	}
 }
 
+func (t *subtransaction) isDone() bool {
+	select {
+	case <-t.doneCh:
+		return true
+	default:
+		return false
+	}
+}
+
 func (t *subtransaction) getResult(node string) (VoteResult, error) {
 	t.lock.RLock()
 	defer t.lock.RUnlock()
diff --git a/internal/praefect/transactions/subtransaction_test.go b/internal/praefect/transactions/subtransaction_test.go
index 07a5c6e36..5ddf5eeec 100644
--- a/internal/praefect/transactions/subtransaction_test.go
+++ b/internal/praefect/transactions/subtransaction_test.go
@@ -23,7 +23,7 @@ func TestSubtransaction_cancel(t *testing.T) {
 
 	s.cancel()
 
-	require.True(t, s.isDone)
+	require.True(t, s.isDone())
 	require.Equal(t, VoteCanceled, s.votersByNode["1"].result)
 	require.Equal(t, VoteCommitted, s.votersByNode["2"].result)
 	require.Equal(t, VoteFailed, s.votersByNode["3"].result)
@@ -41,7 +41,7 @@ func TestSubtransaction_stop(t *testing.T) {
 
 		require.NoError(t, s.stop())
 
-		require.True(t, s.isDone)
+		require.True(t, s.isDone())
 		require.Equal(t, VoteStopped, s.votersByNode["1"].result)
 		require.Equal(t, VoteCommitted, s.votersByNode["2"].result)
 		require.Equal(t, VoteFailed, s.votersByNode["3"].result)
@@ -57,7 +57,7 @@ func TestSubtransaction_stop(t *testing.T) {
 		require.NoError(t, err)
 
 		require.Equal(t, s.stop(), ErrTransactionCanceled)
-		require.False(t, s.isDone)
+		require.False(t, s.isDone())
 	})
 
 	t.Run("stop of stopped transaction fails", func(t *testing.T) {
@@ -70,7 +70,7 @@ func TestSubtransaction_stop(t *testing.T) {
 		require.NoError(t, err)
 
 		require.Equal(t, s.stop(), ErrTransactionStopped)
-		require.False(t, s.isDone)
+		require.False(t, s.isDone())
 	})
 }
 
@@ -401,7 +401,9 @@ func TestSubtransaction_mustSignalVoters(t *testing.T) {
 			}
 
 			s.voteCounts = voteCounts
-			s.isDone = tc.isDone
+			if tc.isDone {
+				close(s.doneCh)
+			}
 
 			require.Equal(t, tc.mustSignal, s.mustSignalVoters())
 		})