honeymoose
/
OpenSearch
mirror of https://github.com/honeymoose/OpenSearch.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Remove and forbid use of j.u.c.ThreadLocalRandom 

This commit removes and now forbids all uses of
java.util.concurrent.ThreadLocalRandom across the codebase. The
underlying issue with ThreadLocalRandom is that it can not be
seeded. This means that if ThreadLocalRandom is used in production code,
then tests that cover any code path containing ThreadLocalRandom will be
prevented from being reproducible by use of ThreadLocalRandom. Instead,
using org.elasticsearch.common.random.Randomness#get will give
reproducible sources of random when running under tests and otherwise
still give an instance of ThreadLocalRandom when running as production
code.
This commit is contained in:
Jason Tedor 2016-01-08 12:10:03 -05:00
parent 349352c85a
commit 871d1b4885
8 changed files with 26 additions and 23 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
buildSrc/src/main/resources/forbidden/all-signatures.txt
View File

@ -125,4 +125,6 @@ java.util.Collections#EMPTY_MAP
java.util.Collections#EMPTY_SET
java.util.Collections#shuffle(java.util.List) @ Use java.util.Collections#shuffle(java.util.List, java.util.Random) with a reproducible source of randomness
java.util.Random#<init>() @ Use org.elasticsearch.common.random.Randomness#create for reproducible sources of randomness
@defaultMessage Use org.elasticsearch.common.Randomness#get for reproducible sources of randomness
java.util.Random#<init>()
java.util.concurrent.ThreadLocalRandom

4
core/src/main/java/org/elasticsearch/action/admin/indices/validate/query/TransportValidateQueryAction.java
View File

@ -37,6 +37,7 @@ import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver;
import org.elasticsearch.cluster.routing.GroupShardsIterator;
import org.elasticsearch.cluster.routing.ShardRouting;
import org.elasticsearch.common.ParsingException;
import org.elasticsearch.common.Randomness;
import org.elasticsearch.common.inject.Inject;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.util.BigArrays;
@ -59,7 +60,6 @@ import java.util.ArrayList;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.ThreadLocalRandom;
import java.util.concurrent.atomic.AtomicReferenceArray;
/**
@ -108,7 +108,7 @@ public class TransportValidateQueryAction extends TransportBroadcastAction<Valid
@Override
protected GroupShardsIterator shards(ClusterState clusterState, ValidateQueryRequest request, String[] concreteIndices) {
// Hard-code routing to limit request to a single shard, but still, randomize it...
Map<String, Set<String>> routingMap = indexNameExpressionResolver.resolveSearchRouting(clusterState, Integer.toString(ThreadLocalRandom.current().nextInt(1000)), request.indices());
Map<String, Set<String>> routingMap = indexNameExpressionResolver.resolveSearchRouting(clusterState, Integer.toString(Randomness.get().nextInt(1000)), request.indices());
return clusterService.operationRouting().searchShards(clusterState, concreteIndices, routingMap, "_local");
}

4
core/src/main/java/org/elasticsearch/cluster/routing/IndexRoutingTable.java
View File

@ -26,6 +26,7 @@ import org.apache.lucene.util.CollectionUtil;
import org.elasticsearch.cluster.AbstractDiffable;
import org.elasticsearch.cluster.metadata.IndexMetaData;
import org.elasticsearch.cluster.metadata.MetaData;
import org.elasticsearch.common.Randomness;
import org.elasticsearch.common.collect.ImmutableOpenIntMap;
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
@ -39,7 +40,6 @@ import java.util.HashSet;
import java.util.Iterator;
import java.util.List;
import java.util.Set;
import java.util.concurrent.ThreadLocalRandom;
/**
* The {@link IndexRoutingTable} represents routing information for a single
@ -71,7 +71,7 @@ public class IndexRoutingTable extends AbstractDiffable<IndexRoutingTable> imple
IndexRoutingTable(String index, ImmutableOpenIntMap<IndexShardRoutingTable> shards) {
this.index = index;
this.shuffler = new RotationShardShuffler(ThreadLocalRandom.current().nextInt());
this.shuffler = new RotationShardShuffler(Randomness.get().nextInt());
this.shards = shards;
List<ShardRouting> allActiveShards = new ArrayList<>();
for (IntObjectCursor<IndexShardRoutingTable> cursor : shards) {

4
core/src/main/java/org/elasticsearch/cluster/routing/IndexShardRoutingTable.java
View File

@ -21,6 +21,7 @@ package org.elasticsearch.cluster.routing;
import org.elasticsearch.cluster.node.DiscoveryNode;
import org.elasticsearch.cluster.node.DiscoveryNodes;
import org.elasticsearch.common.Randomness;
import org.elasticsearch.common.collect.MapBuilder;
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
@ -36,7 +37,6 @@ import java.util.LinkedList;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.ThreadLocalRandom;
import static java.util.Collections.emptyMap;
@ -73,7 +73,7 @@ public class IndexShardRoutingTable implements Iterable<ShardRouting> {
IndexShardRoutingTable(ShardId shardId, List<ShardRouting> shards) {
this.shardId = shardId;
this.shuffler = new RotationShardShuffler(ThreadLocalRandom.current().nextInt());
this.shuffler = new RotationShardShuffler(Randomness.get().nextInt());
this.shards = Collections.unmodifiableList(shards);
ShardRouting primary = null;

1
core/src/main/java/org/elasticsearch/common/Randomness.java
View File

@ -109,6 +109,7 @@ public final class Randomness {
}
}
@SuppressForbidden(reason = "ThreadLocalRandom is okay when not running tests")
private static Random getWithoutSeed() {
assert currentMethod == null && getRandomMethod == null : "running under tests but tried to create non-reproducible random";
return ThreadLocalRandom.current();

4
core/src/main/java/org/elasticsearch/node/internal/InternalSettingsPreparer.java
View File

@ -22,6 +22,7 @@ package org.elasticsearch.node.internal;
import org.elasticsearch.bootstrap.BootstrapInfo;
import org.elasticsearch.cluster.ClusterName;
import org.elasticsearch.common.Booleans;
import org.elasticsearch.common.Randomness;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.cli.Terminal;
import org.elasticsearch.common.collect.Tuple;
@ -41,7 +42,6 @@ import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.ThreadLocalRandom;
import static org.elasticsearch.common.Strings.cleanPath;
import static org.elasticsearch.common.settings.Settings.settingsBuilder;
@ -207,7 +207,7 @@ public class InternalSettingsPreparer {
name = reader.readLine();
}
}
int index = ThreadLocalRandom.current().nextInt(names.size());
int index = Randomness.get().nextInt(names.size());
return names.get(index);
} catch (IOException e) {
throw new RuntimeException("Could not read node names list", e);

14
plugins/lang-javascript/src/test/java/org/elasticsearch/script/javascript/JavaScriptScriptMultiThreadedTests.java
View File

@ -19,6 +19,7 @@
package org.elasticsearch.script.javascript;
import org.elasticsearch.common.Randomness;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.script.CompiledScript;
import org.elasticsearch.script.ExecutableScript;
@ -30,7 +31,6 @@ import java.util.HashMap;
import java.util.Map;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.CyclicBarrier;
import java.util.concurrent.ThreadLocalRandom;
import java.util.concurrent.atomic.AtomicBoolean;
import static org.hamcrest.Matchers.equalTo;
@ -53,8 +53,8 @@ public class JavaScriptScriptMultiThreadedTests extends ESTestCase {
public void run() {
try {
barrier.await();
long x = ThreadLocalRandom.current().nextInt();
long y = ThreadLocalRandom.current().nextInt();
long x = Randomness.get().nextInt();
long y = Randomness.get().nextInt();
long addition = x + y;
Map<String, Object> vars = new HashMap<String, Object>();
vars.put("x", x);
@ -95,12 +95,12 @@ public class JavaScriptScriptMultiThreadedTests extends ESTestCase {
public void run() {
try {
barrier.await();
long x = ThreadLocalRandom.current().nextInt();
long x = Randomness.get().nextInt();
Map<String, Object> vars = new HashMap<String, Object>();
vars.put("x", x);
ExecutableScript script = se.executable(new CompiledScript(ScriptService.ScriptType.INLINE, "testExecutableNoRuntimeParams", "js", compiled), vars);
for (int i = 0; i < 100000; i++) {
long y = ThreadLocalRandom.current().nextInt();
long y = Randomness.get().nextInt();
long addition = x + y;
script.setNextVar("y", y);
long result = ((Number) script.run()).longValue();
@ -139,8 +139,8 @@ public class JavaScriptScriptMultiThreadedTests extends ESTestCase {
barrier.await();
Map<String, Object> runtimeVars = new HashMap<String, Object>();
for (int i = 0; i < 100000; i++) {
long x = ThreadLocalRandom.current().nextInt();
long y = ThreadLocalRandom.current().nextInt();
long x = Randomness.get().nextInt();
long y = Randomness.get().nextInt();
long addition = x + y;
runtimeVars.put("x", x);
runtimeVars.put("y", y);

14
plugins/lang-python/src/test/java/org/elasticsearch/script/python/PythonScriptMultiThreadedTests.java
View File

@ -19,6 +19,7 @@
package org.elasticsearch.script.python;
import org.elasticsearch.common.Randomness;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.script.CompiledScript;
import org.elasticsearch.script.ExecutableScript;
@ -30,7 +31,6 @@ import java.util.HashMap;
import java.util.Map;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.CyclicBarrier;
import java.util.concurrent.ThreadLocalRandom;
import java.util.concurrent.atomic.AtomicBoolean;
import static org.hamcrest.Matchers.equalTo;
@ -55,8 +55,8 @@ public class PythonScriptMultiThreadedTests extends ESTestCase {
public void run() {
try {
barrier.await();
long x = ThreadLocalRandom.current().nextInt();
long y = ThreadLocalRandom.current().nextInt();
long x = Randomness.get().nextInt();
long y = Randomness.get().nextInt();
long addition = x + y;
Map<String, Object> vars = new HashMap<String, Object>();
vars.put("x", x);
@ -97,13 +97,13 @@ public class PythonScriptMultiThreadedTests extends ESTestCase {
// @Override public void run() {
// try {
// barrier.await();
// long x = ThreadLocalRandom.current().nextInt();
// long x = Randomness.get().nextInt();
// Map<String, Object> vars = new HashMap<String, Object>();
// vars.put("x", x);
// ExecutableScript script = se.executable(compiled, vars);
// Map<String, Object> runtimeVars = new HashMap<String, Object>();
// for (int i = 0; i < 100000; i++) {
// long y = ThreadLocalRandom.current().nextInt();
// long y = Randomness.get().nextInt();
// long addition = x + y;
// runtimeVars.put("y", y);
// long result = ((Number) script.run(runtimeVars)).longValue();
@ -143,8 +143,8 @@ public class PythonScriptMultiThreadedTests extends ESTestCase {
barrier.await();
Map<String, Object> runtimeVars = new HashMap<String, Object>();
for (int i = 0; i < 10000; i++) {
long x = ThreadLocalRandom.current().nextInt();
long y = ThreadLocalRandom.current().nextInt();
long x = Randomness.get().nextInt();
long y = Randomness.get().nextInt();
long addition = x + y;
runtimeVars.put("x", x);
runtimeVars.put("y", y);