opened
vanzin
username_0
This should help reduce latency for new executor allocations.
[SPARK-8059] [yarn] Wake up allocation thread when new requests arrive.
issue
created
srowen
username_1
LGTM. I was trying to think if there was anything in `java.util.concurrent` that makes this simpler but couldn't find anything, and this is pretty straightforward as is.
comment
created
srowen
username_1
Yeah spurious wakeup doesn't matter here. It would just check again early.
comment
created
andrewor14
username_2
LGTM. I'll let this sit for another day or so in case others have anything else to add.
comment
created
harishreedharan
username_3
Yep, in this case it is likely fine, but is good practice to have the loop
inside the synchronized (the variable that is checked by the loop should be
updated inside the synchronized). Many static analysis tools will flag this
though (not sure about Google ErrorProne though).
comment
created
vanzin
username_0
In case you guys have not noticed, there's no variable to check, so you can't loop on anything. This is just a plain "wake up".

Yes, you could add one, but that would just complicate the code (e.g. figuring out how much to sleep after a spurious wake up).
comment
created
srowen
username_1
It's good practice when you are `wait()`-ing on some condition to be true, but that is not the case here. It may be flagged but it is not an instance of the actual problem these tools would be trying to detect.
comment
created
harishreedharan
username_3
Not suggesting that we change it here - but it looks like ErrorProne does have a check which we might have to disable for this file - http://errorprone.info/bugpattern/WaitNotInLoop
comment
created
andrewor14
username_2
master 1.4
comment