You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2019/12/07 01:18:24 UTC

[GitHub] [spark] bmarcott commented on issue #26696: [WIP][SPARK-18886][CORE] Make locality wait time be the time since a TSM's available slots were fully utilized

bmarcott commented on issue #26696: [WIP][SPARK-18886][CORE] Make locality wait time be the time since a TSM's available slots were fully utilized
URL: https://github.com/apache/spark/pull/26696#issuecomment-562798874
 
 
   
   >  arguably you should never reset back to a lower level. 
   
   Yea I was thinking the same thing, but I decided not to touch that part yet, since I have mostly thought about **when** to reset.
   
   
   
   > Note on the fairscheduler side - I think you do have the issue Kay brought up in v2 of comment: https://issues.apache.org/jira/browse/SPARK-18886?focusedCommentId=15931009&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-15931009
   
   I do not believe this PR has that issue, as once the non-greedy taskset has waited its locality wait time it will increase locality level since it wasn't able to take advantage of its slots. Her proposed v1 solution had that problem since it only increased locality level if there were slots not being used by **any job**.
   
   > I assume it fixes the performance on #26633? thanks for adding more unit tests, did run any manual tests as well?
   
   I haven't looked in depth at their problem, but it does seem it would solve most of the problem.
   I have not yet had the chance to run manual tests. 
   
   > so looking at the fairscheduler side of this it brings me back to the real question of what is the definition of a task wait. If you look at Kay's example:
   
   I did not catch your point from the comment that started with above quote. This PR handles the case she mentioned since locality level would not increase because it is utilizing all its available slots (timers will keep getting reset). That means when the taskset is offered the new non-local resource, it wouldn't immediately take advantage of it
   
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org