[SPARK-14042][CORE] Add custom coalescer support

[nezihyigitbasi]

What changes were proposed in this pull request?

This PR adds support for specifying an optional custom coalescer to the coalesce() method. Currently I have only added this feature to the RDD interface, and once we sort out the details we can proceed with adding this feature to the other APIs (Dataset etc.)

How was this patch tested?

Added a unit test for this functionality.

/cc @rxin (per our discussion on the mailing list)

[nezihyigitbasi]

Since HadoopPartition is not public a user who wants to implement this outside of Spark can have some trouble.

[nezihyigitbasi]

@rxin any comments?

[hbhanawat]

@nezihyigitbasi, do you plan to add something similar for DF/DS API?

[nezihyigitbasi]

@hbhanawat once we figure out the details I think it makes sense to do that.

[nezihyigitbasi]

@rxin any plans to review this?

[rxin]

we don't need a val here do we?

[nezihyigitbasi]

no we don't.

[rxin]

The API change looks alright. I'd separate the dataset changes from this one. Are there other things you want to do before this is not WIP?

[nezihyigitbasi]

If the API changes look OK to you, then I don't have anything else before this is not WIP. I only need to resolve conflicts with the master.

[nezihyigitbasi]

@rxin rebased & addressed comments.

[rxin]

this should have parentheses since it has side effect

[rxin]

Also can you tag all these apis as DeveloperApi? Thanks.'

[nezihyigitbasi]

I followed the naming convention for other classes, let me know if you still want lower-case.

[rxin]

Yea actually many of the naming in spark core is wrong but we never bothered changing them. Usually SomeWord.scala means there is a class named SomeWord. The scala style guide actually recommends when there are multiple classes that are part of a coherent group, start with lowercase (similar to a lot of c++ naming guides).

[nezihyigitbasi]

@rxin thanks for the comments. Updated.

[rxin]

We would need to add the label here. something like ::DeveloperApi. look up other classes to confirm.

[rxin]

LGTM other than that couple minor feedback.

[nezihyigitbasi]

@rxin thanks, comments addressed. Renamed that file to use lower-case too.

[rxin]

Thanks - let's wait for Jenkins. Can you update the title / description of the pull request?

[SparkQA]

Test build #2818 has finished for PR 11865 at commit 9d91f77.

• This patch fails MiMa tests.
• This patch merges cleanly.
• This patch adds the following public classes (experimental):
  • trait PartitionCoalescer
  • class PartitionGroup(val prefLoc: Option[String] = None)

[nezihyigitbasi]

Mima tests failing, I guess we can exclude them all. What do you think?

[info] spark-core: found 3 potential binary incompatibilities while checking against org.apache.spark:spark-core_2.11:1.6.0  (filtered 1299)
[error]  * method coalesce(Int,Boolean,scala.math.Ordering)org.apache.spark.rdd.RDD in class org.apache.spark.rdd.RDD does not have a correspondent in current version
[error]    filter with: ProblemFilters.exclude[DirectMissingMethodProblem]("org.apache.spark.rdd.RDD.coalesce")
[error]  * class org.apache.spark.rdd.PartitionCoalescer#LocationIterator does not have a correspondent in current version
[error]    filter with: ProblemFilters.exclude[MissingClassProblem]("org.apache.spark.rdd.PartitionCoalescer$LocationIterator")
[error]  * declaration of class org.apache.spark.rdd.PartitionCoalescer is interface org.apache.spark.rdd.PartitionCoalescer in current version; changing class to interface breaks client code
[error]    filter with: ProblemFilters.exclude[IncompatibleTemplateDefProblem]("org.apache.spark.rdd.PartitionCoalescer")

[rxin]

Yup go for it.

[nezihyigitbasi]

@rxin somehow jenkins didn't start the tests after my last push, can you please kick it off?

[SparkQA]

Test build #2829 has finished for PR 11865 at commit 5a12586.

• This patch passes all tests.
• This patch merges cleanly.
• This patch adds the following public classes (experimental):
  • trait PartitionCoalescer
  • class PartitionGroup(val prefLoc: Option[String] = None)

[nezihyigitbasi]

@rxin tests look OK, do you have any other comments?

[rxin]

Merging in master. Thanks.