You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@spark.apache.org by Jacek Laskowski <ja...@japila.pl> on 2016/10/13 12:56:29 UTC

DAGScheduler.handleJobCancellation uses jobIdToStageIds for verification while jobIdToActiveJob for lookup?

Hi,

Is there a reason why DAGScheduler.handleJobCancellation checks the
active job id in jobIdToStageIds [1] while looking the job up in
jobIdToActiveJob [2]? Perhaps synchronized earlier yet still
inconsistent.

[1] https://github.com/apache/spark/blob/master/core/src/main/scala/org/apache/spark/scheduler/DAGScheduler.scala#L1372
[2] https://github.com/apache/spark/blob/master/core/src/main/scala/org/apache/spark/scheduler/DAGScheduler.scala#L1376

Pozdrawiam,
Jacek Laskowski
----
https://medium.com/@jaceklaskowski/
Mastering Apache Spark 2.0 https://bit.ly/mastering-apache-spark
Follow me at https://twitter.com/jaceklaskowski

---------------------------------------------------------------------
To unsubscribe e-mail: dev-unsubscribe@spark.apache.org


Re: DAGScheduler.handleJobCancellation uses jobIdToStageIds for verification while jobIdToActiveJob for lookup?

Posted by Mark Hamstra <ma...@clearstorydata.com>.
There were at least a couple of ideas behind the original thinking on using
both of those Maps: 1) We needed the ability to efficiently get from jobId
to both ActiveJob objects and to sets of associated Stages, and using both
Maps here was an opportunity to do a little sanity checking to make sure
that the Maps looked at least minimally consistent for the Job at issue; 2)
Similarly, it could serve as a kind of hierarchical check -- first, for the
Job which we are being asked to cancel, that we ever knew enough to even
register its existence; second, that for a JobId that passes the first
test, that we still have an ActiveJob that can be canceled.

Now, without doing a bunch of digging into the code archives, I can't tell
you for sure whether those ideas were ever implemented completely correctly
or whether they still make valid sense in the current code, but from
looking at the lines that you highlighted, I can tell you that even if the
ideas still make sense and are worth carrying forward, the current code
doesn't implement them correctly.  In particular, if it is possible for the
`jobId` to not be in `jobIdToActiveJob`, we're going to produce a
`NoSuchElementException` for the missing key instead of handling it or even
reporting it in a more useful way.

On Thu, Oct 13, 2016 at 8:11 AM, Jacek Laskowski <ja...@japila.pl> wrote:

> Thanks Imran! Not only did the response come so promptly, but also
> it's something I could work on (and have another Spark contributor
> badge unlocked)! Thanks.
>
> Pozdrawiam,
> Jacek Laskowski
> ----
> https://medium.com/@jaceklaskowski/
> Mastering Apache Spark 2.0 https://bit.ly/mastering-apache-spark
> Follow me at https://twitter.com/jaceklaskowski
>
>
> On Thu, Oct 13, 2016 at 5:02 PM, Imran Rashid <ir...@cloudera.com>
> wrote:
> > Hi Jacek,
> >
> > doesn't look like there is any good reason -- Mark Hamstra might know
> this
> > best.  Feel free to open a jira & pr for it, you can ping Mark, Kay
> > Ousterhout, and me (@squito) for review.
> >
> > Imran
> >
> > On Thu, Oct 13, 2016 at 7:56 AM, Jacek Laskowski <ja...@japila.pl>
> wrote:
> >>
> >> Hi,
> >>
> >> Is there a reason why DAGScheduler.handleJobCancellation checks the
> >> active job id in jobIdToStageIds [1] while looking the job up in
> >> jobIdToActiveJob [2]? Perhaps synchronized earlier yet still
> >> inconsistent.
> >>
> >> [1]
> >> https://github.com/apache/spark/blob/master/core/src/
> main/scala/org/apache/spark/scheduler/DAGScheduler.scala#L1372
> >> [2]
> >> https://github.com/apache/spark/blob/master/core/src/
> main/scala/org/apache/spark/scheduler/DAGScheduler.scala#L1376
> >>
> >> Pozdrawiam,
> >> Jacek Laskowski
> >> ----
> >> https://medium.com/@jaceklaskowski/
> >> Mastering Apache Spark 2.0 https://bit.ly/mastering-apache-spark
> >> Follow me at https://twitter.com/jaceklaskowski
> >>
> >> ---------------------------------------------------------------------
> >> To unsubscribe e-mail: dev-unsubscribe@spark.apache.org
> >>
> >
>

Re: DAGScheduler.handleJobCancellation uses jobIdToStageIds for verification while jobIdToActiveJob for lookup?

Posted by Jacek Laskowski <ja...@japila.pl>.
Thanks Imran! Not only did the response come so promptly, but also
it's something I could work on (and have another Spark contributor
badge unlocked)! Thanks.

Pozdrawiam,
Jacek Laskowski
----
https://medium.com/@jaceklaskowski/
Mastering Apache Spark 2.0 https://bit.ly/mastering-apache-spark
Follow me at https://twitter.com/jaceklaskowski


On Thu, Oct 13, 2016 at 5:02 PM, Imran Rashid <ir...@cloudera.com> wrote:
> Hi Jacek,
>
> doesn't look like there is any good reason -- Mark Hamstra might know this
> best.  Feel free to open a jira & pr for it, you can ping Mark, Kay
> Ousterhout, and me (@squito) for review.
>
> Imran
>
> On Thu, Oct 13, 2016 at 7:56 AM, Jacek Laskowski <ja...@japila.pl> wrote:
>>
>> Hi,
>>
>> Is there a reason why DAGScheduler.handleJobCancellation checks the
>> active job id in jobIdToStageIds [1] while looking the job up in
>> jobIdToActiveJob [2]? Perhaps synchronized earlier yet still
>> inconsistent.
>>
>> [1]
>> https://github.com/apache/spark/blob/master/core/src/main/scala/org/apache/spark/scheduler/DAGScheduler.scala#L1372
>> [2]
>> https://github.com/apache/spark/blob/master/core/src/main/scala/org/apache/spark/scheduler/DAGScheduler.scala#L1376
>>
>> Pozdrawiam,
>> Jacek Laskowski
>> ----
>> https://medium.com/@jaceklaskowski/
>> Mastering Apache Spark 2.0 https://bit.ly/mastering-apache-spark
>> Follow me at https://twitter.com/jaceklaskowski
>>
>> ---------------------------------------------------------------------
>> To unsubscribe e-mail: dev-unsubscribe@spark.apache.org
>>
>

---------------------------------------------------------------------
To unsubscribe e-mail: dev-unsubscribe@spark.apache.org


Re: DAGScheduler.handleJobCancellation uses jobIdToStageIds for verification while jobIdToActiveJob for lookup?

Posted by Imran Rashid <ir...@cloudera.com>.
Hi Jacek,

doesn't look like there is any good reason -- Mark Hamstra might know this
best.  Feel free to open a jira & pr for it, you can ping Mark, Kay
Ousterhout, and me (@squito) for review.

Imran

On Thu, Oct 13, 2016 at 7:56 AM, Jacek Laskowski <ja...@japila.pl> wrote:

> Hi,
>
> Is there a reason why DAGScheduler.handleJobCancellation checks the
> active job id in jobIdToStageIds [1] while looking the job up in
> jobIdToActiveJob [2]? Perhaps synchronized earlier yet still
> inconsistent.
>
> [1] https://github.com/apache/spark/blob/master/core/src/main/
> scala/org/apache/spark/scheduler/DAGScheduler.scala#L1372
> [2] https://github.com/apache/spark/blob/master/core/src/main/
> scala/org/apache/spark/scheduler/DAGScheduler.scala#L1376
>
> Pozdrawiam,
> Jacek Laskowski
> ----
> https://medium.com/@jaceklaskowski/
> Mastering Apache Spark 2.0 https://bit.ly/mastering-apache-spark
> Follow me at https://twitter.com/jaceklaskowski
>
> ---------------------------------------------------------------------
> To unsubscribe e-mail: dev-unsubscribe@spark.apache.org
>
>