You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Anand Mazumdar <ma...@gmail.com> on 2015/08/04 18:33:01 UTC

Re: Review Request 36720: Add subscribe-> subscribed workflow for http frameworks

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36720/
-----------------------------------------------------------

(Updated Aug. 4, 2015, 4:33 p.m.)


Review request for mesos, Ben Mahler and Vinod Kone.


Changes
-------

Added functionality to failover http frameworks.


Bugs: MESOS-2294
    https://issues.apache.org/jira/browse/MESOS-2294


Repository: mesos


Description (updated)
-------

Split review out of r36318. This change adds the functionality of making a http call for subscribe and the master responding with a subscribed event on the persistent stream.

Also added functionality for framework failover equivalent of re-register. It should now be possible to merge the subscribed(...) introduced in this review and the re-factor that happened in MESOS-3182.

- Made a new function for exited()/failoverFramework for http frameworks that invoke into the common continuation function for pid/http frameworks thereafter.
- The re-register functionality equivalent goes in _subscribe(...)


Diffs (updated)
-----

  src/master/http.cpp 3772e39015a22655dcad00ad844dc5ddc90db43f 
  src/master/master.hpp ea18c4e0bb0743747401b9cd5ea14ae9b56ae3cc 
  src/master/master.cpp 351a3c2b5f551ad065682cea601d2436258e4544 

Diff: https://reviews.apache.org/r/36720/diff/


Testing (updated)
-------

make check + adding tests in a different patch.


Thanks,

Anand Mazumdar


Re: Review Request 36720: Add subscribe-> subscribed workflow for http frameworks

Posted by Ben Mahler <be...@gmail.com>.

> On Aug. 4, 2015, 9:07 p.m., Ben Mahler wrote:
> > I'll get the /call validation committed.

Let's follow up to get tests added for each of the error cases in /call.


- Ben


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36720/#review94111
-----------------------------------------------------------


On Aug. 4, 2015, 10 p.m., Anand Mazumdar wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36720/
> -----------------------------------------------------------
> 
> (Updated Aug. 4, 2015, 10 p.m.)
> 
> 
> Review request for mesos, Ben Mahler and Vinod Kone.
> 
> 
> Bugs: MESOS-2294
>     https://issues.apache.org/jira/browse/MESOS-2294
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Split review out of r36318. This change adds the functionality of making a http call for subscribe and the master responding with a subscribed event on the persistent stream.
> 
> Also added functionality for framework failover equivalent of re-register. It should now be possible to merge the subscribed(...) introduced in this review and the re-factor that happened in MESOS-3182.
> 
> - Made a new function for exited()/failoverFramework for http frameworks that invoke into the common continuation function for pid/http frameworks thereafter.
> - The re-register functionality equivalent goes in _subscribe(...)
> 
> 
> Diffs
> -----
> 
>   src/master/http.cpp 76e70801925041f08bc94f0ca18c86f1a573b2b3 
>   src/master/master.hpp cd0a5c863d4f74c3bcfd61d8e3046b3ab249be67 
>   src/master/master.cpp 87e11d512f68e2b7285ee1901d422991208baf15 
> 
> Diff: https://reviews.apache.org/r/36720/diff/
> 
> 
> Testing
> -------
> 
> make check + adding tests in a different patch.
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>


Re: Review Request 36720: Add subscribe-> subscribed workflow for http frameworks

Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36720/#review94111
-----------------------------------------------------------


I'll get the /call validation committed.


src/master/http.cpp (line 355)
<https://reviews.apache.org/r/36720/#comment148622>

    Need an else here for UnsupportedMediaType, I'll add this in.


- Ben Mahler


On Aug. 4, 2015, 4:33 p.m., Anand Mazumdar wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36720/
> -----------------------------------------------------------
> 
> (Updated Aug. 4, 2015, 4:33 p.m.)
> 
> 
> Review request for mesos, Ben Mahler and Vinod Kone.
> 
> 
> Bugs: MESOS-2294
>     https://issues.apache.org/jira/browse/MESOS-2294
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Split review out of r36318. This change adds the functionality of making a http call for subscribe and the master responding with a subscribed event on the persistent stream.
> 
> Also added functionality for framework failover equivalent of re-register. It should now be possible to merge the subscribed(...) introduced in this review and the re-factor that happened in MESOS-3182.
> 
> - Made a new function for exited()/failoverFramework for http frameworks that invoke into the common continuation function for pid/http frameworks thereafter.
> - The re-register functionality equivalent goes in _subscribe(...)
> 
> 
> Diffs
> -----
> 
>   src/master/http.cpp 3772e39015a22655dcad00ad844dc5ddc90db43f 
>   src/master/master.hpp ea18c4e0bb0743747401b9cd5ea14ae9b56ae3cc 
>   src/master/master.cpp 351a3c2b5f551ad065682cea601d2436258e4544 
> 
> Diff: https://reviews.apache.org/r/36720/diff/
> 
> 
> Testing
> -------
> 
> make check + adding tests in a different patch.
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>


Re: Review Request 36720: Add subscribe-> subscribed workflow for http frameworks

Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36720/#review94130
-----------------------------------------------------------


I'll get the exited changes committed.


src/master/master.cpp (line 969)
<https://reviews.apache.org/r/36720/#comment148645>

    Hm.. let's check that the framework ids are equal when the writers match.



src/master/master.cpp (line 971)
<https://reviews.apache.org/r/36720/#comment148644>

    s/exited/disconnection/



src/master/master.cpp (line 1021)
<https://reviews.apache.org/r/36720/#comment148648>

    Let's put `_exited` after both `exited` implementations, since it is a shared continuation.


- Ben Mahler


On Aug. 4, 2015, 10 p.m., Anand Mazumdar wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36720/
> -----------------------------------------------------------
> 
> (Updated Aug. 4, 2015, 10 p.m.)
> 
> 
> Review request for mesos, Ben Mahler and Vinod Kone.
> 
> 
> Bugs: MESOS-2294
>     https://issues.apache.org/jira/browse/MESOS-2294
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Split review out of r36318. This change adds the functionality of making a http call for subscribe and the master responding with a subscribed event on the persistent stream.
> 
> Also added functionality for framework failover equivalent of re-register. It should now be possible to merge the subscribed(...) introduced in this review and the re-factor that happened in MESOS-3182.
> 
> - Made a new function for exited()/failoverFramework for http frameworks that invoke into the common continuation function for pid/http frameworks thereafter.
> - The re-register functionality equivalent goes in _subscribe(...)
> 
> 
> Diffs
> -----
> 
>   src/master/http.cpp 76e70801925041f08bc94f0ca18c86f1a573b2b3 
>   src/master/master.hpp cd0a5c863d4f74c3bcfd61d8e3046b3ab249be67 
>   src/master/master.cpp 87e11d512f68e2b7285ee1901d422991208baf15 
> 
> Diff: https://reviews.apache.org/r/36720/diff/
> 
> 
> Testing
> -------
> 
> make check + adding tests in a different patch.
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>


Re: Review Request 36720: Add subscribe-> subscribed workflow for http frameworks

Posted by Anand Mazumdar <ma...@gmail.com>.

> On Aug. 6, 2015, 4:10 p.m., Vinod Kone wrote:
> > src/common/http.cpp, line 60
> > <https://reviews.apache.org/r/36720/diff/8/?file=1033227#file1033227line60>
> >
> >     need a return here to aviod compiler warning.

I am putting in a fix for that. This looks to be as a GCC bug. With -Werror=uninitialized, -Wswitch turned on in our code. It shouldn't ever complain. Clang rightfully figures this out but GCC does not. Would go ahead and put in a default: UNREACHABLE() workaround here for now.


- Anand


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36720/#review94413
-----------------------------------------------------------


On Aug. 6, 2015, 4:08 a.m., Anand Mazumdar wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36720/
> -----------------------------------------------------------
> 
> (Updated Aug. 6, 2015, 4:08 a.m.)
> 
> 
> Review request for mesos, Ben Mahler and Vinod Kone.
> 
> 
> Bugs: MESOS-2294
>     https://issues.apache.org/jira/browse/MESOS-2294
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Split review out of r36318. This change adds the functionality of making a http call for subscribe and the master responding with a subscribed event on the persistent stream.
> 
> Also added functionality for framework failover equivalent of re-register. It should now be possible to merge the subscribed(...) introduced in this review and the re-factor that happened in MESOS-3182.
> 
> - Made a new function for exited()/failoverFramework for http frameworks that invoke into the common continuation function for pid/http frameworks thereafter.
> - The re-register functionality equivalent goes in _subscribe(...)
> 
> 
> Diffs
> -----
> 
>   src/common/http.hpp 9e4290f9e1f72730f63466d498a953cc5031a92b 
>   src/common/http.cpp a74c51d9392d0b0a67d51a0143eb314cfca11245 
>   src/master/http.cpp 76e70801925041f08bc94f0ca18c86f1a573b2b3 
>   src/master/master.hpp e44174976aa64176916827bec4c911333c9a91db 
>   src/master/master.cpp 50b98248463fc4cd48962890c14c7ad64f2b6f43 
> 
> Diff: https://reviews.apache.org/r/36720/diff/
> 
> 
> Testing
> -------
> 
> make check + adding tests in a different patch.
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>


Re: Review Request 36720: Add subscribe-> subscribed workflow for http frameworks

Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36720/#review94413
-----------------------------------------------------------



src/common/http.cpp (line 60)
<https://reviews.apache.org/r/36720/#comment148999>

    need a return here to aviod compiler warning.


- Vinod Kone


On Aug. 6, 2015, 4:08 a.m., Anand Mazumdar wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36720/
> -----------------------------------------------------------
> 
> (Updated Aug. 6, 2015, 4:08 a.m.)
> 
> 
> Review request for mesos, Ben Mahler and Vinod Kone.
> 
> 
> Bugs: MESOS-2294
>     https://issues.apache.org/jira/browse/MESOS-2294
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Split review out of r36318. This change adds the functionality of making a http call for subscribe and the master responding with a subscribed event on the persistent stream.
> 
> Also added functionality for framework failover equivalent of re-register. It should now be possible to merge the subscribed(...) introduced in this review and the re-factor that happened in MESOS-3182.
> 
> - Made a new function for exited()/failoverFramework for http frameworks that invoke into the common continuation function for pid/http frameworks thereafter.
> - The re-register functionality equivalent goes in _subscribe(...)
> 
> 
> Diffs
> -----
> 
>   src/common/http.hpp 9e4290f9e1f72730f63466d498a953cc5031a92b 
>   src/common/http.cpp a74c51d9392d0b0a67d51a0143eb314cfca11245 
>   src/master/http.cpp 76e70801925041f08bc94f0ca18c86f1a573b2b3 
>   src/master/master.hpp e44174976aa64176916827bec4c911333c9a91db 
>   src/master/master.cpp 50b98248463fc4cd48962890c14c7ad64f2b6f43 
> 
> Diff: https://reviews.apache.org/r/36720/diff/
> 
> 
> Testing
> -------
> 
> make check + adding tests in a different patch.
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>


Re: Review Request 36720: Add subscribe-> subscribed workflow for http frameworks

Posted by Anand Mazumdar <ma...@gmail.com>.

> On Aug. 6, 2015, 6:39 p.m., Ben Mahler wrote:
> > src/master/master.cpp, line 4990
> > <https://reviews.apache.org/r/36720/diff/9/?file=1033626#file1033626line4990>
> >
> >     Don't we still want this since we call pid.get()?

Persisted the CHECK_SOME. Removed the misleading message though as we have now implemented http framework failover.


- Anand


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36720/#review94428
-----------------------------------------------------------


On Aug. 6, 2015, 4:25 p.m., Anand Mazumdar wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36720/
> -----------------------------------------------------------
> 
> (Updated Aug. 6, 2015, 4:25 p.m.)
> 
> 
> Review request for mesos, Ben Mahler and Vinod Kone.
> 
> 
> Bugs: MESOS-2294
>     https://issues.apache.org/jira/browse/MESOS-2294
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Split review out of r36318. This change adds the functionality of making a http call for subscribe and the master responding with a subscribed event on the persistent stream.
> 
> Also added functionality for framework failover equivalent of re-register. It should now be possible to merge the subscribed(...) introduced in this review and the re-factor that happened in MESOS-3182.
> 
> - Made a new function for exited()/failoverFramework for http frameworks that invoke into the common continuation function for pid/http frameworks thereafter.
> - The re-register functionality equivalent goes in _subscribe(...)
> 
> 
> Diffs
> -----
> 
>   src/common/http.hpp 9e4290f9e1f72730f63466d498a953cc5031a92b 
>   src/common/http.cpp a74c51d9392d0b0a67d51a0143eb314cfca11245 
>   src/master/http.cpp 76e70801925041f08bc94f0ca18c86f1a573b2b3 
>   src/master/master.hpp e44174976aa64176916827bec4c911333c9a91db 
>   src/master/master.cpp 50b98248463fc4cd48962890c14c7ad64f2b6f43 
> 
> Diff: https://reviews.apache.org/r/36720/diff/
> 
> 
> Testing
> -------
> 
> make check + adding tests in a different patch.
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>


Re: Review Request 36720: Add subscribe-> subscribed workflow for http frameworks

Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36720/#review94428
-----------------------------------------------------------


Thanks, I'll get the HttpConnection change committed, left some notes about things I updated.

Also opened some issues in the subscribe path that I noticed while doing this.


src/common/http.hpp (line 50)
<https://reviews.apache.org/r/36720/#comment149022>

    Hm.. let's at least say that it's based on the content type.



src/common/http.cpp (line 49)
<https://reviews.apache.org/r/36720/#comment149021>

    No need for std:: here



src/common/http.cpp (lines 61 - 63)
<https://reviews.apache.org/r/36720/#comment149020>

    How about we just put UNREACHABLE at the bottom outside the switch? That way, we don't lose the switch warning for missing enum cases, right?



src/master/master.hpp (lines 1542 - 1544)
<https://reviews.apache.org/r/36720/#comment149023>

    Let's have the master decide when to set up this callback. On that note, how about we add a `closed()` on the HttpConnection as well?



src/master/master.cpp (line 2216)
<https://reviews.apache.org/r/36720/#comment149024>

    This change looks like a regression, now we're sending to '`framework->pid`' instead of '`from`'?



src/master/master.cpp 
<https://reviews.apache.org/r/36720/#comment149027>

    Don't we still want this since we call pid.get()?


- Ben Mahler


On Aug. 6, 2015, 4:25 p.m., Anand Mazumdar wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36720/
> -----------------------------------------------------------
> 
> (Updated Aug. 6, 2015, 4:25 p.m.)
> 
> 
> Review request for mesos, Ben Mahler and Vinod Kone.
> 
> 
> Bugs: MESOS-2294
>     https://issues.apache.org/jira/browse/MESOS-2294
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Split review out of r36318. This change adds the functionality of making a http call for subscribe and the master responding with a subscribed event on the persistent stream.
> 
> Also added functionality for framework failover equivalent of re-register. It should now be possible to merge the subscribed(...) introduced in this review and the re-factor that happened in MESOS-3182.
> 
> - Made a new function for exited()/failoverFramework for http frameworks that invoke into the common continuation function for pid/http frameworks thereafter.
> - The re-register functionality equivalent goes in _subscribe(...)
> 
> 
> Diffs
> -----
> 
>   src/common/http.hpp 9e4290f9e1f72730f63466d498a953cc5031a92b 
>   src/common/http.cpp a74c51d9392d0b0a67d51a0143eb314cfca11245 
>   src/master/http.cpp 76e70801925041f08bc94f0ca18c86f1a573b2b3 
>   src/master/master.hpp e44174976aa64176916827bec4c911333c9a91db 
>   src/master/master.cpp 50b98248463fc4cd48962890c14c7ad64f2b6f43 
> 
> Diff: https://reviews.apache.org/r/36720/diff/
> 
> 
> Testing
> -------
> 
> make check + adding tests in a different patch.
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>


Re: Review Request 36720: Add subscribe-> subscribed workflow for http frameworks

Posted by Anand Mazumdar <ma...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36720/
-----------------------------------------------------------

(Updated Aug. 10, 2015, 7:20 p.m.)


Review request for mesos, Ben Mahler and Vinod Kone.


Changes
-------

Re-opened to remove stale depedency.


Bugs: MESOS-2294
    https://issues.apache.org/jira/browse/MESOS-2294


Repository: mesos


Description
-------

Split review out of r36318. This change adds the functionality of making a http call for subscribe and the master responding with a subscribed event on the persistent stream.

Also added functionality for framework failover equivalent of re-register. It should now be possible to merge the subscribed(...) introduced in this review and the re-factor that happened in MESOS-3182.

- Made a new function for exited()/failoverFramework for http frameworks that invoke into the common continuation function for pid/http frameworks thereafter.
- The re-register functionality equivalent goes in _subscribe(...)


Diffs
-----

  src/master/http.cpp 76e70801925041f08bc94f0ca18c86f1a573b2b3 
  src/master/master.hpp 53420ca7d503296fbe11b1ea0795afe2ebf86255 
  src/master/master.cpp d699e4bc3cf734a516a6baf329919e04744b5702 

Diff: https://reviews.apache.org/r/36720/diff/


Testing
-------

make check + adding tests in a different patch.


Thanks,

Anand Mazumdar


Re: Review Request 36720: Add subscribe-> subscribed workflow for http frameworks

Posted by Anand Mazumdar <ma...@gmail.com>.

> On Aug. 7, 2015, 7:14 a.m., Ben Mahler wrote:
> > src/master/master.cpp, line 4950
> > <https://reviews.apache.org/r/36720/diff/10/?file=1034017#file1034017line4950>
> >
> >     Much like when we remove a pid-based framework, we need to wipe the authentication related data here.

Good catch ! Since currently http framework workflow only works when the authentication workflow is not active, I never noticed this, my bad.


> On Aug. 7, 2015, 7:14 a.m., Ben Mahler wrote:
> > src/master/master.cpp, line 4983
> > <https://reviews.apache.org/r/36720/diff/10/?file=1034017#file1034017line4983>
> >
> >     This check will crash when an http scheduler is downgrading to a pid framework.

I was under the impression that we would be tackling downgrades in a later patch and thought that it would be good to crash for the time being then perform "incorrect" behavior. I would add tests around the downgrade scenarios too similar to the upgrade ones I added in 37082.


- Anand


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36720/#review94496
-----------------------------------------------------------


On Aug. 7, 2015, 2:27 a.m., Anand Mazumdar wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36720/
> -----------------------------------------------------------
> 
> (Updated Aug. 7, 2015, 2:27 a.m.)
> 
> 
> Review request for mesos, Ben Mahler and Vinod Kone.
> 
> 
> Bugs: MESOS-2294
>     https://issues.apache.org/jira/browse/MESOS-2294
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Split review out of r36318. This change adds the functionality of making a http call for subscribe and the master responding with a subscribed event on the persistent stream.
> 
> Also added functionality for framework failover equivalent of re-register. It should now be possible to merge the subscribed(...) introduced in this review and the re-factor that happened in MESOS-3182.
> 
> - Made a new function for exited()/failoverFramework for http frameworks that invoke into the common continuation function for pid/http frameworks thereafter.
> - The re-register functionality equivalent goes in _subscribe(...)
> 
> 
> Diffs
> -----
> 
>   src/master/http.cpp 76e70801925041f08bc94f0ca18c86f1a573b2b3 
>   src/master/master.hpp 53420ca7d503296fbe11b1ea0795afe2ebf86255 
>   src/master/master.cpp d699e4bc3cf734a516a6baf329919e04744b5702 
> 
> Diff: https://reviews.apache.org/r/36720/diff/
> 
> 
> Testing
> -------
> 
> make check + adding tests in a different patch.
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>


Re: Review Request 36720: Add subscribe-> subscribed workflow for http frameworks

Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36720/#review94496
-----------------------------------------------------------

Ship it!


I left some comments here so you could see the issues I noticed as I went over this, mostly around upgrading and downgrading across pid and http.

I've applied fixes for all of these to avoid more round-trips and will get this committed shortly, nice work!


src/master/master.cpp (line 4950)
<https://reviews.apache.org/r/36720/#comment149133>

    Much like when we remove a pid-based framework, we need to wipe the authentication related data here.



src/master/master.cpp (lines 4963 - 4968)
<https://reviews.apache.org/r/36720/#comment149135>

    Perhaps we should just push the unsetting of the other connection into updateConnection and have one for both http and pid.



src/master/master.cpp (line 4967)
<https://reviews.apache.org/r/36720/#comment149136>

    We need to remove the old one from `authenticated` and `principals` here when updrading to http, much like we do when a pid based framework is removed.
    
    Yes.. this stuff is a nasty mess, we need to make failover as simple as the composition of: disconnect -> reconnect!



src/master/master.cpp (line 4983)
<https://reviews.apache.org/r/36720/#comment149129>

    This check will crash when an http scheduler is downgrading to a pid framework.



src/master/master.cpp (lines 5010 - 5015)
<https://reviews.apache.org/r/36720/#comment149132>

    This won't work for downgrades from http to pid, since there was no principal in the map yet.



src/master/master.cpp (lines 5039 - 5040)
<https://reviews.apache.org/r/36720/#comment149138>

    Could we do this before reactivating? Note that your comment seems to hint that it must come after for some reason, but it doesn't need to.


- Ben Mahler


On Aug. 7, 2015, 2:27 a.m., Anand Mazumdar wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36720/
> -----------------------------------------------------------
> 
> (Updated Aug. 7, 2015, 2:27 a.m.)
> 
> 
> Review request for mesos, Ben Mahler and Vinod Kone.
> 
> 
> Bugs: MESOS-2294
>     https://issues.apache.org/jira/browse/MESOS-2294
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Split review out of r36318. This change adds the functionality of making a http call for subscribe and the master responding with a subscribed event on the persistent stream.
> 
> Also added functionality for framework failover equivalent of re-register. It should now be possible to merge the subscribed(...) introduced in this review and the re-factor that happened in MESOS-3182.
> 
> - Made a new function for exited()/failoverFramework for http frameworks that invoke into the common continuation function for pid/http frameworks thereafter.
> - The re-register functionality equivalent goes in _subscribe(...)
> 
> 
> Diffs
> -----
> 
>   src/master/http.cpp 76e70801925041f08bc94f0ca18c86f1a573b2b3 
>   src/master/master.hpp 53420ca7d503296fbe11b1ea0795afe2ebf86255 
>   src/master/master.cpp d699e4bc3cf734a516a6baf329919e04744b5702 
> 
> Diff: https://reviews.apache.org/r/36720/diff/
> 
> 
> Testing
> -------
> 
> make check + adding tests in a different patch.
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>


Re: Review Request 36720: Add subscribe-> subscribed workflow for http frameworks

Posted by Anand Mazumdar <ma...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36720/
-----------------------------------------------------------

(Updated Aug. 7, 2015, 2:27 a.m.)


Review request for mesos, Ben Mahler and Vinod Kone.


Changes
-------

Modified signature for subscribe(...), added _failoverFramework. Modified some TODO messages to be more explicit.


Bugs: MESOS-2294
    https://issues.apache.org/jira/browse/MESOS-2294


Repository: mesos


Description
-------

Split review out of r36318. This change adds the functionality of making a http call for subscribe and the master responding with a subscribed event on the persistent stream.

Also added functionality for framework failover equivalent of re-register. It should now be possible to merge the subscribed(...) introduced in this review and the re-factor that happened in MESOS-3182.

- Made a new function for exited()/failoverFramework for http frameworks that invoke into the common continuation function for pid/http frameworks thereafter.
- The re-register functionality equivalent goes in _subscribe(...)


Diffs (updated)
-----

  src/master/http.cpp 76e70801925041f08bc94f0ca18c86f1a573b2b3 
  src/master/master.hpp 53420ca7d503296fbe11b1ea0795afe2ebf86255 
  src/master/master.cpp d699e4bc3cf734a516a6baf329919e04744b5702 

Diff: https://reviews.apache.org/r/36720/diff/


Testing
-------

make check + adding tests in a different patch.


Thanks,

Anand Mazumdar


Re: Review Request 36720: Add subscribe-> subscribed workflow for http frameworks

Posted by Anand Mazumdar <ma...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36720/
-----------------------------------------------------------

(Updated Aug. 6, 2015, 4:25 p.m.)


Review request for mesos, Ben Mahler and Vinod Kone.


Changes
-------

Added a UNREACHABLE for default case to put off GCC for now.


Bugs: MESOS-2294
    https://issues.apache.org/jira/browse/MESOS-2294


Repository: mesos


Description
-------

Split review out of r36318. This change adds the functionality of making a http call for subscribe and the master responding with a subscribed event on the persistent stream.

Also added functionality for framework failover equivalent of re-register. It should now be possible to merge the subscribed(...) introduced in this review and the re-factor that happened in MESOS-3182.

- Made a new function for exited()/failoverFramework for http frameworks that invoke into the common continuation function for pid/http frameworks thereafter.
- The re-register functionality equivalent goes in _subscribe(...)


Diffs (updated)
-----

  src/common/http.hpp 9e4290f9e1f72730f63466d498a953cc5031a92b 
  src/common/http.cpp a74c51d9392d0b0a67d51a0143eb314cfca11245 
  src/master/http.cpp 76e70801925041f08bc94f0ca18c86f1a573b2b3 
  src/master/master.hpp e44174976aa64176916827bec4c911333c9a91db 
  src/master/master.cpp 50b98248463fc4cd48962890c14c7ad64f2b6f43 

Diff: https://reviews.apache.org/r/36720/diff/


Testing
-------

make check + adding tests in a different patch.


Thanks,

Anand Mazumdar


Re: Review Request 36720: Add subscribe-> subscribed workflow for http frameworks

Posted by Anand Mazumdar <ma...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36720/
-----------------------------------------------------------

(Updated Aug. 6, 2015, 4:08 a.m.)


Review request for mesos, Ben Mahler and Vinod Kone.


Changes
-------

Added a continuation function for failoverFramework(...) that is common to both http and pid frameworks. Made the readerClosed callback as part of addFramework similar to link(...)


Bugs: MESOS-2294
    https://issues.apache.org/jira/browse/MESOS-2294


Repository: mesos


Description
-------

Split review out of r36318. This change adds the functionality of making a http call for subscribe and the master responding with a subscribed event on the persistent stream.

Also added functionality for framework failover equivalent of re-register. It should now be possible to merge the subscribed(...) introduced in this review and the re-factor that happened in MESOS-3182.

- Made a new function for exited()/failoverFramework for http frameworks that invoke into the common continuation function for pid/http frameworks thereafter.
- The re-register functionality equivalent goes in _subscribe(...)


Diffs (updated)
-----

  src/common/http.hpp 9e4290f9e1f72730f63466d498a953cc5031a92b 
  src/common/http.cpp a74c51d9392d0b0a67d51a0143eb314cfca11245 
  src/master/http.cpp 76e70801925041f08bc94f0ca18c86f1a573b2b3 
  src/master/master.hpp e44174976aa64176916827bec4c911333c9a91db 
  src/master/master.cpp 50b98248463fc4cd48962890c14c7ad64f2b6f43 

Diff: https://reviews.apache.org/r/36720/diff/


Testing
-------

make check + adding tests in a different patch.


Thanks,

Anand Mazumdar


Re: Review Request 36720: Add subscribe-> subscribed workflow for http frameworks

Posted by Anand Mazumdar <ma...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36720/
-----------------------------------------------------------

(Updated Aug. 5, 2015, 10:37 p.m.)


Review request for mesos, Ben Mahler and Vinod Kone.


Changes
-------

Added a new HttpConnection struct instead of optimistically creating framework objects and deleting them for re-registration.


Bugs: MESOS-2294
    https://issues.apache.org/jira/browse/MESOS-2294


Repository: mesos


Description
-------

Split review out of r36318. This change adds the functionality of making a http call for subscribe and the master responding with a subscribed event on the persistent stream.

Also added functionality for framework failover equivalent of re-register. It should now be possible to merge the subscribed(...) introduced in this review and the re-factor that happened in MESOS-3182.

- Made a new function for exited()/failoverFramework for http frameworks that invoke into the common continuation function for pid/http frameworks thereafter.
- The re-register functionality equivalent goes in _subscribe(...)


Diffs (updated)
-----

  src/common/http.hpp 9e4290f9e1f72730f63466d498a953cc5031a92b 
  src/common/http.cpp a74c51d9392d0b0a67d51a0143eb314cfca11245 
  src/master/http.cpp 76e70801925041f08bc94f0ca18c86f1a573b2b3 
  src/master/master.hpp e44174976aa64176916827bec4c911333c9a91db 
  src/master/master.cpp 50b98248463fc4cd48962890c14c7ad64f2b6f43 

Diff: https://reviews.apache.org/r/36720/diff/


Testing
-------

make check + adding tests in a different patch.


Thanks,

Anand Mazumdar


Re: Review Request 36720: Add subscribe-> subscribed workflow for http frameworks

Posted by Anand Mazumdar <ma...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36720/
-----------------------------------------------------------

(Updated Aug. 4, 2015, 11:57 p.m.)


Review request for mesos, Ben Mahler and Vinod Kone.


Changes
-------

Rebased from master after Ben's commit for exited


Bugs: MESOS-2294
    https://issues.apache.org/jira/browse/MESOS-2294


Repository: mesos


Description
-------

Split review out of r36318. This change adds the functionality of making a http call for subscribe and the master responding with a subscribed event on the persistent stream.

Also added functionality for framework failover equivalent of re-register. It should now be possible to merge the subscribed(...) introduced in this review and the re-factor that happened in MESOS-3182.

- Made a new function for exited()/failoverFramework for http frameworks that invoke into the common continuation function for pid/http frameworks thereafter.
- The re-register functionality equivalent goes in _subscribe(...)


Diffs (updated)
-----

  src/master/http.cpp 76e70801925041f08bc94f0ca18c86f1a573b2b3 
  src/master/master.hpp e44174976aa64176916827bec4c911333c9a91db 
  src/master/master.cpp 5aa0a5410804fe16abd50b6953f1ffe46a019ecf 

Diff: https://reviews.apache.org/r/36720/diff/


Testing
-------

make check + adding tests in a different patch.


Thanks,

Anand Mazumdar


Re: Review Request 36720: Add subscribe-> subscribed workflow for http frameworks

Posted by Anand Mazumdar <ma...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36720/
-----------------------------------------------------------

(Updated Aug. 4, 2015, 10 p.m.)


Review request for mesos, Ben Mahler and Vinod Kone.


Changes
-------

Rebased from master from Ben's latest commit.


Bugs: MESOS-2294
    https://issues.apache.org/jira/browse/MESOS-2294


Repository: mesos


Description
-------

Split review out of r36318. This change adds the functionality of making a http call for subscribe and the master responding with a subscribed event on the persistent stream.

Also added functionality for framework failover equivalent of re-register. It should now be possible to merge the subscribed(...) introduced in this review and the re-factor that happened in MESOS-3182.

- Made a new function for exited()/failoverFramework for http frameworks that invoke into the common continuation function for pid/http frameworks thereafter.
- The re-register functionality equivalent goes in _subscribe(...)


Diffs (updated)
-----

  src/master/http.cpp 76e70801925041f08bc94f0ca18c86f1a573b2b3 
  src/master/master.hpp cd0a5c863d4f74c3bcfd61d8e3046b3ab249be67 
  src/master/master.cpp 87e11d512f68e2b7285ee1901d422991208baf15 

Diff: https://reviews.apache.org/r/36720/diff/


Testing
-------

make check + adding tests in a different patch.


Thanks,

Anand Mazumdar