ivoson commented on code in PR #39459: URL: https://github.com/apache/spark/pull/39459#discussion_r1118005755
########## core/src/main/scala/org/apache/spark/storage/BlockInfoManager.scala: ########## @@ -399,7 +426,14 @@ private[storage] class BlockInfoManager extends Logging { try { val wrapper = new BlockInfoWrapper(newBlockInfo, lock) while (true) { - val previous = blockInfoWrappers.putIfAbsent(blockId, wrapper) + val previous = invisibleRDDBlocks.synchronized { + val res = blockInfoWrappers.putIfAbsent(blockId, wrapper) + if (res == null && trackingCacheVisibility) { + // Added to invisible blocks if it doesn't exist before. + blockId.asRDDId.foreach(invisibleRDDBlocks.add) Review Comment: Currently we syncrhonized all the write operations, and also the read operations which need to check both the variables. For other read operations, since the variabled won't change, I think synchronized block for such read operations may be not necessary. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org