diff options
author | Patrick Steinhardt <psteinhardt@gitlab.com> | 2021-01-28 16:06:50 +0300 |
---|---|---|
committer | Patrick Steinhardt <psteinhardt@gitlab.com> | 2021-02-01 12:11:53 +0300 |
commit | 289f034f47aa8bdc8329ac367a3dfe9e421e0f57 (patch) | |
tree | f49b46e9d618295120640102f905ecf10a857873 /internal/praefect/transactions | |
parent | 4f916e02513cafce0888d488861ce0ca78a1d593 (diff) |
transactions: Consolidate transaction and subtransaction errors
Initially, there was no support for subtransactions, so every node could
only cast a single vote for a given transaction. This was later extended
to support subtransactions, where nodes can cast a sequence of votes.
Due to this organic growth, there now is some confusion when it comes to
errors: some errors talk about transactions, while others talk about
subtransactions. This commit thus consolidates the errors to only ever
talk about transactions -- callers needn't be aware of subtransactions,
even less so as they're handled completely transparently.
Diffstat (limited to 'internal/praefect/transactions')
-rw-r--r-- | internal/praefect/transactions/manager.go | 2 | ||||
-rw-r--r-- | internal/praefect/transactions/subtransaction.go | 11 | ||||
-rw-r--r-- | internal/praefect/transactions/transaction.go | 16 |
3 files changed, 10 insertions, 19 deletions
diff --git a/internal/praefect/transactions/manager.go b/internal/praefect/transactions/manager.go index d77aa5ec3..577cd7b80 100644 --- a/internal/praefect/transactions/manager.go +++ b/internal/praefect/transactions/manager.go @@ -229,7 +229,7 @@ func (mgr *Manager) VoteTransaction(ctx context.Context, transactionID uint64, n if err := mgr.voteTransaction(ctx, transactionID, node, hash); err != nil { if errors.Is(err, ErrTransactionStopped) { mgr.counterMetric.WithLabelValues("stopped").Inc() - } else if errors.Is(err, ErrTransactionVoteFailed) { + } else if errors.Is(err, ErrTransactionFailed) { mgr.counterMetric.WithLabelValues("aborted").Inc() mgr.log(ctx).WithFields(logrus.Fields{ diff --git a/internal/praefect/transactions/subtransaction.go b/internal/praefect/transactions/subtransaction.go index 5f46d55a4..4b836b80b 100644 --- a/internal/praefect/transactions/subtransaction.go +++ b/internal/praefect/transactions/subtransaction.go @@ -3,19 +3,10 @@ package transactions import ( "context" "crypto/sha1" - "errors" "fmt" "sync" ) -var ( - // ErrTransactionVoteFailed indicates the transaction didn't reach quorum. - ErrTransactionVoteFailed = errors.New("transaction did not reach quorum") - // ErrTransactionCanceled indicates the transaction was canceled before - // reaching quorum. - ErrTransactionCanceled = errors.New("transaction was canceled") -) - // VoteResult represents the outcome of a transaction for a single voter. type VoteResult int @@ -230,7 +221,7 @@ func (t *subtransaction) collectVotes(ctx context.Context, node string) error { // exceeding it, we know we're the winner in that case. if t.voteCounts[voter.vote] < t.threshold { voter.result = VoteFailed - return fmt.Errorf("%w: got %d/%d votes", ErrTransactionVoteFailed, t.voteCounts[voter.vote], t.threshold) + return fmt.Errorf("%w: got %d/%d votes", ErrTransactionFailed, t.voteCounts[voter.vote], t.threshold) } voter.result = VoteCommitted diff --git a/internal/praefect/transactions/transaction.go b/internal/praefect/transactions/transaction.go index 3f0dd7f97..770a0ac8f 100644 --- a/internal/praefect/transactions/transaction.go +++ b/internal/praefect/transactions/transaction.go @@ -16,12 +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 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. - ErrSubtransactionCanceled = errors.New("subtransaction has been canceled") + + // ErrTransactionFailed indicates the transaction didn't reach quorum. + ErrTransactionFailed = errors.New("transaction did not reach quorum") + // ErrTransactionCanceled indicates the transaction was canceled before + // reaching quorum. + ErrTransactionCanceled = errors.New("transaction has been canceled") // ErrTransactionStopped indicates the transaction was gracefully stopped. ErrTransactionStopped = errors.New("transaction has been stopped") ) @@ -206,12 +206,12 @@ func (t *Transaction) getOrCreateSubtransaction(node string) (*subtransaction, e // 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 + return nil, ErrTransactionFailed 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, ErrSubtransactionCanceled + return nil, ErrTransactionCanceled case VoteStopped: // If the transaction was stopped, then we need to fail // with a graceful error. |