-
Notifications
You must be signed in to change notification settings - Fork 28.2k
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-2208] fix zero shuffle wait time in fast machine #3380
Conversation
Can one of the admins verify this patch? |
cc @aarondav |
I am not familiar with this issue or fix. Could you describe in a bit more detail what the intended solution is? |
this unit test is try to verify the local metrics, and the shuffleReadMetrics measure time fetch data. |
@aarondav any more suggestion ? |
@@ -19,12 +19,19 @@ package org.apache.spark.scheduler | |||
|
|||
import java.util.concurrent.Semaphore | |||
|
|||
import akka.actor.ActorSystem |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: import ordering should abide by the style guide
Actually, I realized it might simplify this patch a lot if instead of extending BlockManager, we extend SortShuffleManager which returns a ShuffleBlockManager that injects a wait into getBlockData. This way you could just do val myConf = conf.duplicate
.set("spark.shuffle.manager", clasOf[SlowShuffleManager].getName)
sc = new SparkContext(myConf) rather than doing all this work creating SparkEnvs and such, which will be annoying any time we add a new parameter to BlockManager or SparkEnv. Note that I would also refactor this whole suite to construct a single conf once: and in each test to just use (rather than in This would allow constructing special SparkContexts without having to have 2 simultaneously like you do with sc2. |
@aarondav , thanks point out the style issue, and I think set SlowShuffleManager is a better idea too. thanks. |
## What changes were proposed in this pull request? A fix for local metrics tests that can fail on fast machines. This is probably what is suggested here #3380 by aarondav? ## How was this patch tested? CI Tests Cheers Author: Joan <[email protected]> Closes #11747 from joan38/SPARK-2208-Local-metrics-tests.
No description provided.