You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Alexander Rukletsov <ru...@gmail.com> on 2017/08/10 20:41:20 UTC
Review Request 61579: Ensured JAVA HTTP adapter propagates a
subscription error.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61579/
-----------------------------------------------------------
Review request for mesos, Anand Mazumdar and Vinod Kone.
Bugs: MESOS-7872
https://issues.apache.org/jira/browse/MESOS-7872
Repository: mesos
Description
-------
Prior to this patch, if an error occurred during subscription /
registration to the master, it was not propagated back to the
scheduler if the HTTP adapter was used. This happened because
the HTTP adapter does not call `scheduler.connected` until after
successful registration and hence the scheduler does not try to
send the `SUBSCRIBE` call, without which the adapter does not
send any events to the scheduler.
A fix is to call `scheduler.connected` if an error occurred
before the scheduler had subscribed.
Diffs
-----
src/java/jni/org_apache_mesos_v1_scheduler_V0Mesos.cpp 1f58fbff4e8414e4d2ae4c8f69b637ee3315e411
Diff: https://reviews.apache.org/r/61579/diff/1/
Testing
-------
See https://reviews.apache.org/r/61580/
Thanks,
Alexander Rukletsov
Re: Review Request 61579: Ensured JAVA HTTP adapter propagates a
subscription error.
Posted by Alexander Rukletsov <ru...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61579/
-----------------------------------------------------------
(Updated Aug. 31, 2017, 7:09 a.m.)
Review request for mesos, Anand Mazumdar, Till Toenshoff, and Vinod Kone.
Bugs: MESOS-7872
https://issues.apache.org/jira/browse/MESOS-7872
Repository: mesos
Description
-------
Prior to this patch, if an error occurred during subscription /
registration to the master, it was not propagated back to the
scheduler if the HTTP adapter was used. This happened because
the HTTP adapter does not call `scheduler.connected` until after
successful registration and hence the scheduler does not try to
send the `SUBSCRIBE` call, without which the adapter does not
send any events to the scheduler.
A fix is to call `scheduler.connected` if an error occurred
before the scheduler had subscribed.
Diffs (updated)
-----
src/java/jni/org_apache_mesos_v1_scheduler_V0Mesos.cpp 1f58fbff4e8414e4d2ae4c8f69b637ee3315e411
Diff: https://reviews.apache.org/r/61579/diff/3/
Changes: https://reviews.apache.org/r/61579/diff/2-3/
Testing
-------
See https://reviews.apache.org/r/61580/
Thanks,
Alexander Rukletsov
Re: Review Request 61579: Ensured JAVA HTTP adapter propagates a
subscription error.
Posted by Anand Mazumdar <an...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61579/#review184203
-----------------------------------------------------------
Fix it, then Ship it!
LGTM minus a small nit.
src/java/jni/org_apache_mesos_v1_scheduler_V0Mesos.cpp
Lines 534-537 (patched)
<https://reviews.apache.org/r/61579/#comment260272>
This is not neccessarily true. An error can also be due to other things like providing the wrong master URL etc.
Can we clarify here that we do this workaround to mimic the invariant of the v1 interface that we can only receive an event after successfully connecting with the master?
- Anand Mazumdar
On Aug. 30, 2017, 2:16 p.m., Alexander Rukletsov wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61579/
> -----------------------------------------------------------
>
> (Updated Aug. 30, 2017, 2:16 p.m.)
>
>
> Review request for mesos, Anand Mazumdar, Till Toenshoff, and Vinod Kone.
>
>
> Bugs: MESOS-7872
> https://issues.apache.org/jira/browse/MESOS-7872
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Prior to this patch, if an error occurred during subscription /
> registration to the master, it was not propagated back to the
> scheduler if the HTTP adapter was used. This happened because
> the HTTP adapter does not call `scheduler.connected` until after
> successful registration and hence the scheduler does not try to
> send the `SUBSCRIBE` call, without which the adapter does not
> send any events to the scheduler.
>
> A fix is to call `scheduler.connected` if an error occurred
> before the scheduler had subscribed.
>
>
> Diffs
> -----
>
> src/java/jni/org_apache_mesos_v1_scheduler_V0Mesos.cpp 1f58fbff4e8414e4d2ae4c8f69b637ee3315e411
>
>
> Diff: https://reviews.apache.org/r/61579/diff/2/
>
>
> Testing
> -------
>
> See https://reviews.apache.org/r/61580/
>
>
> Thanks,
>
> Alexander Rukletsov
>
>
Re: Review Request 61579: Ensured JAVA HTTP adapter propagates a
subscription error.
Posted by Alexander Rukletsov <ru...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61579/
-----------------------------------------------------------
(Updated Aug. 30, 2017, 2:16 p.m.)
Review request for mesos, Anand Mazumdar, Till Toenshoff, and Vinod Kone.
Bugs: MESOS-7872
https://issues.apache.org/jira/browse/MESOS-7872
Repository: mesos
Description
-------
Prior to this patch, if an error occurred during subscription /
registration to the master, it was not propagated back to the
scheduler if the HTTP adapter was used. This happened because
the HTTP adapter does not call `scheduler.connected` until after
successful registration and hence the scheduler does not try to
send the `SUBSCRIBE` call, without which the adapter does not
send any events to the scheduler.
A fix is to call `scheduler.connected` if an error occurred
before the scheduler had subscribed.
Diffs (updated)
-----
src/java/jni/org_apache_mesos_v1_scheduler_V0Mesos.cpp 1f58fbff4e8414e4d2ae4c8f69b637ee3315e411
Diff: https://reviews.apache.org/r/61579/diff/2/
Changes: https://reviews.apache.org/r/61579/diff/1-2/
Testing
-------
See https://reviews.apache.org/r/61580/
Thanks,
Alexander Rukletsov