diff options
author | Patrick Steinhardt <psteinhardt@gitlab.com> | 2021-01-28 16:17:43 +0300 |
---|---|---|
committer | Patrick Steinhardt <psteinhardt@gitlab.com> | 2021-02-01 12:11:53 +0300 |
commit | a08019e6ab078b0bf68510421b5078b2f308110d (patch) | |
tree | 4c35960b632712fdc99df88ee69c2ffd9ff1075e | |
parent | 289f034f47aa8bdc8329ac367a3dfe9e421e0f57 (diff) |
transactions: Discern failed/cancelled transactions for metrics
The Prometheus transaction counter currently tracks "stopped",
"invalid", "aborted", "committed" and "registered" transactions. Notably
absent are both "cancelled" and "failed" votes, as they're lumped
together in the "aborted" state.
Fix this by properly discerning both cases to get better metrics.
-rw-r--r-- | internal/praefect/coordinator_pg_test.go | 2 | ||||
-rw-r--r-- | internal/praefect/transactions/manager.go | 34 |
2 files changed, 20 insertions, 16 deletions
diff --git a/internal/praefect/coordinator_pg_test.go b/internal/praefect/coordinator_pg_test.go index e00b70660..574c433b3 100644 --- a/internal/praefect/coordinator_pg_test.go +++ b/internal/praefect/coordinator_pg_test.go @@ -208,7 +208,7 @@ func TestStreamDirectorMutator_Transaction(t *testing.T) { if node.shouldSucceed { assert.NoError(t, err) } else { - assert.True(t, errors.Is(err, transactions.ErrTransactionVoteFailed)) + assert.True(t, errors.Is(err, transactions.ErrTransactionFailed)) } }() } diff --git a/internal/praefect/transactions/manager.go b/internal/praefect/transactions/manager.go index 577cd7b80..f27513fa6 100644 --- a/internal/praefect/transactions/manager.go +++ b/internal/praefect/transactions/manager.go @@ -227,26 +227,30 @@ func (mgr *Manager) VoteTransaction(ctx context.Context, transactionID uint64, n }).Debug("VoteTransaction") if err := mgr.voteTransaction(ctx, transactionID, node, hash); err != nil { + fields := logrus.Fields{ + "transaction_id": transactionID, + "node": node, + "hash": hex.EncodeToString(hash), + } + var counterLabel string + if errors.Is(err, ErrTransactionStopped) { - mgr.counterMetric.WithLabelValues("stopped").Inc() + counterLabel = "stopped" + // Stopped transactions indicate a graceful + // termination, so we should not log an error here. } else if errors.Is(err, ErrTransactionFailed) { - mgr.counterMetric.WithLabelValues("aborted").Inc() - - mgr.log(ctx).WithFields(logrus.Fields{ - "transaction_id": transactionID, - "node": node, - "hash": hex.EncodeToString(hash), - }).WithError(err).Error("VoteTransaction: did not reach quorum") + counterLabel = "failed" + mgr.log(ctx).WithFields(fields).WithError(err).Error("VoteTransaction: did not reach quorum") + } else if errors.Is(err, ErrTransactionCanceled) { + counterLabel = "canceled" + mgr.log(ctx).WithFields(fields).WithError(err).Error("VoteTransaction: transaction was canceled") } else { - mgr.counterMetric.WithLabelValues("invalid").Inc() - - mgr.log(ctx).WithFields(logrus.Fields{ - "transaction_id": transactionID, - "node": node, - "hash": hex.EncodeToString(hash), - }).WithError(err).Error("VoteTransaction: vote failed") + counterLabel = "invalid" + mgr.log(ctx).WithFields(fields).WithError(err).Error("VoteTransaction: failure") } + mgr.counterMetric.WithLabelValues(counterLabel).Inc() + return err } |