[SPARK-28593][CORE] Rename ShuffleClient to BlockStoreClient which more close to its usage

[xuanyuanking]

What changes were proposed in this pull request?

After SPARK-27677, the shuffle client not only handles the shuffle block but also responsible for local persist RDD blocks. For better code scalability and precise semantics(as the discussion), here we did several changes:

• Rename ShuffleClient to BlockStoreClient.
• Correspondingly rename the ExternalShuffleClient to ExternalBlockStoreClient, also change the server-side class from ExternalShuffleBlockHandler to ExternalBlockHandler.
• Move MesosExternalBlockStoreClient to Mesos package.

Note, we still keep the name of BlockTransferService, because the Service contains both client and server, also the name of BlockTransferService is not referencing shuffle client only.

How was this patch tested?

Existing UT.

[SparkQA]

Test build #108522 has finished for PR 25327 at commit 6e02ff0.

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

[xuanyuanking]

@cloud-fan

[cloud-fan]

rename the variable as well?

[xuanyuanking]

Initially I tried this, but there are plenty of names bind with Shuffle, so in ae64538 I just change this param name and the member var shuffleClient.

[cloud-fan]

the new name loos clearer, also cc @attilapiros @vanzin @tgravescs

[SparkQA]

Test build #108544 has finished for PR 25327 at commit f8d9777.

• This patch fails Spark unit tests.
• This patch merges cleanly.
• This patch adds no public classes.

[SparkQA]

Test build #108547 has finished for PR 25327 at commit ae64538.

• This patch fails due to an unknown error code, -9.
• This patch merges cleanly.
• This patch adds no public classes.

[xuanyuanking]

retest this please.

[SparkQA]

Test build #108560 has finished for PR 25327 at commit ae64538.

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

[tgravescs]

I'm ok with renaming, it is more clear. My only concern is changing public api's when maybe not truly needed, although this is 3.0 so the time to do it and I don't think this will impact very many users.

[cloud-fan]

AFAIK the network is an internal module, these interfaces/classes are not even developer APIs. I think it's fine to change them.

[attilapiros]

I think the new names are much bette even self explanatory.

But what about going one step further and renaming the ExternalShuffleService to ExternalBlockStoreService?

I know it will generate a lot of documentation changes and a temporary mapping in the mind of the operators running Spark but if we would like to ever fix this then this is the best time to do so.

cc @squito

[xuanyuanking]

Thanks for the review and comments.
@attilapiros Yep, I tried to rename the ExternalShuffleService to ExternalBlockStoreService, compare with the present changes, it's more like an API change and influence for end users. After changing the class to ExternalBlockStoreService, we'll get a dilemma that should we also change the plenty of variable names, related classes and configs name. Maybe we just limit the scope to the developers, better not to the end-users. WDYT?

[cloud-fan]

It takes time to educate the users that external shuffle service is now external block service. We need to update documents, configs, etc. and probably need a few blog posts, presentations, etc.

I don't think it's a good idea to do them with one single PR. I'd like to merge this PR first, as a start of internal renaming.

[cloud-fan]

thanks, merging to master!

[attilapiros]

ok, I see skipping its rename was result of a conscientious decision. That's fine.

[xuanyuanking]

Yep, thanks for the advice and review :)