Skip to content

Commit

Permalink
Ensure MapperService#getAllMetaFields elements order is deterministic
Browse files Browse the repository at this point in the history
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 elastic#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.
  • Loading branch information
javanna committed Dec 17, 2018
1 parent f1e1f93 commit 2be76d9
Showing 1 changed file with 6 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -109,10 +109,11 @@ public enum MergeReason {

//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<String> 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<String> META_FIELDS = ObjectHashSet.from(SORTED_META_FIELDS);

private static final DeprecationLogger deprecationLogger = new DeprecationLogger(LogManager.getLogger(MapperService.class));

Expand Down Expand Up @@ -762,7 +763,7 @@ public static boolean isMetadataField(String fieldName) {
}

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 */
Expand All @@ -789,5 +790,4 @@ protected Analyzer getWrappedAnalyzer(String fieldName) {
return defaultAnalyzer;
}
}

}

0 comments on commit 2be76d9

Please sign in to comment.