fix race condition in TinkerIndex My colleage @fabsx00 discovered a race condition in tinkergraph's index creation. He fixed it by simply replacing parallelStream with stream. Quoting his analysis: So, reading the code, you see that this.put is called in parallel, but that method seems to contain a race as get is called on the index, checked for null, and a subsequent write is performed. It still seems like using stream here fixes the problem we've been seeing, and the performance hit is not significant.
Ticket: https://issues.apache.org/jira/browse/TINKERPOP-1830 Project: http://git-wip-us.apache.org/repos/asf/tinkerpop/repo Commit: http://git-wip-us.apache.org/repos/asf/tinkerpop/commit/7be8a0f2 Tree: http://git-wip-us.apache.org/repos/asf/tinkerpop/tree/7be8a0f2 Diff: http://git-wip-us.apache.org/repos/asf/tinkerpop/diff/7be8a0f2 Branch: refs/heads/TINKERPOP-1489 Commit: 7be8a0f25672e20fb0fef58075c2a201501f0578 Parents: ac99e3c Author: Michael Pollmeier <mich...@michaelpollmeier.com> Authored: Sun Nov 12 12:32:37 2017 +1300 Committer: Michael Pollmeier <mich...@michaelpollmeier.com> Committed: Tue Nov 14 14:22:40 2017 +1300 ---------------------------------------------------------------------- CHANGELOG.asciidoc | 1 + .../gremlin/tinkergraph/structure/TinkerIndex.java | 11 +++++------ 2 files changed, 6 insertions(+), 6 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/7be8a0f2/CHANGELOG.asciidoc ---------------------------------------------------------------------- diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc index ffd9de5..cd5dc38 100644 --- a/CHANGELOG.asciidoc +++ b/CHANGELOG.asciidoc @@ -23,6 +23,7 @@ image::https://raw.githubusercontent.com/apache/tinkerpop/master/docs/static/ima [[release-3-2-7]] === TinkerPop 3.2.7 (Release Date: NOT OFFICIALLY RELEASED YET) +* Fixed a race condition in `TinkerIndex`. * Fixed an `ArrayOutOfBoundsException` in `hasId()` for the rare situation when the provided collection is empty. * Bump to Netty 4.0.52 * `TraversalVertexProgram` `profile()` now accounts for worker iteration in `GraphComputer` OLAP. http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/7be8a0f2/tinkergraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/TinkerIndex.java ---------------------------------------------------------------------- diff --git a/tinkergraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/TinkerIndex.java b/tinkergraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/TinkerIndex.java index 69afb39..f5872cf 100644 --- a/tinkergraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/TinkerIndex.java +++ b/tinkergraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/TinkerIndex.java @@ -48,17 +48,16 @@ final class TinkerIndex<T extends Element> { protected void put(final String key, final Object value, final T element) { Map<Object, Set<T>> keyMap = this.index.get(key); - if (keyMap == null) { - keyMap = new ConcurrentHashMap<>(); - this.index.put(key, keyMap); + if (null == keyMap) { + this.index.putIfAbsent(key, new ConcurrentHashMap<Object, Set<T>>()); + keyMap = this.index.get(key); } Set<T> objects = keyMap.get(value); if (null == objects) { - objects = new HashSet<>(); - keyMap.put(value, objects); + keyMap.putIfAbsent(value, ConcurrentHashMap.newKeySet()); + objects = keyMap.get(value); } objects.add(element); - } public List<T> get(final String key, final Object value) {