[SPARK-8430]ExternalShuffleBlockResolver of shuffle service should support UnsafeShuffleManager

[lianhuiwang]

@andrewor14 can you take a look?thanks

[SparkQA]

Test build #35108 has finished for PR 6873 at commit 2b27b19.

• This patch fails SparkR unit tests.
• This patch merges cleanly.
• This patch adds the following public classes (experimental):
  • class ElementwiseProduct(VectorTransformer):
  • case class CreateStruct(children: Seq[Expression]) extends Expression
  • case class Logarithm(left: Expression, right: Expression)
  • case class SetCommand(kv: Option[(String, Option[String])]) extends RunnableCommand with Logging

[JoshRosen]

Ah, interesting: I purposely made sure that the on-disk format would be compatible with the format used by the SortShuffleManager, but I guess I overlooked this.

@andrewor14, the fact that tests didn't catch this suggests that we're not testing many combinations of shuffle managers + external shuffle service. Do you want to open some followup JIRAs / tasks to improve coverage here as part of separate PRs?

[JoshRosen]

Jenkins, retest this please.

[andrewor14]

this could just be merged with the previous case:

} else if ("org.apache.spark.shuffle.sort.SortShuffleManager".equals(executor.shuffleManager) ||
    "org.apache.spark.shuffle.unsafe.UnsafeShuffleManager".equals(executor.shuffleManager)) {
  return getSortBasedShuffleBlockData(...);
}

[andrewor14]

Ok, I filed https://issues.apache.org/jira/browse/SPARK-8447.

[andrewor14]

@JoshRosen is this fix sufficient in fixing the issue?

[andrewor14]

Also, @lianhuiwang would you mind updating the title of the PR to exclude references to YARN? This is not a YARN issue, but rather an external shuffle service issue in general.

[JoshRosen]

@andrewor14, yep, this fix looks good to me.

[SparkQA]

Test build #35143 has finished for PR 6873 at commit 2b27b19.

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

[lianhuiwang]

@andrewor14 @JoshRosen now i have tested unsafeShuffleManger on yarn's external shuffle service and that's ok because now file structures of unsafeManger's shuffle are same as sortShuffleManger.

[JoshRosen]

@lianhuiwang SGTM. I'll merge this after it passes tests.

[lianhuiwang]

@JoshRosen thanks.

[SparkQA]

Test build #35204 has finished for PR 6873 at commit 51c47ca.

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

[JoshRosen]

@lianhuiwang, I think this is ready to merge, but do you mind addressing @andrewor14's comment about the title first? The external shuffle service isn't only for use in YARN, so we shouldn't include YARN in the PR title (which becomes the commit title unless I manually fix it up myself on merge).

[lianhuiwang]

@JoshRosen yes, i have update PR's title and removed words about yarn. thanks.

[andrewor14]

Ok, merging into master 1.4