[GitHub] tinkerpop issue #745: TINKERPOP-1830: fix race condition in TinkerIndex

Github user mpollmeier commented on the issue: https://github.com/apache/tinkerpop/pull/745 ok that's done. added a changelog entry, merged this branch into tp32 and merged tp32 into master. ---

[GitHub] tinkerpop issue #745: TINKERPOP-1830: fix race condition in TinkerIndex

Github user spmallette commented on the issue: https://github.com/apache/tinkerpop/pull/745 sorry - i missed the `tp32` comment in your description. your changes look fine now. VOTE +1 ---

[GitHub] tinkerpop issue #745: TINKERPOP-1830: fix race condition in TinkerIndex

Github user mpollmeier commented on the issue: https://github.com/apache/tinkerpop/pull/745 Instead of marking them as final I simply dropped the assignment. Re tp32: yes, I also mentioned this in the description of the ticket. I've just squashed all commits and rebased onto tp32

[GitHub] tinkerpop issue #745: TINKERPOP-1830: fix race condition in TinkerIndex

Github user spmallette commented on the issue: https://github.com/apache/tinkerpop/pull/745 Please also include an update to CHANGELOG for this fix. ---

[GitHub] tinkerpop issue #745: TINKERPOP-1830: fix race condition in TinkerIndex

Github user robertdale commented on the issue: https://github.com/apache/tinkerpop/pull/745 `mvn -pl tinkergraph-gremlin -DskipIntegrationTests=false -am clean install` build SUCCESS VOTE +1 ---

[GitHub] tinkerpop issue #745: TINKERPOP-1830: fix race condition in TinkerIndex

Github user mpollmeier commented on the issue: https://github.com/apache/tinkerpop/pull/745 I made the same incorrect assumption. Fixed with your latest snippet and tested with our large testbase. No NPEs, no race conditions so far, looks good. VOTE +1 ---

[GitHub] tinkerpop issue #745: TINKERPOP-1830: fix race condition in TinkerIndex

Github user robertdale commented on the issue: https://github.com/apache/tinkerpop/pull/745 @mpollmeier Sorry, I thought that `putIfAbsent()` returned the current map but it returns old mapping or `null` hence the `NPEs`. Should be: ```java protected void put(final String

[GitHub] tinkerpop issue #745: TINKERPOP-1830: fix race condition in TinkerIndex

Github user mpollmeier commented on the issue: https://github.com/apache/tinkerpop/pull/745 Agreed, that's a better solution @robertdale. Note that `putIfAbsent` is not generally thread safe, but since we use `ConcurrentHashMap` it should be. I'll amend the commit. ---

[GitHub] tinkerpop issue #745: TINKERPOP-1830: fix race condition in TinkerIndex

Github user robertdale commented on the issue: https://github.com/apache/tinkerpop/pull/745 The race condition is in `put()` and it's used elsewhere. It would make more sense to fix `put()` to be thread-safe. Then we can keep `parallelStream()` ``` Map