transactions: Discern cancellation and failure

When a transaction gets cancelled or when the transaction failed to
reach quorum, then we both account this under the `VoteAborted` result.
It is thus impossible to tell apart both cases, which is especially bad
when one tries to comprehend logs.

This commit thus splits the state into two, `VoteFailed` which
represents that the vote did not reach majority. And `VoteCanceled`,
which represents that the vote was cancelled before majority was
reached.

3 files changed, 43 insertions, 34 deletions

diff --git a/internal/praefect/transaction_test.go b/internal/praefect/transaction_test.go
index 5023a5aa8..35bdececf 100644
--- a/internal/praefect/transaction_test.go
+++ b/internal/praefect/transaction_test.go
@@ -446,10 +446,10 @@ func TestTransactionReachesQuorum(t *testing.T) {
 
 func TestTransactionWithMultipleVotes(t *testing.T) {
 	type multiVoter struct {
-		voteCount     uint
-		votes         []string
-		voteSucceeds  []bool
-		shouldSucceed bool
+		voteCount      uint
+		votes          []string
+		voteSucceeds   []bool
+		expectedResult transactions.VoteResult
 	}
 
 	tc := []struct {
@@ -460,33 +460,33 @@ func TestTransactionWithMultipleVotes(t *testing.T) {
 		{
 			desc: "quorum is reached with multiple votes",
 			voters: []multiVoter{
-				{voteCount: 1, votes: []string{"foo", "bar"}, voteSucceeds: []bool{true, true}, shouldSucceed: true},
-				{voteCount: 1, votes: []string{"foo", "bar"}, voteSucceeds: []bool{true, true}, shouldSucceed: true},
+				{voteCount: 1, votes: []string{"foo", "bar"}, voteSucceeds: []bool{true, true}, expectedResult: transactions.VoteCommitted},
+				{voteCount: 1, votes: []string{"foo", "bar"}, voteSucceeds: []bool{true, true}, expectedResult: transactions.VoteCommitted},
 			},
 			threshold: 2,
 		},
 		{
 			desc: "quorum is not reached with disagreeing votes",
 			voters: []multiVoter{
-				{voteCount: 1, votes: []string{"foo", "bar"}, voteSucceeds: []bool{true, false}, shouldSucceed: false},
-				{voteCount: 1, votes: []string{"foo", "rab"}, voteSucceeds: []bool{true, false}, shouldSucceed: false},
+				{voteCount: 1, votes: []string{"foo", "bar"}, voteSucceeds: []bool{true, false}, expectedResult: transactions.VoteFailed},
+				{voteCount: 1, votes: []string{"foo", "rab"}, voteSucceeds: []bool{true, false}, expectedResult: transactions.VoteFailed},
 			},
 			threshold: 2,
 		},
 		{
 			desc: "quorum is reached with unweighted disagreeing voter",
 			voters: []multiVoter{
-				{voteCount: 1, votes: []string{"foo", "bar", "qux"}, voteSucceeds: []bool{true, true, true}, shouldSucceed: true},
-				{voteCount: 0, votes: []string{"foo", "rab"}, voteSucceeds: []bool{true, false}, shouldSucceed: false},
+				{voteCount: 1, votes: []string{"foo", "bar", "qux"}, voteSucceeds: []bool{true, true, true}, expectedResult: transactions.VoteCommitted},
+				{voteCount: 0, votes: []string{"foo", "rab"}, voteSucceeds: []bool{true, false}, expectedResult: transactions.VoteCanceled},
 			},
 			threshold: 1,
 		},
 		{
 			desc: "quorum is reached with outweighed disagreeing voter",
 			voters: []multiVoter{
-				{voteCount: 1, votes: []string{"foo", "bar", "qux"}, voteSucceeds: []bool{true, true, true}, shouldSucceed: true},
-				{voteCount: 1, votes: []string{"foo", "bar", "qux"}, voteSucceeds: []bool{true, true, true}, shouldSucceed: true},
-				{voteCount: 1, votes: []string{"foo", "rab"}, voteSucceeds: []bool{true, false}, shouldSucceed: false},
+				{voteCount: 1, votes: []string{"foo", "bar", "qux"}, voteSucceeds: []bool{true, true, true}, expectedResult: transactions.VoteCommitted},
+				{voteCount: 1, votes: []string{"foo", "bar", "qux"}, voteSucceeds: []bool{true, true, true}, expectedResult: transactions.VoteCommitted},
+				{voteCount: 1, votes: []string{"foo", "rab"}, voteSucceeds: []bool{true, false}, expectedResult: transactions.VoteCanceled},
 			},
 			threshold: 2,
 		},
@@ -546,11 +546,7 @@ func TestTransactionWithMultipleVotes(t *testing.T) {
 			results, err := transaction.State()
 			require.NoError(t, err)
 			for i, voter := range tc.voters {
-				if voter.shouldSucceed {
-					require.Equal(t, transactions.VoteCommitted, results[fmt.Sprintf("node-%d", i)])
-				} else {
-					require.Equal(t, transactions.VoteAborted, results[fmt.Sprintf("node-%d", i)])
-				}
+				require.Equal(t, voter.expectedResult, results[fmt.Sprintf("node-%d", i)])
 			}
 		})
 	}
@@ -682,8 +678,10 @@ func TestTransactionCancellation(t *testing.T) {
 			for i, v := range tc.voters {
 				if v.shouldSucceed {
 					require.Equal(t, transactions.VoteCommitted, results[fmt.Sprintf("node-%d", i)], "result mismatches expected node state")
+				} else if v.showsUp {
+					require.Equal(t, transactions.VoteFailed, results[fmt.Sprintf("node-%d", i)], "result mismatches expected node state")
 				} else {
-					require.Equal(t, transactions.VoteAborted, results[fmt.Sprintf("node-%d", i)], "result mismatches expected node state")
+					require.Equal(t, transactions.VoteCanceled, results[fmt.Sprintf("node-%d", i)], "result mismatches expected node state")
 				}
 			}
 
diff --git a/internal/praefect/transactions/subtransaction.go b/internal/praefect/transactions/subtransaction.go
index 665001f70..5f46d55a4 100644
--- a/internal/praefect/transactions/subtransaction.go
+++ b/internal/praefect/transactions/subtransaction.go
@@ -25,8 +25,11 @@ const (
 	VoteUndecided VoteResult = iota
 	// VoteCommitted means that the voter committed his vote.
 	VoteCommitted
-	// VoteAborted means that the voter aborted his vote.
-	VoteAborted
+	// VoteFailed means that the voter has failed the vote because a
+	// majority of nodes has elected a different result.
+	VoteFailed
+	// VoteCanceled means that the transaction was cancelled.
+	VoteCanceled
 	// VoteStopped means that the transaction was gracefully stopped.
 	VoteStopped
 )
@@ -87,7 +90,7 @@ func (t *subtransaction) cancel() {
 		// 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
+			voter.result = VoteCanceled
 		}
 	}
 
@@ -100,8 +103,8 @@ func (t *subtransaction) stop() error {
 
 	for _, voter := range t.votersByNode {
 		switch voter.result {
-		case VoteAborted:
-			// If the vote was aborted already, we cannot stop it.
+		case VoteCanceled:
+			// If the vote was canceled already, we cannot stop it.
 			return ErrTransactionCanceled
 		case VoteStopped:
 			// Similar if the vote was stopped already.
@@ -109,7 +112,7 @@ func (t *subtransaction) stop() error {
 		case VoteUndecided:
 			// Undecided voters will get stopped, ...
 			voter.result = VoteStopped
-		case VoteCommitted:
+		case VoteCommitted, VoteFailed:
 			// ... while decided voters cannot be changed anymore.
 			continue
 		}
@@ -212,9 +215,9 @@ func (t *subtransaction) collectVotes(ctx context.Context, node string) error {
 	switch voter.result {
 	case VoteUndecided:
 		// Happy case, no decision was yet made.
-	case VoteAborted:
+	case VoteCanceled:
 		// It may happen that the vote was cancelled or stopped just after majority was
-		// reached. In that case, the node's state is now VoteAborted/VoteStopped, so we
+		// reached. In that case, the node's state is now VoteCanceled/VoteStopped, so we
 		// have to return an error here.
 		return ErrTransactionCanceled
 	case VoteStopped:
@@ -226,7 +229,7 @@ func (t *subtransaction) collectVotes(ctx context.Context, node string) error {
 	// See if our vote crossed the threshold. As there can be only one vote
 	// exceeding it, we know we're the winner in that case.
 	if t.voteCounts[voter.vote] < t.threshold {
-		voter.result = VoteAborted
+		voter.result = VoteFailed
 		return fmt.Errorf("%w: got %d/%d votes", ErrTransactionVoteFailed, t.voteCounts[voter.vote], t.threshold)
 	}
 
@@ -240,7 +243,7 @@ func (t *subtransaction) getResult(node string) (VoteResult, error) {
 
 	voter, ok := t.votersByNode[node]
 	if !ok {
-		return VoteAborted, fmt.Errorf("invalid node for transaction: %q", node)
+		return VoteCanceled, fmt.Errorf("invalid node for transaction: %q", node)
 	}
 
 	return voter.result, nil
diff --git a/internal/praefect/transactions/transaction.go b/internal/praefect/transactions/transaction.go
index ae63947b4..3f0dd7f97 100644
--- a/internal/praefect/transactions/transaction.go
+++ b/internal/praefect/transactions/transaction.go
@@ -16,9 +16,12 @@ var (
 	// invalid threshold that may either allow for multiple different
 	// quorums or none at all.
 	ErrInvalidThreshold = errors.New("transaction has invalid threshold")
-	// ErrSubtransactionFailed indicates a vote was cast on a
+	// ErrSubtransactionFailed indicates a vote was cast on an outcome
+	// which didn't reach majority.
+	ErrSubtransactionFailed = errors.New("subtransaction did not reach majority")
+	// ErrSubtransactionCanceled indicates a vote was cast on a
 	// subtransaction which failed already.
-	ErrSubtransactionFailed = errors.New("subtransaction has failed")
+	ErrSubtransactionCanceled = errors.New("subtransaction has been canceled")
 	// ErrTransactionStopped indicates the transaction was gracefully stopped.
 	ErrTransactionStopped = errors.New("transaction has been stopped")
 )
@@ -139,7 +142,7 @@ func (t *Transaction) State() (map[string]VoteResult, error) {
 			case transactionOpen:
 				results[voter.Name] = VoteUndecided
 			case transactionCanceled:
-				results[voter.Name] = VoteAborted
+				results[voter.Name] = VoteCanceled
 			case transactionStopped:
 				results[voter.Name] = VoteStopped
 			default:
@@ -199,11 +202,16 @@ func (t *Transaction) getOrCreateSubtransaction(node string) (*subtransaction, e
 			// If we have committed this subtransaction, we're good
 			// to go.
 			continue
-		case VoteAborted:
+		case VoteFailed:
+			// If a vote was cast on a subtransaction which failed
+			// to reach majority, then we cannot proceed with any
+			// subsequent votes anymore.
+			return nil, ErrSubtransactionFailed
+		case VoteCanceled:
 			// If the subtransaction was aborted, then we need to
 			// fail as we cannot proceed if the path leading to the
 			// end result has intermittent failures.
-			return nil, ErrSubtransactionFailed
+			return nil, ErrSubtransactionCanceled
 		case VoteStopped:
 			// If the transaction was stopped, then we need to fail
 			// with a graceful error.