-
Notifications
You must be signed in to change notification settings - Fork 380
Utility function to get a setup & cleanup function for mapping each partition #456
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
base: master
Are you sure you want to change the base?
Changes from 2 commits
56d6f5c
b648558
aebda1d
e347e49
a93d195
0807ee3
74c606a
b69c619
2a33765
dc3dbbc
8387fdb
442e941
90c7fab
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,25 @@ | ||
| package spark.util | ||
|
|
||
| /** | ||
| * Wrapper around an iterator which calls a cleanup method when its finished iterating through its elements | ||
| */ | ||
| abstract class CleanupIterator[+A, +I <: Iterator[A]](sub: I) extends Iterator[A]{ | ||
| def next = sub.next | ||
| def hasNext = { | ||
| val r = sub.hasNext | ||
| if (!r) { | ||
| cleanup | ||
| } | ||
| r | ||
| } | ||
|
|
||
| def cleanup | ||
| } | ||
|
|
||
| object CleanupIterator { | ||
| def apply[A, I <: Iterator[A]](sub: I, cleanupFunction: () => Unit) : CleanupIterator[A,I] = { | ||
| new CleanupIterator[A,I](sub) { | ||
| def cleanup = cleanupFunction() | ||
| } | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,6 +4,8 @@ import scala.collection.mutable.HashMap | |
| import org.scalatest.FunSuite | ||
| import spark.SparkContext._ | ||
| import spark.rdd.{CoalescedRDD, PartitionPruningRDD} | ||
| import spark.RDD.PartitionMapper | ||
| import collection._ | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This style of import doesn't match the convention used in the rest of the codebase. It should probably be replaced with an import of |
||
|
|
||
| class RDDSuite extends FunSuite with LocalSparkContext { | ||
|
|
||
|
|
@@ -173,4 +175,48 @@ class RDDSuite extends FunSuite with LocalSparkContext { | |
| assert(prunedData.size === 1) | ||
| assert(prunedData(0) === 10) | ||
| } | ||
|
|
||
| test("mapPartitionWithSetupAndCleanup") { | ||
| sc = new SparkContext("local[4]", "test") | ||
| val data = sc.parallelize(1 to 100, 4) | ||
| val acc = sc.accumulableCollection(new HashMap[Int,Set[Int]]()) | ||
| val mapped = data.mapWithSetupAndCleanup(new PartitionMapper[Int,Int](){ | ||
| var partition = -1 | ||
| val values = mutable.Set[Int]() | ||
| def setup(partition:Int) {this.partition = partition} | ||
| def map(i:Int) = {values += i; i * 2} | ||
| def cleanup = { | ||
| //the purpose of this strange code is just to make sure this method is called | ||
| // after the data has been iterated through completely. | ||
| acc.localValue += (partition -> Set[Int](values.toSeq: _*)) | ||
| } | ||
| }).collect | ||
|
|
||
| assert(mapped.toSet === (1 to 100).map{_ * 2}.toSet) | ||
| assert(acc.value.keySet == (0 to 3).toSet) | ||
| acc.value.foreach { case(partition, values) => | ||
| assert(values.size === 25) | ||
| } | ||
|
|
||
|
|
||
| //the naive alternative doesn't work | ||
| val acc2 = sc.accumulableCollection(new HashMap[Int,Set[Int]]()) | ||
| val m2 = data.mapPartitionsWithSplit{ | ||
| case (partition, itr) => | ||
| val values = mutable.Set[Int]() | ||
| val mItr = itr.map{i => values += i; i * 2} | ||
| //you haven't actually put anything into values yet, b/c itr.map defines another | ||
| // iterator, which is lazily computed. so the Set is empty | ||
| acc2.localValue += (partition -> Set[Int](values.toSeq: _*)) | ||
| mItr | ||
| }.collect | ||
|
|
||
| assert(m2.toSet === (1 to 100).map{_ * 2}.toSet) | ||
| assert(acc2.value.keySet == (0 to 3).toSet) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should be a triple-equals. |
||
| //this condition will fail | ||
| // acc2.value.foreach { case(partition, values) => | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If this is supposed to fail, should we wrap it in an
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. well, its not really supposed to fail -- there is no exception, it just doesn't give the "expected" result. that second part isn't really a unit test at all, its just documentation of why this method is needed. probably doesn't belong here at all, I just wanted it as part of the pull request to demonstrate why the method was needed. I guess I can just write it up somewhere else (seems too long to put in the spark docs also, at least in the current layout ...) |
||
| // assert(values.size === 25) | ||
| // } | ||
|
|
||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please don't use relative import names for packages (see http://spark-project.org/docs/latest/contributing-to-spark.html)