You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Joris Van Remoortere <jo...@gmail.com> on 2015/08/10 09:05:52 UTC

Re: Review Request 37180: Maintenance Primitives: Implemented Master::inverseOffer.

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

(Updated Aug. 10, 2015, 7:05 a.m.)


Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Joseph Wu.


Summary (updated)
-----------------

Maintenance Primitives: Implemented Master::inverseOffer.


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


Repository: mesos


Description
-------

See summary.


Diffs (updated)
-----

  src/master/master.hpp 28356e4ca24312b8be0138a34805b3d9035a99a3 
  src/master/master.cpp 08dd34d9d18f547c6e8d04caf9e39a2b3ffc5f63 
  src/messages/messages.proto 8977d8e0f3b16003128b6b9cab556a7b224f083c 

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


Testing
-------

The tests break as expected.
With the scheduler API change there are CHECKs that fail.
Once we update the API these will be resolved.


Thanks,

Joris Van Remoortere

Re: Review Request 37180: Maintenance Primitives: Implemented Master::inverseOffer.

Posted by Joris Van Remoortere <jo...@gmail.com>.

> On Aug. 12, 2015, 9:16 p.m., Joseph Wu wrote:
> > Why are the InverseOffers (and Offers) hashed as pointers?

Can you point to an example. I don't understand your question.


- Joris


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


On Aug. 18, 2015, 6:58 p.m., Joris Van Remoortere wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37180/
> -----------------------------------------------------------
> 
> (Updated Aug. 18, 2015, 6:58 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Joseph Wu.
> 
> 
> Bugs: MESOS-1474
>     https://issues.apache.org/jira/browse/MESOS-1474
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   src/master/master.hpp 0432842d77beba024c7895291ca410964bae96be 
>   src/master/master.cpp 95207d24db0aa052eb70c4cc7eb75d0611c365cf 
> 
> Diff: https://reviews.apache.org/r/37180/diff/
> 
> 
> Testing
> -------
> 
> The tests break as expected.
> With the scheduler API change there are CHECKs that fail.
> Once we update the API these will be resolved.
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>

Re: Review Request 37180: Maintenance Primitives: Implemented Master::inverseOffer.

Posted by Joseph Wu <jo...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37180/#review95172
-----------------------------------------------------------


Why are the InverseOffers (and Offers) hashed as pointers?


src/master/master.cpp (lines 4795 - 4796)
<https://reviews.apache.org/r/37180/#comment150011>

    This second sentence might be better placed in the InverseOffer protobuf.



src/master/master.cpp (line 5667)
<https://reviews.apache.org/r/37180/#comment150014>

    `callled`


- Joseph Wu


On Aug. 10, 2015, 12:05 a.m., Joris Van Remoortere wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37180/
> -----------------------------------------------------------
> 
> (Updated Aug. 10, 2015, 12:05 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Joseph Wu.
> 
> 
> Bugs: MESOS-1474
>     https://issues.apache.org/jira/browse/MESOS-1474
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   src/master/master.hpp 28356e4ca24312b8be0138a34805b3d9035a99a3 
>   src/master/master.cpp 08dd34d9d18f547c6e8d04caf9e39a2b3ffc5f63 
>   src/messages/messages.proto 8977d8e0f3b16003128b6b9cab556a7b224f083c 
> 
> Diff: https://reviews.apache.org/r/37180/diff/
> 
> 
> Testing
> -------
> 
> The tests break as expected.
> With the scheduler API change there are CHECKs that fail.
> Once we update the API these will be resolved.
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>

Re: Review Request 37180: Maintenance Primitives: Implemented Master::inverseOffer.

Posted by Joseph Wu <jo...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37180/#review95786
-----------------------------------------------------------

Ship it!


LGTM.


src/master/master.cpp (line 5745)
<https://reviews.apache.org/r/37180/#comment150914>

    Typo still present.


- Joseph Wu


On Aug. 18, 2015, 11:58 a.m., Joris Van Remoortere wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37180/
> -----------------------------------------------------------
> 
> (Updated Aug. 18, 2015, 11:58 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Joseph Wu.
> 
> 
> Bugs: MESOS-1474
>     https://issues.apache.org/jira/browse/MESOS-1474
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   src/master/master.hpp 0432842d77beba024c7895291ca410964bae96be 
>   src/master/master.cpp 95207d24db0aa052eb70c4cc7eb75d0611c365cf 
> 
> Diff: https://reviews.apache.org/r/37180/diff/
> 
> 
> Testing
> -------
> 
> The tests break as expected.
> With the scheduler API change there are CHECKs that fail.
> Once we update the API these will be resolved.
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>

Re: Review Request 37180: Maintenance Primitives: Implemented Master::inverseOffer.

Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37180/#review96921
-----------------------------------------------------------

Ship it!



src/master/master.cpp (line 4144)
<https://reviews.apache.org/r/37180/#comment152613>

    NOTE:



src/master/master.cpp (line 4828)
<https://reviews.apache.org/r/37180/#comment152614>

    Indentation looks off here.


- Benjamin Hindman


On Aug. 26, 2015, 2:13 a.m., Joris Van Remoortere wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37180/
> -----------------------------------------------------------
> 
> (Updated Aug. 26, 2015, 2:13 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Joseph Wu.
> 
> 
> Bugs: MESOS-1474
>     https://issues.apache.org/jira/browse/MESOS-1474
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   src/master/master.hpp 36c67599ef2c470da8d95f2caf926a154342d2cc 
>   src/master/master.cpp 95207d24db0aa052eb70c4cc7eb75d0611c365cf 
> 
> Diff: https://reviews.apache.org/r/37180/diff/
> 
> 
> Testing
> -------
> 
> The tests break as expected.
> With the scheduler API change there are CHECKs that fail.
> Once we update the API these will be resolved.
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>

Re: Review Request 37180: Maintenance Primitives: Implemented Master::inverseOffer.

Posted by Joris Van Remoortere <jo...@gmail.com>.

> On Aug. 31, 2015, 7:49 a.m., Jian Qiu wrote:
> > src/master/master.cpp, line 4904
> > <https://reviews.apache.org/r/37180/diff/6/?file=1052944#file1052944line4904>
> >
> >     If the allocator calls inverse callback followed by an offer callback, will the two messages be handled in two separate threads of the framework? I think it will be framework's responsibility to ensure data synchronization between the two threads?

This is up to how the framework driver is implemented. We have a reference implementation, but with the V1 API there will be many implementations.
The current C++ scheduler driver handles 1 message type at a time, so they will be synchronized. If the framework decides to do asynchronous calls during those callback invocations, then it is up to the framework writer to synchronize them.
Does this make sense?
Since I'm closing this review feel free to reach out to me via e-mail to continue this discussion!


- Joris


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


On Sept. 13, 2015, 8:33 p.m., Joris Van Remoortere wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37180/
> -----------------------------------------------------------
> 
> (Updated Sept. 13, 2015, 8:33 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Joseph Wu.
> 
> 
> Bugs: MESOS-1474
>     https://issues.apache.org/jira/browse/MESOS-1474
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   src/master/master.hpp 12cc1ad45de3291ec22d4fe2b7ee11c4d7565c24 
>   src/master/master.cpp c90311fa2152810e7604a0a2dee630bd14929574 
> 
> Diff: https://reviews.apache.org/r/37180/diff/
> 
> 
> Testing
> -------
> 
> The tests break as expected.
> With the scheduler API change there are CHECKs that fail.
> Once we update the API these will be resolved.
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>

Re: Review Request 37180: Maintenance Primitives: Implemented Master::inverseOffer.

Posted by Jian Qiu <qi...@cn.ibm.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37180/#review97064
-----------------------------------------------------------



src/master/master.cpp (line 4904)
<https://reviews.apache.org/r/37180/#comment152785>

    If the allocator calls inverse callback followed by an offer callback, will the two messages be handled in two separate threads of the framework? I think it will be framework's responsibility to ensure data synchronization between the two threads?


- Jian Qiu


On 八月 26, 2015, 2:13 a.m., Joris Van Remoortere wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37180/
> -----------------------------------------------------------
> 
> (Updated 八月 26, 2015, 2:13 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Joseph Wu.
> 
> 
> Bugs: MESOS-1474
>     https://issues.apache.org/jira/browse/MESOS-1474
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   src/master/master.hpp 36c67599ef2c470da8d95f2caf926a154342d2cc 
>   src/master/master.cpp 95207d24db0aa052eb70c4cc7eb75d0611c365cf 
> 
> Diff: https://reviews.apache.org/r/37180/diff/
> 
> 
> Testing
> -------
> 
> The tests break as expected.
> With the scheduler API change there are CHECKs that fail.
> Once we update the API these will be resolved.
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>

Re: Review Request 37180: Maintenance Primitives: Implemented Master::inverseOffer.

Posted by Yong Qiao Wang <yq...@cn.ibm.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37180/#review98187
-----------------------------------------------------------

Ship it!


Ship It!

- Yong Qiao Wang


On Sept. 2, 2015, 7:32 p.m., Joris Van Remoortere wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37180/
> -----------------------------------------------------------
> 
> (Updated Sept. 2, 2015, 7:32 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Joseph Wu.
> 
> 
> Bugs: MESOS-1474
>     https://issues.apache.org/jira/browse/MESOS-1474
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   src/master/master.hpp 594dd25f9aa9b6147680d0a838a77c3222941f4b 
>   src/master/master.cpp 56bcbcc08fa0f98416c5048080adb25efc588019 
> 
> Diff: https://reviews.apache.org/r/37180/diff/
> 
> 
> Testing
> -------
> 
> The tests break as expected.
> With the scheduler API change there are CHECKs that fail.
> Once we update the API these will be resolved.
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>

Re: Review Request 37180: Maintenance Primitives: Implemented Master::inverseOffer.

Posted by Qian Zhang <zh...@cn.ibm.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37180/#review98845
-----------------------------------------------------------



src/master/master.cpp (lines 4906 - 4907)
<https://reviews.apache.org/r/37180/#comment155463>

    Framework scheduler's resourceOffers() callback will be invoked to handle the "offers" in ResourceOffersMessage, but here in the ResourceOffersMessage, we only set "pids" and "inverse_offers", but not "offers". So that means framework scheduler will receive an empty offers? This seems strange, maybe it is something that we need to fix in MESOS-2063 (Add InverseOffer to C++ Scheduler API)?


- Qian Zhang


On Sept. 14, 2015, 4:33 a.m., Joris Van Remoortere wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37180/
> -----------------------------------------------------------
> 
> (Updated Sept. 14, 2015, 4:33 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Joseph Wu.
> 
> 
> Bugs: MESOS-1474
>     https://issues.apache.org/jira/browse/MESOS-1474
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   src/master/master.hpp 12cc1ad45de3291ec22d4fe2b7ee11c4d7565c24 
>   src/master/master.cpp c90311fa2152810e7604a0a2dee630bd14929574 
> 
> Diff: https://reviews.apache.org/r/37180/diff/
> 
> 
> Testing
> -------
> 
> The tests break as expected.
> With the scheduler API change there are CHECKs that fail.
> Once we update the API these will be resolved.
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>

Re: Review Request 37180: Maintenance Primitives: Implemented Master::inverseOffer.

Posted by Joris Van Remoortere <jo...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37180/
-----------------------------------------------------------

(Updated Sept. 13, 2015, 8:33 p.m.)


Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Joseph Wu.


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


Repository: mesos


Description
-------

See summary.


Diffs (updated)
-----

  src/master/master.hpp 12cc1ad45de3291ec22d4fe2b7ee11c4d7565c24 
  src/master/master.cpp c90311fa2152810e7604a0a2dee630bd14929574 

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


Testing
-------

The tests break as expected.
With the scheduler API change there are CHECKs that fail.
Once we update the API these will be resolved.


Thanks,

Joris Van Remoortere

Re: Review Request 37180: Maintenance Primitives: Implemented Master::inverseOffer.

Posted by Joris Van Remoortere <jo...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37180/
-----------------------------------------------------------

(Updated Sept. 2, 2015, 7:32 p.m.)


Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Joseph Wu.


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


Repository: mesos


Description
-------

See summary.


Diffs (updated)
-----

  src/master/master.hpp 594dd25f9aa9b6147680d0a838a77c3222941f4b 
  src/master/master.cpp 56bcbcc08fa0f98416c5048080adb25efc588019 

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


Testing
-------

The tests break as expected.
With the scheduler API change there are CHECKs that fail.
Once we update the API these will be resolved.


Thanks,

Joris Van Remoortere

Re: Review Request 37180: Maintenance Primitives: Implemented Master::inverseOffer.

Posted by Joris Van Remoortere <jo...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37180/
-----------------------------------------------------------

(Updated Aug. 26, 2015, 2:13 a.m.)


Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Joseph Wu.


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


Repository: mesos


Description
-------

See summary.


Diffs (updated)
-----

  src/master/master.hpp 36c67599ef2c470da8d95f2caf926a154342d2cc 
  src/master/master.cpp 95207d24db0aa052eb70c4cc7eb75d0611c365cf 

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


Testing
-------

The tests break as expected.
With the scheduler API change there are CHECKs that fail.
Once we update the API these will be resolved.


Thanks,

Joris Van Remoortere

Re: Review Request 37180: Maintenance Primitives: Implemented Master::inverseOffer.

Posted by Guangya Liu <gy...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37180/#review96393
-----------------------------------------------------------

Ship it!


Ship It!

- Guangya Liu


On Aug. 25, 2015, 2:13 a.m., Joris Van Remoortere wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37180/
> -----------------------------------------------------------
> 
> (Updated Aug. 25, 2015, 2:13 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Joseph Wu.
> 
> 
> Bugs: MESOS-1474
>     https://issues.apache.org/jira/browse/MESOS-1474
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   src/master/master.hpp 0432842d77beba024c7895291ca410964bae96be 
>   src/master/master.cpp 95207d24db0aa052eb70c4cc7eb75d0611c365cf 
> 
> Diff: https://reviews.apache.org/r/37180/diff/
> 
> 
> Testing
> -------
> 
> The tests break as expected.
> With the scheduler API change there are CHECKs that fail.
> Once we update the API these will be resolved.
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>

Re: Review Request 37180: Maintenance Primitives: Implemented Master::inverseOffer.

Posted by Joris Van Remoortere <jo...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37180/
-----------------------------------------------------------

(Updated Aug. 25, 2015, 2:13 a.m.)


Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Joseph Wu.


Changes
-------

Rebased.


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


Repository: mesos


Description
-------

See summary.


Diffs (updated)
-----

  src/master/master.hpp 0432842d77beba024c7895291ca410964bae96be 
  src/master/master.cpp 95207d24db0aa052eb70c4cc7eb75d0611c365cf 

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


Testing
-------

The tests break as expected.
With the scheduler API change there are CHECKs that fail.
Once we update the API these will be resolved.


Thanks,

Joris Van Remoortere

Re: Review Request 37180: Maintenance Primitives: Implemented Master::inverseOffer.

Posted by Guangya Liu <gy...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37180/#review95803
-----------------------------------------------------------



src/master/master.cpp (line 4145)
<https://reviews.apache.org/r/37180/#comment150947>

    I think the blank line can be removed



src/master/master.cpp (line 4841)
<https://reviews.apache.org/r/37180/#comment150946>

    I think the blank line can be removed


- Guangya Liu


On 八月 18, 2015, 6:58 p.m., Joris Van Remoortere wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37180/
> -----------------------------------------------------------
> 
> (Updated 八月 18, 2015, 6:58 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Joseph Wu.
> 
> 
> Bugs: MESOS-1474
>     https://issues.apache.org/jira/browse/MESOS-1474
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   src/master/master.hpp 0432842d77beba024c7895291ca410964bae96be 
>   src/master/master.cpp 95207d24db0aa052eb70c4cc7eb75d0611c365cf 
> 
> Diff: https://reviews.apache.org/r/37180/diff/
> 
> 
> Testing
> -------
> 
> The tests break as expected.
> With the scheduler API change there are CHECKs that fail.
> Once we update the API these will be resolved.
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>

Re: Review Request 37180: Maintenance Primitives: Implemented Master::inverseOffer.

Posted by Joris Van Remoortere <jo...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37180/
-----------------------------------------------------------

(Updated Aug. 18, 2015, 6:58 p.m.)


Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Joseph Wu.


Changes
-------

Rebased.


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


Repository: mesos


Description
-------

See summary.


Diffs (updated)
-----

  src/master/master.hpp 0432842d77beba024c7895291ca410964bae96be 
  src/master/master.cpp 95207d24db0aa052eb70c4cc7eb75d0611c365cf 

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


Testing
-------

The tests break as expected.
With the scheduler API change there are CHECKs that fail.
Once we update the API these will be resolved.


Thanks,

Joris Van Remoortere