diff options
author | Patrick Steinhardt <psteinhardt@gitlab.com> | 2020-07-20 09:08:21 +0300 |
---|---|---|
committer | Patrick Steinhardt <psteinhardt@gitlab.com> | 2020-07-20 09:56:19 +0300 |
commit | 974cf6a450f8f36bd6b7f73c534a952196b879d2 (patch) | |
tree | 11956ffcef0bd6236afa5df0a84fa836b04dd9f4 | |
parent | f8f38f43a8ad0f5825161b1da5e13a2eb6e75377 (diff) |
transactions: Do not return state when cancelling
The transaction state is currently returned whenever one calls the
transaction cleanup function. This is an artifact caused by the
transaction type not being publicly visible earlier, but it starts to
become a hinderance when implementing new features and querying
transacton state. So let's just stop returning the state from the
cleanup function and ask callers to retrieve the state by themselves.
-rw-r--r-- | internal/praefect/coordinator.go | 5 | ||||
-rw-r--r-- | internal/praefect/transaction_test.go | 7 | ||||
-rw-r--r-- | internal/praefect/transactions/manager.go | 11 |
3 files changed, 12 insertions, 11 deletions
diff --git a/internal/praefect/coordinator.go b/internal/praefect/coordinator.go index 4e63245d0..df00fe4a8 100644 --- a/internal/praefect/coordinator.go +++ b/internal/praefect/coordinator.go @@ -236,11 +236,12 @@ func (c *Coordinator) mutatorStreamParameters(ctx context.Context, call grpcCall return nil, fmt.Errorf("registering transactions: %w", err) } finalizers = append(finalizers, func() error { - successByNode, err := transactionCleanup() - if err != nil { + if err := transactionCleanup(); err != nil { return err } + successByNode := transaction.State() + // If the primary node failed the transaction, then // there's no sense in trying to replicate from primary // to secondaries. diff --git a/internal/praefect/transaction_test.go b/internal/praefect/transaction_test.go index eb22067cf..9f927cbd1 100644 --- a/internal/praefect/transaction_test.go +++ b/internal/praefect/transaction_test.go @@ -543,7 +543,8 @@ func TestTransactionWithMultipleVotes(t *testing.T) { wg.Wait() - results, _ := cancel() + require.NoError(t, cancel()) + results := transaction.State() for i, voter := range tc.voters { require.Equal(t, voter.shouldSucceed, results[fmt.Sprintf("node-%d", i)]) } @@ -672,9 +673,9 @@ func TestTransactionCancellation(t *testing.T) { } wg.Wait() - results, err := cancelTransaction() - require.NoError(t, err) + require.NoError(t, cancelTransaction()) + results := transaction.State() for i, v := range tc.voters { require.Equal(t, results[fmt.Sprintf("node-%d", i)], v.shouldSucceed, "result mismatches expected node state") } diff --git a/internal/praefect/transactions/manager.go b/internal/praefect/transactions/manager.go index 32af609e1..1afc03588 100644 --- a/internal/praefect/transactions/manager.go +++ b/internal/praefect/transactions/manager.go @@ -111,9 +111,8 @@ func (mgr *Manager) log(ctx context.Context) logrus.FieldLogger { // CancelFunc is the transaction cancellation function returned by // `RegisterTransaction`. Calling it will cause the transaction to be removed -// from the transaction manager. It returns the outcome for each node: `true` -// if the node committed the transaction, `false` if it aborted. -type CancelFunc func() (map[string]bool, error) +// from the transaction manager. +type CancelFunc func() error // RegisterTransaction registers a new reference transaction for a set of nodes // taking part in the transaction. `threshold` is the threshold at which an @@ -146,12 +145,12 @@ func (mgr *Manager) RegisterTransaction(ctx context.Context, voters []Voter, thr mgr.counterMetric.WithLabelValues("registered").Add(float64(len(voters))) - return transaction, func() (map[string]bool, error) { + return transaction, func() error { return mgr.cancelTransaction(transaction) }, nil } -func (mgr *Manager) cancelTransaction(transaction *Transaction) (map[string]bool, error) { +func (mgr *Manager) cancelTransaction(transaction *Transaction) error { mgr.lock.Lock() defer mgr.lock.Unlock() @@ -160,7 +159,7 @@ func (mgr *Manager) cancelTransaction(transaction *Transaction) (map[string]bool transaction.cancel() mgr.subtransactionsMetric.Observe(float64(transaction.CountSubtransactions())) - return transaction.State(), nil + return nil } func (mgr *Manager) voteTransaction(ctx context.Context, transactionID uint64, node string, hash []byte) error { |