From 8a412c6a266a84cfb2b7009f57d262fa1559d955 Mon Sep 17 00:00:00 2001 From: Luca Cavanna Date: Tue, 18 Dec 2018 10:43:14 +0100 Subject: [PATCH] Ensure MapperService#getAllMetaFields elements order is deterministic (#36739) MapperService#getAllMetaFields returns an array, which is created out of an `ObjectHashSet`. Such set does not guarantee deterministic hash ordering. The array returned by its toArray may be sorted differently at each run. This caused some repeatability issues in our tests (see #29080) as we pick random fields from the array of possible metadata fields, but that won't be repeatable if the input array is sorted differently at every run. Once setting the tests seed, hppc picks that up and the sorting is deterministic, but failures don't repeat with the seed that gets printed out originally (as a seed was not originally set). See also https://issues.carrot2.org/projects/HPPC/issues/HPPC-173. With this commit, we simply create a static sorted array that is used for `getAllMetaFields`. The change is in production code but really affects only testing as the only production usage of this method was to iterate through all values when parsing fields in the high-level REST client code. Anyways, this seems like a good change as returning an array would imply that it's deterministically sorted. --- .../elasticsearch/index/mapper/MapperService.java | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/mapper/MapperService.java b/server/src/main/java/org/elasticsearch/index/mapper/MapperService.java index 7663ec817a0..6424e75eaf6 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/MapperService.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/MapperService.java @@ -109,10 +109,11 @@ public class MapperService extends AbstractIndexComponent implements Closeable { //TODO this needs to be cleaned up: _timestamp and _ttl are not supported anymore, _field_names, _seq_no, _version and _source are //also missing, not sure if on purpose. See IndicesModule#getMetadataMappers - private static ObjectHashSet META_FIELDS = ObjectHashSet.from( - "_id", "_type", "_routing", "_index", - "_size", "_timestamp", "_ttl", IgnoredFieldMapper.NAME - ); + private static final String[] SORTED_META_FIELDS = new String[]{ + "_id", IgnoredFieldMapper.NAME, "_index", "_routing", "_size", "_timestamp", "_ttl", "_type" + }; + + private static final ObjectHashSet META_FIELDS = ObjectHashSet.from(SORTED_META_FIELDS); private static final DeprecationLogger deprecationLogger = new DeprecationLogger(LogManager.getLogger(MapperService.class)); @@ -762,7 +763,7 @@ public class MapperService extends AbstractIndexComponent implements Closeable { } public static String[] getAllMetaFields() { - return META_FIELDS.toArray(String.class); + return Arrays.copyOf(SORTED_META_FIELDS, SORTED_META_FIELDS.length); } /** An analyzer wrapper that can lookup fields within the index mappings */ @@ -789,5 +790,4 @@ public class MapperService extends AbstractIndexComponent implements Closeable { return defaultAnalyzer; } } - }