transactions: Split cancellation and state retrieval

Currently, all state about a transaction is reported on transaction
cancellation. While this is the natural point in time when it should
happen, it is an awkward interface to have the state be returned by the
cancellation function. So let's split up cancellation of transactions
and state retrieval such that callers interestend in the state need to
call `transaction.State()` after the transaction was cancelled.

3 files changed, 28 insertions, 7 deletions

diff --git a/internal/praefect/transactions/manager.go b/internal/praefect/transactions/manager.go
index 0084eee1f..719d4826c 100644
--- a/internal/praefect/transactions/manager.go
+++ b/internal/praefect/transactions/manager.go
@@ -157,9 +157,10 @@ func (mgr *Manager) cancelTransaction(transactionID uint64, transaction *transac
 
 	delete(mgr.transactions, transactionID)
 
+	transaction.cancel()
 	mgr.subtransactionsMetric.Observe(float64(transaction.countSubtransactions()))
 
-	return transaction.cancel(), nil
+	return transaction.State(), nil
 }
 
 func (mgr *Manager) voteTransaction(ctx context.Context, transactionID uint64, node string, hash []byte) error {
diff --git a/internal/praefect/transactions/subtransaction.go b/internal/praefect/transactions/subtransaction.go
index 9950ea53a..58bca0c92 100644
--- a/internal/praefect/transactions/subtransaction.go
+++ b/internal/praefect/transactions/subtransaction.go
@@ -74,22 +74,30 @@ func newSubtransaction(voters []Voter, threshold uint) (*subtransaction, error)
 	}, nil
 }
 
-func (t *subtransaction) cancel() map[string]bool {
+func (t *subtransaction) cancel() {
 	t.lock.Lock()
 	defer t.lock.Unlock()
 
-	results := make(map[string]bool, len(t.votersByNode))
-	for node, voter := range t.votersByNode {
+	for _, voter := range t.votersByNode {
 		// If a voter didn't yet show up or is still undecided, we need
 		// to mark it as failed so it won't get the idea of committing
 		// the transaction at a later point anymore.
 		if voter.result == voteUndecided {
 			voter.result = voteAborted
 		}
-		results[node] = voter.result == voteCommitted
 	}
 
 	close(t.cancelCh)
+}
+
+func (t *subtransaction) state() map[string]bool {
+	t.lock.Lock()
+	defer t.lock.Unlock()
+
+	results := make(map[string]bool, len(t.votersByNode))
+	for node, voter := range t.votersByNode {
+		results[node] = voter.result == voteCommitted
+	}
 
 	return results
 }
diff --git a/internal/praefect/transactions/transaction.go b/internal/praefect/transactions/transaction.go
index feb215c2c..90dc877cb 100644
--- a/internal/praefect/transactions/transaction.go
+++ b/internal/praefect/transactions/transaction.go
@@ -80,7 +80,19 @@ func newTransaction(voters []Voter, threshold uint) (*transaction, error) {
 	}, nil
 }
 
-func (t *transaction) cancel() map[string]bool {
+func (t *transaction) cancel() {
+	t.lock.Lock()
+	defer t.lock.Unlock()
+
+	for _, subtransaction := range t.subtransactions {
+		subtransaction.cancel()
+	}
+}
+
+// State returns the voting state mapped by voters. A voting state of `true`
+// means all subtransactions were successful, a voting state of `false` means
+// either no subtransactions were created or any of the subtransactions failed.
+func (t *transaction) State() map[string]bool {
 	t.lock.Lock()
 	defer t.lock.Unlock()
 
@@ -91,7 +103,7 @@ func (t *transaction) cancel() map[string]bool {
 	// node as well. Otherwise, if all subtransactions for the node
 	// succeeded, the transaction did as well.
 	for _, subtransaction := range t.subtransactions {
-		for voter, result := range subtransaction.cancel() {
+		for voter, result := range subtransaction.state() {
 			// If there already is an entry indicating failure, keep it.
 			if didSucceed, ok := results[voter]; ok && !didSucceed {
 				continue