You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Quinn Leng <qu...@gmail.com> on 2017/07/18 00:18:38 UTC
Review Request 60929: Added 'FRAMEWORK_UPDATED' event for master
streaming event.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60929/
-----------------------------------------------------------
Review request for mesos, Anand Mazumdar, Greg Mann, Vinod Kone, and Zhitao Li.
Bugs: MESOS-6101
https://issues.apache.org/jira/browse/MESOS-6101
Repository: mesos
Description
-------
Added event 'FRAMEWORK_UPDATED' for master's v1 streaming operator API.
It will be generated when the framework re-registers with the master.
Diffs
-----
include/mesos/master/master.proto 7a722a6deac484f28a85f63fd68ef49cb6a47606
include/mesos/v1/master/master.proto cf88ea64fa08e4bdcf685de4def28d374eb32fdf
src/common/protobuf_utils.hpp 2156f6d158bfee16d29aa3531bdd156af581df01
src/common/protobuf_utils.cpp 4e5ab02c90ad8490efbcc471f7fd7c48be0c4678
src/master/master.cpp d895154c00eef17511c46ddd0b75922f952707eb
Diff: https://reviews.apache.org/r/60929/diff/1/
Testing
-------
make check
Thanks,
Quinn Leng
Re: Review Request 60929: Added 'FRAMEWORK_UPDATED' event for master
streaming api.
Posted by Quinn Leng <qu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60929/
-----------------------------------------------------------
(Updated July 27, 2017, 6:23 p.m.)
Review request for mesos, Anand Mazumdar, Greg Mann, and Zhitao Li.
Bugs: MESOS-6101
https://issues.apache.org/jira/browse/MESOS-6101
Repository: mesos
Description
-------
Added event 'FRAMEWORK_UPDATED' for master's v1 streaming operator API.
It will be generated when the framework re-registers with the master.
Diffs (updated)
-----
include/mesos/master/master.proto 7a722a6deac484f28a85f63fd68ef49cb6a47606
include/mesos/v1/master/master.proto cf88ea64fa08e4bdcf685de4def28d374eb32fdf
src/common/protobuf_utils.hpp 2156f6d158bfee16d29aa3531bdd156af581df01
src/common/protobuf_utils.cpp 4e5ab02c90ad8490efbcc471f7fd7c48be0c4678
src/master/master.cpp e12c997dad04f8a4ddb47a993a84b2b05c9e2f32
Diff: https://reviews.apache.org/r/60929/diff/6/
Changes: https://reviews.apache.org/r/60929/diff/5-6/
Testing
-------
make check
Thanks,
Quinn Leng
Re: Review Request 60929: Added 'FRAMEWORK_UPDATED' event for master
streaming api.
Posted by Anand Mazumdar <an...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60929/#review181248
-----------------------------------------------------------
Looks good for the most part; just some comments around better representing what an updated event means.
include/mesos/master/master.proto
Lines 521-524 (patched)
<https://reviews.apache.org/r/60929/#comment256775>
How about:
```cpp
// Forwarded by the master when a framework re-registers with the
// master upon a disconnection (network error) or upon a master failover.
```
src/common/protobuf_utils.cpp
Lines 853-867 (patched)
<https://reviews.apache.org/r/60929/#comment256773>
Similar comments as per earlier review.
src/master/master.cpp
Lines 2739-2742 (patched)
<https://reviews.apache.org/r/60929/#comment256772>
this should be done for both the code blocks. See my comment later.
src/master/master.cpp
Line 2754 (original), 2759 (patched)
<https://reviews.apache.org/r/60929/#comment256771>
The `FRAMEWORK_UPDATED` event should be sent whenever a framework re-registers. So, it should ideally be sent here i.e., for all instances when a framework is recovered or not.
src/master/master.cpp
Lines 3138-3141 (patched)
<https://reviews.apache.org/r/60929/#comment256774>
Similar comments around where this should be placed as earlier.
- Anand Mazumdar
On July 18, 2017, 12:18 a.m., Quinn Leng wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60929/
> -----------------------------------------------------------
>
> (Updated July 18, 2017, 12:18 a.m.)
>
>
> Review request for mesos, Anand Mazumdar, Greg Mann, and Zhitao Li.
>
>
> Bugs: MESOS-6101
> https://issues.apache.org/jira/browse/MESOS-6101
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added event 'FRAMEWORK_UPDATED' for master's v1 streaming operator API.
> It will be generated when the framework re-registers with the master.
>
>
> Diffs
> -----
>
> include/mesos/master/master.proto 7a722a6deac484f28a85f63fd68ef49cb6a47606
> include/mesos/v1/master/master.proto cf88ea64fa08e4bdcf685de4def28d374eb32fdf
> src/common/protobuf_utils.hpp 2156f6d158bfee16d29aa3531bdd156af581df01
> src/common/protobuf_utils.cpp 4e5ab02c90ad8490efbcc471f7fd7c48be0c4678
> src/master/master.cpp d895154c00eef17511c46ddd0b75922f952707eb
>
>
> Diff: https://reviews.apache.org/r/60929/diff/4/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Quinn Leng
>
>
Re: Review Request 60929: Added 'FRAMEWORK_UPDATED' event for master
streaming api.
Posted by Anand Mazumdar <an...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60929/#review181589
-----------------------------------------------------------
Fix it, then Ship it!
include/mesos/master/master.proto
Lines 522 (patched)
<https://reviews.apache.org/r/60929/#comment257183>
Missing dot at the end. :-)
include/mesos/v1/master/master.proto
Lines 522 (patched)
<https://reviews.apache.org/r/60929/#comment257181>
s/re-register/re-registers
src/master/master.cpp
Lines 2738 (patched)
<https://reviews.apache.org/r/60929/#comment257180>
Why newline here?
- Anand Mazumdar
On July 18, 2017, 12:18 a.m., Quinn Leng wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60929/
> -----------------------------------------------------------
>
> (Updated July 18, 2017, 12:18 a.m.)
>
>
> Review request for mesos, Anand Mazumdar, Greg Mann, and Zhitao Li.
>
>
> Bugs: MESOS-6101
> https://issues.apache.org/jira/browse/MESOS-6101
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added event 'FRAMEWORK_UPDATED' for master's v1 streaming operator API.
> It will be generated when the framework re-registers with the master.
>
>
> Diffs
> -----
>
> include/mesos/master/master.proto 7a722a6deac484f28a85f63fd68ef49cb6a47606
> include/mesos/v1/master/master.proto cf88ea64fa08e4bdcf685de4def28d374eb32fdf
> src/common/protobuf_utils.hpp 2156f6d158bfee16d29aa3531bdd156af581df01
> src/common/protobuf_utils.cpp 4e5ab02c90ad8490efbcc471f7fd7c48be0c4678
> src/master/master.cpp e12c997dad04f8a4ddb47a993a84b2b05c9e2f32
>
>
> Diff: https://reviews.apache.org/r/60929/diff/5/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Quinn Leng
>
>