Repository navigation
SPARK-1597: Add a version of reduceByKey that takes the Partitioner as a... - #550
techaddict wants to merge 1 commit into
Conversation
…s a second argument Most of our shuffle methods can take a Partitioner or a number of partitions as a second argument, but for some reason reduceByKey takes the Partitioner as a first argument: http://spark.apache.org/docs/0.9.1/api/core/#org.apache.spark.rdd.PairRDDFunctions. Deprecated that version and added one where the Partitioner is the second argument.
|
We'll need to specify the parameter types for function passed to reduceByKey @mateiz IMHO we should leave the method as it is, as this will make the code ugly. |
|
Can one of the admins verify this patch? |
|
Ah, wow, I never knew that. So if one takes a Partitioner first and one takes a function, the types are inferred, but if both take a function first, they're not? In that case we might want to change our other methods too, like cogroup and groupByKey, to take a Partitioner first. Wouldn't this problem also affect them? |
|
@mateiz I think this only applies with anon function's, thus isn't affecting either cogroup or groupByKey. |
There was a problem hiding this comment.
This line is over 100 chars wide
There was a problem hiding this comment.
@rxin will fix this as soon as, a decision is made over whether we want to do this or not.
|
I never even realized we had a version of reduceByKey where the first argument is not the closure ... |
|
I have one solution to this, although it is technically an API change, so just throwing it out there for discussion. We can remove all the numPartitions: Int arguments, and add an implicit conversion from int to HashPartitioner. |
|
@rxin +1 |
|
I'd rather not add the implicit conversion from int to partitioner, it will be very hard to discover on its own. Instead maybe we can just leave this API as is. It's strange but there's a good reason for it. |
|
QA tests have started for PR 550. This patch merges cleanly. |
|
QA results for PR 550: |
|
It sounds like the conclusion here is to close this issue then. |
This commit exists to close the following pull requests on Github: Closes apache#1328 (close requested by 'pwendell') Closes apache#2314 (close requested by 'pwendell') Closes apache#997 (close requested by 'pwendell') Closes apache#550 (close requested by 'pwendell') Closes apache#1506 (close requested by 'pwendell') Closes apache#2423 (close requested by 'mengxr') Closes apache#554 (close requested by 'joshrosen')
Brings apache#544 (task 233's post) and apache#550 (thirteen rows moved to milestone 7); no conflicts. Generated-by: Claude Code (Claude Opus 5.5)
Brings apache#544 (task 233's post) and apache#550 (thirteen rows moved to milestone 7). PLAN_MILESTONE_6.md conflicted where the moved rows 222 and 224 sit beside row 223, and where row 239 sits beside row 181: each row takes its own side, 222, 224 and 181 master's, 223 and 239 this branch's. Generated-by: Claude Code (Claude Opus 5.5)
Brings apache#544, on which this branch was built, and apache#550 (thirteen rows moved to milestone 7). PLAN_TASK_233.md and POST_MILESTONE_6_GIVEUPS.md conflicted only where this branch had already changed apache#544's lines - section 17 and the two corrected sentences - so this branch's text is kept; PLAN_MILESTONE_6.md is master's, with this branch's note on row 233. Generated-by: Claude Code (Claude Opus 5.5)
6 participants
... second argument
Most of our shuffle methods can take a Partitioner or a number of partitions as a second argument, but for some reason reduceByKey takes the Partitioner as a first argument: http://spark.apache.org/docs/0.9.1/api/core/#org.apache.spark.rdd.PairRDDFunctions.
Deprecated that version and added one where the Partitioner is the second argument.