Randomness.get() can make tests non repeatable

[javanna]

The Randomness class was introduced to ease tests repeatability, especially around collections shuffling. Randomness works by essentially looking for randomized testing classes in our classpath. If they are found, which is always true in our test classes, the random instance is retrieved via RandomizedContext.current().getRandom(). This is the right thing to do in most cases (e.g. while executing tests), but breaks repeatability when something should happen on a separate random instance. For instance, our IngestActionForwarder uses Randomness.get()during its initialization. The class is initialized while initializing a node. That breaks repeatability in our integ tests as nodes are generally initialized once per suite, before the first test runs. To make sure that the sequence is repeatable, we fork a different random doing new Random(random().nextLong()). That though is not getting picked up by IngestActionForwarder which ends up "stealing" a random.nextInt() call from the shared randomized context.

I think that all these random behaviours within a node (in our codebase actually) should rather be seeded and repeatable as much as possible. We already have the node seed setting, hence we should always try and use it when set (if it is not set there isn't much that we can do though). That means always using the node seed when available and make sure that our tests always provide the setting (for instance EsSingleNodeTestCase doesn't but should). The seed should have the precedence over the randomized context. I was surprised to see that there's only one place where we actually use the node seed (when we generate the node id). Every other place using randomness ends up either using randomized context if available in the classpath (potentially causing problems as the node seed is not taken into account), or creating a new random without seed (non repeatable sequence).

It seems that the Randomness.get() method potentially cause problems, I wonder if we have settings available wherever we use it, in which case we could simply remove the method and replace it with the one that takes the node seed into account. What do people think about this?

[javanna]

I didn't realize that what I thought was the node seed is actually the node id seed, so meant to be specific for the generation of the node id. I wonder why we don't have a node seed that can be used for any random event. Wondering what people think about that.

[javanna]

I also did not realize that we can use RandomizedContext.runWithPrivateRandomness(). Code run in that way will retrieve a private random through random(), and this way we make sure that node initialization doesn't affect other randomess in test methods. Will update #22283 accordingly, with that fix Randomness.get() is perfectly fine.