Re: Review Request 53896: Refactored representation of framework connectedness.

2016-12-07 Thread Neil Conway 

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

(Updated Dec. 7, 2016, 8:06 p.m.)


Review request for mesos and Vinod Kone.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Previously, the master used a bool to track whether a given framework is
connected. This commit adjusts the master to use an enum instead. The
enum currently only has two values, CONNECTED and DISCONNECTED, but an
additional value (RECOVERED) will be introduced shortly.


Diffs (updated)
-

  src/master/http.cpp ac560d1fdd219d0de0c5d987a32a7112e149602f 
  src/master/master.hpp b444b2352360fb4f7179acd97dffc0cd81cc7afa 
  src/master/master.cpp 67f32229470da4cf7953881d1c5dcb99393002de 
  src/master/quota_handler.cpp 5578663f26d9b737499dc4f5a286c34c37645442 

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


Testing
---

`make check`


Thanks,

Neil Conway

Re: Review Request 53896: Refactored representation of framework connectedness.

2016-12-05 Thread Vinod Kone 

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


Ship it!




Ship It!

- Vinod Kone


On Dec. 5, 2016, 5:06 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53896/
> ---
> 
> (Updated Dec. 5, 2016, 5:06 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-6419
> https://issues.apache.org/jira/browse/MESOS-6419
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously, the master used a bool to track whether a given framework is
> connected. This commit adjusts the master to use an enum instead. The
> enum currently only has two values, CONNECTED and DISCONNECTED, but an
> additional value (RECOVERED) will be introduced shortly.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp ac560d1fdd219d0de0c5d987a32a7112e149602f 
>   src/master/master.hpp b444b2352360fb4f7179acd97dffc0cd81cc7afa 
>   src/master/master.cpp b0670d993348d189fafff0f83f9da0c5b18d1c51 
>   src/master/quota_handler.cpp 5578663f26d9b737499dc4f5a286c34c37645442 
> 
> Diff: https://reviews.apache.org/r/53896/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 53896: Refactored representation of framework connectedness.

2016-12-05 Thread Neil Conway 

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

(Updated Dec. 5, 2016, 5:06 p.m.)


Review request for mesos and Vinod Kone.


Changes
---

Add TODO comment.


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


Repository: mesos


Description
---

Previously, the master used a bool to track whether a given framework is
connected. This commit adjusts the master to use an enum instead. The
enum currently only has two values, CONNECTED and DISCONNECTED, but an
additional value (RECOVERED) will be introduced shortly.


Diffs (updated)
-

  src/master/http.cpp ac560d1fdd219d0de0c5d987a32a7112e149602f 
  src/master/master.hpp b444b2352360fb4f7179acd97dffc0cd81cc7afa 
  src/master/master.cpp b0670d993348d189fafff0f83f9da0c5b18d1c51 
  src/master/quota_handler.cpp 5578663f26d9b737499dc4f5a286c34c37645442 

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


Testing
---

`make check`


Thanks,

Neil Conway

Re: Review Request 53896: Refactored representation of framework connectedness.

2016-12-05 Thread Neil Conway 


> On Dec. 2, 2016, 10:54 p.m., Vinod Kone wrote:
> > src/master/master.hpp, line 2547
> > 
> >
> > can you convert this into ACTIVE and INACTIVE states? if you want to do 
> > that later, please add a TODO (and maybe a ticket to track).

Created a separate JIRA for this, 
https://issues.apache.org/jira/browse/MESOS-6719


> On Dec. 2, 2016, 10:54 p.m., Vinod Kone wrote:
> > src/master/master.hpp, line 2528
> > 
> >
> > I would kill this helper in favor of checking state directly. The 
> > reason being, as we add more and more states having such helpers seems 
> > redundant.

Per discussion, I like the helpers for two reasons:

1. When we do MESOS-6719, whether a framework is "connected" might correspond 
to more than one `state` enumeration value.
2. Checking `if (framework->state == Framework::State::CONNECTED)` is a more 
verbose than checking `if (framework->connected())`


- Neil


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


On Nov. 18, 2016, 7:21 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53896/
> ---
> 
> (Updated Nov. 18, 2016, 7:21 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-6419
> https://issues.apache.org/jira/browse/MESOS-6419
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously, the master used a bool to track whether a given framework is
> connected. This commit adjusts the master to use an enum instead. The
> enum currently only has two values, CONNECTED and DISCONNECTED, but an
> additional value (RECOVERED) will be introduced shortly.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 90cbed1ba411e18906fe9c26bc14576a26d1b7b9 
>   src/master/master.hpp 7829f3f5f6125714b2fa48fe7c2813c26d14e26d 
>   src/master/master.cpp 7ed1d259f02991bcd1389d0529a4bc97b0aa0245 
>   src/master/quota_handler.cpp 5578663f26d9b737499dc4f5a286c34c37645442 
> 
> Diff: https://reviews.apache.org/r/53896/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 53896: Refactored representation of framework connectedness.

2016-12-02 Thread Vinod Kone 

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




src/master/master.hpp (line 2527)


I would kill this helper in favor of checking state directly. The reason 
being, as we add more and more states having such helpers seems redundant.



src/master/master.hpp (line 2545)


can you convert this into ACTIVE and INACTIVE states? if you want to do 
that later, please add a TODO (and maybe a ticket to track).



src/master/master.cpp (line 1956)


as noted above, just do

`framework->state != CONNECTED`

here and below.


- Vinod Kone


On Nov. 18, 2016, 7:21 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53896/
> ---
> 
> (Updated Nov. 18, 2016, 7:21 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-6419
> https://issues.apache.org/jira/browse/MESOS-6419
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously, the master used a bool to track whether a given framework is
> connected. This commit adjusts the master to use an enum instead. The
> enum currently only has two values, CONNECTED and DISCONNECTED, but an
> additional value (RECOVERED) will be introduced shortly.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 90cbed1ba411e18906fe9c26bc14576a26d1b7b9 
>   src/master/master.hpp 7829f3f5f6125714b2fa48fe7c2813c26d14e26d 
>   src/master/master.cpp 7ed1d259f02991bcd1389d0529a4bc97b0aa0245 
>   src/master/quota_handler.cpp 5578663f26d9b737499dc4f5a286c34c37645442 
> 
> Diff: https://reviews.apache.org/r/53896/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Neil Conway
> 
>