mridulm commented on code in PR #39459:
URL: https://github.com/apache/spark/pull/39459#discussion_r1090953345


##########
core/src/main/scala/org/apache/spark/storage/BlockManager.scala:
##########
@@ -1325,14 +1325,47 @@ private[spark] class BlockManager(
     blockInfoManager.releaseAllLocksForTask(taskAttemptId)
   }
 
+  /**
+   * Retrieve the given rdd block if it exists and is visible, otherwise call 
the provided
+   * `makeIterator` method to compute the block, persist it, and return its 
values.
+   *
+   * @return either a BlockResult if the block was successfully cached, or an 
iterator if the block
+   *         could not be cached.
+   */
+  def getOrElseUpdateRDDBlock[T](
+      taskId: Long,
+      blockId: RDDBlockId,
+      level: StorageLevel,
+      classTag: ClassTag[T],
+      makeIterator: () => Iterator[T]): Either[BlockResult, Iterator[T]] = {
+    val isCacheVisible = isRDDBlockVisible(blockId)
+    var computed: Boolean = false
+    val getIterator = () => {
+      computed = true
+      makeIterator()
+    }
+
+    val res = getOrElseUpdate(blockId, level, classTag, getIterator)
+    if (res.isLeft && !isCacheVisible) {
+      if (!computed) {
+        // Loaded from cache, re-compute to update accumulators.
+        makeIterator()
+      }

Review Comment:
   This is a good point - and couple of issues that come to mind:
   
   * We do not disable speculative execution for indeterminate computation 
currently - and data generated from two task attempts can vary (which could be 
cached, and so differ for same partition)
   * We do not invalidate previously cached data, when there is a stage 
re-execution due to failures for an indeterminate computation,
   This is something we should revisit @Ngone51, @jiangxb1987 - thoughts ?



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to