[GitHub] spark issue #22371: [SPARK-25386][CORE] Don't need to synchronize the IndexS...

Github user ConeyLiu commented on the issue: https://github.com/apache/spark/pull/22371 OK, thanks everyone for the help. Close it --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional

[GitHub] spark issue #22371: [SPARK-25386][CORE] Don't need to synchronize the IndexS...

Github user cloud-fan commented on the issue: https://github.com/apache/spark/pull/22371 My opinion is, it's not worth to spend time on it. The lock is not likely to be a bottleneck and it's better to keep it simple even it's sub-optimal. ---

[GitHub] spark issue #22371: [SPARK-25386][CORE] Don't need to synchronize the IndexS...

Github user cloud-fan commented on the issue: https://github.com/apache/spark/pull/22371 @ConeyLiu we may have an executor lost and then come back, and may have 2 same tasks running on the same executor. --- - To

[GitHub] spark issue #22371: [SPARK-25386][CORE] Don't need to synchronize the IndexS...

Github user ConeyLiu commented on the issue: https://github.com/apache/spark/pull/22371 @squito , thanks for the review. I intend to using `ConcurrentHashMap[Int, AtomicReferenceArray]` previously. After re-think the code, I can know the lock here is used to prevent the same

[GitHub] spark issue #22371: [SPARK-25386][CORE] Don't need to synchronize the IndexS...

Github user ConeyLiu commented on the issue: https://github.com/apache/spark/pull/22371 Thanks @felixcheung, @srowen, @cloud-fan for your time. There is only one instance of `IndexShuffleBlockResolver` per executor, and the synchronize is used to protect the modify safely when there

[GitHub] spark issue #22371: [SPARK-25386][CORE] Don't need to synchronize the IndexS...

Github user cloud-fan commented on the issue: https://github.com/apache/spark/pull/22371 How much perf can we save here? I don't think shuffle writing will be bottlenecked by this lock. --- - To unsubscribe,

[GitHub] spark issue #22371: [SPARK-25386][CORE] Don't need to synchronize the IndexS...

Github user felixcheung commented on the issue: https://github.com/apache/spark/pull/22371 + @srowen @squito @JoshRosen --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands,

[GitHub] spark issue #22371: [SPARK-25386][CORE] Don't need to synchronize the IndexS...

Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/22371 Can one of the admins verify this patch? --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional

[GitHub] spark issue #22371: [SPARK-25386][CORE] Don't need to synchronize the IndexS...

Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/22371 Can one of the admins verify this patch? --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional

[GitHub] spark issue #22371: [SPARK-25386][CORE] Don't need to synchronize the IndexS...

Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/22371 Can one of the admins verify this patch? --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional

[GitHub] spark issue #22371: [SPARK-25386][CORE] Don't need to synchronize the IndexS...

Github user ConeyLiu commented on the issue: https://github.com/apache/spark/pull/22371 @cloud-fan @jiangxb1987 Could you help to review this? Thanks a lot. --- - To unsubscribe, e-mail: