TestMetricTxnAborts is racy: baseline fetchMetrics is not retried while the assertion is

Summary

TestMetricTxnAborts reads its baseline metric value with an unretried fetchMetrics, while the assertion that follows uses retryableFetchMetrics. Prometheus metric export is asynchronous, so the baseline read can land before dgraph_txn_aborts_total has been published and the test fails with a message that reads like a missing metric rather than a race.

The code

dgraph/cmd/alpha/metrics_test.go:

func TestMetricTxnAborts(t *testing.T) {
	metricName := "dgraph_txn_aborts_total"
	...
	require.NoError(t, commitWithTs(mr1, false))
	require.Error(t, commitWithTs(mr2, false))

	metrics := fetchMetrics(t, metricName)          // <-- not retried

	... second round of mutations ...

	require.NoError(t, retryableFetchMetrics(t, map[string]int{
		metricName: metrics[metricName] + 1,        // <-- retried
	}))
}

The asymmetry looks unintentional: the same value is fetched twice, once without tolerance for propagation delay and once with.

Note also that dgraph_txn_aborts_total does not exist on the /metrics endpoint until the first abort is actually exported, so on a fresh cluster the baseline read is the most likely of the two to race — it is the one that has to wait for the counter to appear at all, not merely to increment.

Observed failure

--- FAIL: TestMetricTxnAborts (0.02s)
    metrics_test.go:153: the required metric 'dgraph_txn_aborts_total' was not found

Line 153 is inside fetchMetrics, reached from the unretried call. It passes on re-run, which is consistent with a propagation race rather than a functional problem. The preceding require.Error(t, commitWithTs(mr2, false)) passes, so the aborting commit did happen.

Seen once in CI on a fork of this repo; green on re-run of the same commit.

Suggested fix

Use the retrying variant for the baseline too, so the test tolerates propagation in both reads:

require.NoError(t, retryableFetchMetrics(t, map[string]int{metricName: 1}))
metrics := fetchMetrics(t, metricName)

Or have fetchMetrics treat "metric absent" as retryable when the caller is establishing a baseline. Either way the intent is that the test waits for the counter to exist before reading it.

TestMetricTxnCommits and TestMetricTxnDiscards have the same shape and are presumably exposed to the same race, though we have only observed the aborts variant failing.

I'm happy to open a PR for this if it's welcome.

Start in dgraph/cmd/alpha/metrics_test.go at TestMetricTxnAborts and compare its baseline fetchMetrics call with retryableFetchMetrics. Check the related TestMetricTxnCommits and TestMetricTxnDiscards cases for the same pattern. Done means the metric baseline tolerates asynchronous export and the affected tests pass without the missing-metric race.