[SPARK-2617] Correct doc and usages of preservesPartitioning

[mengxr]

The name preservesPartitioning is ambiguous: 1) preserves the indices of partitions, 2) preserves the partitioner. The latter is correct and preservesPartitioning should really be called preservesPartitioner to avoid confusion. Unfortunately, this is already part of the API and we cannot change. We should be clear in the doc and fix wrong usages.

This PR

1. adds notes in maPartitions*,
2. makes RDD.sample preserve partitioner,
3. changes preservesPartitioning to false in RDD.zip because the keys of the first RDD are no longer the keys of the zipped RDD,
4. fixes some wrong usages in MLlib.

[SparkQA]

QA tests have started for PR 1526. This patch merges cleanly.
View progress: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/16960/consoleFull

[SparkQA]

QA results for PR 1526:
- This patch FAILED unit tests.
- This patch merges cleanly
- This patch adds no public classes

For more information see test ouptut:
https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/16960/consoleFull

[SparkQA]

QA tests have started for PR 1526. This patch merges cleanly.
View progress: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/16973/consoleFull

[SparkQA]

QA results for PR 1526:
- This patch FAILED unit tests.
- This patch merges cleanly
- This patch adds no public classes

For more information see test ouptut:
https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/16973/consoleFull

[SparkQA]

QA tests have started for PR 1526. This patch merges cleanly.
View progress: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/16984/consoleFull

[SparkQA]

QA results for PR 1526:
- This patch PASSES unit tests.
- This patch merges cleanly
- This patch adds no public classes

For more information see test ouptut:
https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/16984/consoleFull

[pwendell]

I think the reason you find it confusing is because you are interpreting it an imperative (please preserve X) instead of descriptive (this preserves X). I always interpreted it as descriptive. I.e. you are doing something that logically preserves the partitioning of the RDD. E.g. you are answering the question "Does this map function preserve the partitioning of the underlying data?".

[pwendell]

I think that's why it's called preservesPartitioning (descriptive) instead of preservePartitioning (imperative).

[mengxr]

@pwendell The confusing part is the definition of "partitioning". It could be the indexing of partitions or the partitioner. The first time I found that mapPartitions has a parameter called perservesPartitioning, I thought this is to preserve the indexing of partitions --- so partition 0 maps to partition 0 and partition 1 maps to partition 1, etc.

This causes problems. For example, we set preservesPartitioning to true in RDD.zip and the following code won't run correctly:

val a = sc.makeRDD(Seq(0, 1, 2, 3, 4)).map(x => (x, 1)).partitionBy(new HashPartitioner(2))
val b = a.map(x => 1)
a.zip(b).join(a.map(x => (x, 1)).collect()

Btw, preservePartitioning is used in streaming instead of preservesPartitioning. @tdas

[SparkQA]

QA tests have started for PR 1526. This patch merges cleanly.
View progress: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/17017/consoleFull

[pwendell]

Nit: "Note that preservesPartitioning means whether" --> "preservesPartitioning indicates whether" (I think it's implicit that we are asking them to note everything, there are only two sentences here).

Also, it might be nice to add a blank line after the first sentence.

[mengxr]

With a blank line now, it is much easier to copy & paste :)

[pwendell]

Only one pedantic comment. LGTM even if you ignore my comment.

[SparkQA]

QA tests have started for PR 1526. This patch merges cleanly.
View progress: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/17018/consoleFull

[SparkQA]

QA results for PR 1526:
- This patch PASSES unit tests.
- This patch merges cleanly
- This patch adds no public classes

For more information see test ouptut:
https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/17017/consoleFull

[rxin]

Merging in master. THanks!

[SparkQA]

QA results for PR 1526:
- This patch PASSES unit tests.
- This patch merges cleanly
- This patch adds no public classes

For more information see test ouptut:
https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/17018/consoleFull