Skip to content

Conversation

@attilapiros
Copy link
Contributor

@attilapiros attilapiros commented Dec 12, 2019

What changes were proposed in this pull request?

When spark.shuffle.useOldFetchProtocol is enabled then switching off the direct disk reading of host-local shuffle blocks and falling back to remote block fetching (and this way avoiding the GetLocalDirsForExecutors block transfer message which is introduced from Spark 3.0.0).

Why are the changes needed?

In [SPARK-27651][Core] Avoid the network when shuffle blocks are fetched from the same host a new block transfer message is introduced, GetLocalDirsForExecutors. This new message could be sent to the external shuffle service and as it is not supported by the previous version of external shuffle service it should be avoided when spark.shuffle.useOldFetchProtocol is true.

In the migration guide I changed the exception type as org.apache.spark.network.shuffle.protocol.BlockTransferMessage.Decoder#fromByteBuffer
throws a IllegalArgumentException with the given text and uses the message type which is just a simple number (byte). I have checked and this is true for version 2.4.4 too.

Does this PR introduce any user-facing change?

No.

How was this patch tested?

This specific case (considering one extra boolean to switch off host local disk reading feature) is not tested but existing tests were run.

@SparkQA
Copy link

SparkQA commented Dec 12, 2019

Test build #115245 has finished for PR 26869 at commit a7b3976.

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

@dongjoon-hyun
Copy link
Member

dongjoon-hyun commented Dec 14, 2019

BTW, @attilapiros . Could you recover the original PR template? What changes were proposed in this pull request? and Does this PR introduce any user-facing change? is missing here.

@SparkQA
Copy link

SparkQA commented Dec 15, 2019

Test build #115357 has finished for PR 26869 at commit 7ca4c6d.

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

@SparkQA
Copy link

SparkQA commented Dec 17, 2019

Test build #115426 has finished for PR 26869 at commit 69459e7.

  • This patch fails to build.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Dec 17, 2019

Test build #115434 has finished for PR 26869 at commit 6258ccd.

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

@attilapiros
Copy link
Contributor Author

jenkins retest this please

@SparkQA
Copy link

SparkQA commented Dec 17, 2019

Test build #115450 has finished for PR 26869 at commit 6258ccd.

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

@vanzin
Copy link
Contributor

vanzin commented Dec 17, 2019

Merging to master.

@vanzin vanzin closed this in cdc8fc6 Dec 17, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants