Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SPARK-10985][CORE] Avoid passing evicted blocks throughout BlockManager #10776

Closed
wants to merge 9 commits into from

Conversation

JoshRosen
Copy link
Contributor

This patch refactors portions of the BlockManager and CacheManager in order to avoid having to pass evictedBlocks lists throughout the code. It appears that these lists were only consumed by TaskContext.taskMetrics, so the new code now directly updates the metrics from the lower-level BlockManager methods.

@SparkQA
Copy link

SparkQA commented Jan 15, 2016

Test build #49484 has finished for PR 10776 at commit deef931.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

Option(TaskContext.get()).foreach { tc =>
val metrics = tc.taskMetrics()
val lastUpdatedBlocks = metrics.updatedBlocks.getOrElse(Seq[(BlockId, BlockStatus)]())
metrics.updatedBlocks = Some(lastUpdatedBlocks ++ evictedBlocks.toSeq)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I plan to move this call closer to the eviction site itself (i.e. perform this call inside either the memory store or the block manager itself rather than here).

case Left(arr) =>
// We have successfully unrolled the entire partition, so cache it in memory
updatedBlocks ++=
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This update has been pushed further down into the BlockManager code. See the changes to doPut(). The same holds for the putIterator() call above.

@JoshRosen JoshRosen changed the title [SPARK-10985][WIP][CORE] Avoid passing evicted blocks throughout BlockManager [SPARK-10985][CORE] Avoid passing evicted blocks throughout BlockManager Jan 18, 2016
@JoshRosen
Copy link
Contributor Author

@andrewor14 @rxin, this should now be ready for review.

@SparkQA
Copy link

SparkQA commented Jan 18, 2016

Test build #49581 has finished for PR 10776 at commit bd1c436.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@@ -847,7 +854,7 @@ private[spark] class BlockManager(
.format(blockId, Utils.getUsedTimeMs(startTimeMs)))
}

updatedBlocks
marked
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not clear what this means, maybe success or putSuccess?

@andrewor14
Copy link
Contributor

Merged into master.

@asfgit asfgit closed this in b8cb548 Jan 18, 2016
@JoshRosen JoshRosen deleted the SPARK-10985 branch January 18, 2016 21:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants