[GitHub] storm issue #2622: STORM-3020: fix possible race condition in AsyncLocalizer

2018-04-09 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2622 @HeartSaVioR thanks for the review. I fixed your concerns by making them all ConcurrentHashMaps and adding a note about why they need to be that. I could not find a good way to remove the

[GitHub] storm issue #2622: STORM-3020: fix possible race condition in AsyncLocalizer

2018-04-07 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2622 @HeartSaVioR Great catch I forgot to update the normal has Map/HashMap to a ConcurrentHashMap. Yes the guarantees of ConcurrentMap allow for retry and we do have side effects in some of

[GitHub] storm issue #2622: STORM-3020: fix possible race condition in AsyncLocalizer

2018-04-06 Thread HeartSaVioR
Github user HeartSaVioR commented on the issue: https://github.com/apache/storm/pull/2622 @revans2 Could you elaborate how protection works? At the first time I thought it leverages atomicity of compute*, but reading description of compute* in Map, looks like

[GitHub] storm issue #2622: STORM-3020: fix possible race condition in AsyncLocalizer

2018-04-05 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2622 Thanks for the review @danny0405 This was not trying to fix STORM-2905. This was a separate race condition I found when reviewing your pull request for STORM-2905. ---

[GitHub] storm issue #2622: STORM-3020: fix possible race condition in AsyncLocalizer

2018-04-05 Thread danny0405
Github user danny0405 commented on the issue: https://github.com/apache/storm/pull/2622 I test it, and it worked ok as before, but a KeyNotFoundException still got of master when finally cleaned the base blobs. ---