Make ring.DoBatch() result deterministic

[duricanikolic]

What this PR does:
This PR fixes a bug present in the ring.DoBatch() function. Namely, there might be usecases in which, for the same set of keys, and the same ring instances failing on the same keys, different order of execution of batchTracker.record() on different keys gives different values.

For example, let us assume the following scenario:

• replication factor is 3
• number of ring instances is 6 ($I_1$, $I_2$, $I_3$, $I_4$, $I_5$, $I_6$).
• ring.DoBatch() is called on 2 keys $k_1$ and $k_2$.
• replication set of $k_1$ is $(I_1, I_2, I_3)$ and of $k_2$ is $(I_4, I_5, I_6)$.
• calls to the callback function on $k_1$ on $I_1$, $I_2$ and $I_3$ are successful.
• calls to the callback function on $k_2$ on $I_4$, $I_5$ and $I_6$ fail with a 4xx error.

The expected outcome of this call to ring.DoBatch() is that it would fail with the 4xx error.
With the current implementation, getting results from ingesters for example in this order: $I_1$, $I_4$, $I_5$, $I_2$, $I_3$, $I_6$ will actually give rise to a failure, but in the case of this order: $I_1$, $I_4$, $I_2$, $I_3$, $I_5$, $I_6$ would actually succeed. The reason for that is that successful execution of $k_1$ on $I_1$, $I_2$, $I_3$ is taken into account more than once.

This PR fixes this problem.

Checklist

• Tests updated
• CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]

[dimitarvdimitrov]

isn't there a race between calling it.suceeded.Inc() and it.succeeded.Load()? It's possible that between the time one invocation of succeeded.Inc() gets to succeeded.Load() another one has already invoked Inc(). This was we lose one invocation of remaining.Dec() of the first one

you can save the return value of succeeded.Inc in a variable and use that instead of calling Load again

[duricanikolic]

That's correct. I will fix this error. Thank you @dimitarvdimitrov

[dimitarvdimitrov]

LGTM, thanks for fixing this

[charleskorn]

Minor thing I noticed post-merge...

[charleskorn]

What's the purpose of waiting for the WaitGroup on a background goroutine?

[duricanikolic]

Thank you for pointing this out. This code was copied from batchTracker.DoBatchWithOptions(), but in the latter the additional go routine also does the cleanup, which is not necessary in this test.
I will now create a PR that removes this go routine.