Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SPARK-28560][SQL][followup] code cleanup for local shuffle reader #26128

Closed
wants to merge 1 commit into from

Conversation

cloud-fan
Copy link
Contributor

@cloud-fan cloud-fan commented Oct 15, 2019

What changes were proposed in this pull request?

A followup of #25295

This PR proposes a few code cleanups:

  1. rename the special getMapSizesByExecutorId to getMapSizesByMapIndex
  2. rename the parameter mapId to mapIndex as that's really a mapper index.
  3. BlockStoreShuffleReader should take blocksByAddress directly instead of a map id.
  4. rename getMapReader to getReaderForOneMapper to be more clearer.

Why are the changes needed?

make code easier to understand

Does this PR introduce any user-facing change?

no

How was this patch tested?

existing tests

@cloud-fan
Copy link
Contributor Author

@JkSelf @xuanyuanking

@SparkQA
Copy link

SparkQA commented Oct 15, 2019

Test build #112111 has finished for PR 26128 at commit 0c0191e.

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

@JkSelf
Copy link
Contributor

JkSelf commented Oct 16, 2019

LGTM. Thanks @cloud-fan working on this.

@cloud-fan cloud-fan closed this in 51f10ed Oct 16, 2019
@cloud-fan
Copy link
Contributor Author

thanks for the review, merging to master!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
4 participants