[SPARK-14075] Refactor MemoryStore to be testable independent of BlockManager #11899

JoshRosen · 2016-03-22T20:50:23Z

This patch refactors the MemoryStore so that it can be tested without needing to construct / mock an entire BlockManager.

The block manager's serialization- and compression-related methods have been moved from BlockManager to SerializerManager.
BlockInfoManageris now passed directly to classes that need it, rather than being passed via the BlockManager.
The MemoryStore now calls dropFromMemory via a new BlockEvictionHandler interface rather than directly calling the BlockManager. This change helps to enforce a narrow interface between the MemoryStore and BlockManager functionality and makes this interface easier to mock in tests.
Several of the block unrolling tests have been moved from BlockManagerSuite into a new MemoryStoreSuite.

… BlockManager interface.

JoshRosen · 2016-03-22T20:51:41Z

/cc @rxin @andrewor14 @sameeragarwal @nongli , this is blocking the new tests in #11791

SparkQA · 2016-03-22T20:54:05Z

Test build #53813 has finished for PR 11899 at commit a02a3cb.

This patch fails Scala style tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2016-03-22T21:02:55Z

Test build #53814 has finished for PR 11899 at commit f64a3be.

This patch fails to build.
This patch merges cleanly.
This patch adds no public classes.

andrewor14 · 2016-03-22T21:57:36Z

core/src/main/scala/org/apache/spark/storage/BlockManagerManagedBuffer.scala

 */
 private[storage] class BlockManagerManagedBuffer(
-    blockManager: BlockManager,
+  blockInfoManager: BlockInfoManager,


2 more spaces

SparkQA · 2016-03-22T21:58:07Z

Test build #53824 has finished for PR 11899 at commit 937bda3.

This patch fails to build.
This patch merges cleanly.
This patch adds no public classes.

andrewor14 · 2016-03-22T21:59:15Z

core/src/test/java/org/apache/spark/util/collection/unsafe/sort/UnsafeExternalSorterSuite.java

    new TestMemoryManager(new SparkConf().set("spark.memory.offHeap.enabled", "false"));
  final TaskMemoryManager taskMemoryManager = new TaskMemoryManager(memoryManager, 0);
+  final SerializerManager serializerManager =
+      new SerializerManager(new JavaSerializer(new SparkConf()), new SparkConf());


andrewor14 · 2016-03-22T22:01:31Z

LGTM, will merge once you make it compile :) and pass tests.

JoshRosen · 2016-03-22T22:06:18Z

core/src/test/scala/org/apache/spark/storage/MemoryStoreSuite.scala

+    assert(memoryStore.currentUnrollMemoryForThisTask === 0)
+
+    def putIterator[T](
+      blockId: BlockId,


JoshRosen · 2016-03-23T00:29:19Z

Jenkins, retest this please.

SparkQA · 2016-03-23T01:36:46Z

Test build #53838 has finished for PR 11899 at commit 6ab0497.

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

JoshRosen · 2016-03-23T01:43:18Z

[error] Test org.apache.spark.util.collection.unsafe.sort.UnsafeExternalSorterSuite.forcedSpillingWithoutComparator failed: java.io.IOException: Stream is corrupted, took 0.072 sec
[error]     at org.apache.spark.io.LZ4BlockInputStream.refill(LZ4BlockInputStream.java:166)
[error]     at org.apache.spark.io.LZ4BlockInputStream.read(LZ4BlockInputStream.java:118)
[error]     at java.io.DataInputStream.readInt(DataInputStream.java:387)
[error]     at org.apache.spark.util.collection.unsafe.sort.UnsafeSorterSpillReader.<init>(UnsafeSorterSpillReader.java:57)
[error]     at org.apache.spark.util.collection.unsafe.sort.UnsafeSorterSpillWriter.getReader(UnsafeSorterSpillWriter.java:149)
[error]     at org.apache.spark.util.collection.unsafe.sort.UnsafeExternalSorter.getIterator(UnsafeExternalSorter.java:559)
[error]     at org.apache.spark.util.collection.unsafe.sort.UnsafeExternalSorterSuite.forcedSpillingWithoutComparator(UnsafeExternalSorterSuite.java:395)
[error]     ...

Alas, this looks like a legitimate failure.

SparkQA · 2016-03-23T02:53:55Z

Test build #53854 has finished for PR 11899 at commit 6ab0497.

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

SparkQA · 2016-03-23T09:18:26Z

Test build #53910 has finished for PR 11899 at commit 548ecc0.

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

JoshRosen · 2016-03-23T14:42:58Z

Jenkins retest this please

SparkQA · 2016-03-23T17:11:30Z

Test build #53937 has finished for PR 11899 at commit 548ecc0.

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

JoshRosen · 2016-03-23T17:13:22Z

This passed tests, so I'm going to merge it in order to unblock progress on my other patches.

JoshRosen added 5 commits March 22, 2016 11:13

Stop indirectly accessing BlockManager's BlockInfoManager

5e71e96

Move compression and serialization methods out of BlockManager.

6478c34

Introduce BlockEvictionHandler interface to decouple MemoryStore from…

83b5bda

… BlockManager interface.

Move unroll-related tests to new MemoryStoreSuite

0716030

Add back MEMORY_AND_DISK tests so that path continues to be exercised.

a02a3cb

Scalastyle

f64a3be

Fix probems in Java files.

937bda3

andrewor14 reviewed Mar 22, 2016
View reviewed changes

JoshRosen reviewed Mar 22, 2016
View reviewed changes

JoshRosen added 2 commits March 22, 2016 16:11

Style fixes.

52df07c

Fix compilation.

6ab0497

Fix tests

548ecc0

asfgit closed this in 3de24ae Mar 23, 2016

JoshRosen deleted the reduce-memorystore-blockmanager-coupling branch March 23, 2016 17:19

[SPARK-14075] Refactor MemoryStore to be testable independent of BlockManager #11899

[SPARK-14075] Refactor MemoryStore to be testable independent of BlockManager #11899

Uh oh!

Conversation

JoshRosen commented Mar 22, 2016

Uh oh!

JoshRosen commented Mar 22, 2016

Uh oh!

SparkQA commented Mar 22, 2016

Uh oh!

SparkQA commented Mar 22, 2016

Uh oh!

andrewor14 Mar 22, 2016

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Mar 22, 2016

Uh oh!

andrewor14 Mar 22, 2016

Choose a reason for hiding this comment

Uh oh!

andrewor14 commented Mar 22, 2016

Uh oh!

JoshRosen Mar 22, 2016

Choose a reason for hiding this comment

Uh oh!

JoshRosen commented Mar 23, 2016

Uh oh!

SparkQA commented Mar 23, 2016

Uh oh!

JoshRosen commented Mar 23, 2016

Uh oh!

SparkQA commented Mar 23, 2016

Uh oh!

SparkQA commented Mar 23, 2016

Uh oh!

JoshRosen commented Mar 23, 2016

Uh oh!

SparkQA commented Mar 23, 2016

Uh oh!

JoshRosen commented Mar 23, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants