You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Yong Qiao Wang <yq...@cn.ibm.com> on 2015/09/02 14:11:33 UTC
Review Request 38050: Master should send PingSlaveMessage instead of
"PING"; Slave should accept PingSlaveMessage but not "PING" message;
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38050/
-----------------------------------------------------------
Review request for mesos and Vinod Kone.
Bugs: MESOS-1831 and MESOS-1832
https://issues.apache.org/jira/browse/MESOS-1831
https://issues.apache.org/jira/browse/MESOS-1832
Repository: mesos
Description
-------
Master should send PingSlaveMessage instead of "PING"; Slave should accept PingSlaveMessage but not "PING" message;
Diffs
-----
src/master/master.cpp 56bcbcc08fa0f98416c5048080adb25efc588019
src/slave/slave.hpp 09172f7ed547049b3bd169b3db9be94e14f6bc39
src/slave/slave.cpp 5e5522e1254a5ed6084de36782753f9aad2894c4
Diff: https://reviews.apache.org/r/38050/diff/
Testing
-------
Thanks,
Yong Qiao Wang
Re: Review Request 38050: Master should send PingSlaveMessage instead
of
"PING"; Slave should accept PingSlaveMessage but not "PING" message;
Posted by Yong Qiao Wang <yq...@cn.ibm.com>.
> On 九月 2, 2015, 12:55 p.m., Klaus Ma wrote:
> >
Thanks @klaus Ma for your comments. I have addressed all of them.
- Yong Qiao
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38050/#review97434
-----------------------------------------------------------
On 九月 2, 2015, 2:16 p.m., Yong Qiao Wang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38050/
> -----------------------------------------------------------
>
> (Updated 九月 2, 2015, 2:16 p.m.)
>
>
> Review request for mesos and Vinod Kone.
>
>
> Bugs: MESOS-1831 and MESOS-1832
> https://issues.apache.org/jira/browse/MESOS-1831
> https://issues.apache.org/jira/browse/MESOS-1832
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Master should send PingSlaveMessage instead of "PING"; Slave should accept PingSlaveMessage but not "PING" message;
>
>
> Diffs
> -----
>
> src/master/master.cpp 56bcbcc08fa0f98416c5048080adb25efc588019
> src/slave/slave.hpp 09172f7ed547049b3bd169b3db9be94e14f6bc39
> src/slave/slave.cpp 5e5522e1254a5ed6084de36782753f9aad2894c4
> src/tests/partition_tests.cpp b7030adcfb1bfb128040aff20f12461412b5382c
> src/tests/slave_recovery_tests.cpp 4d137e0f1278fdacf71f101b1967df35bfbcdd23
> src/tests/slave_tests.cpp 24119183ef04961fe2fdac73de7672cbe5b2408c
>
> Diff: https://reviews.apache.org/r/38050/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Yong Qiao Wang
>
>
Re: Review Request 38050: Master should send PingSlaveMessage instead
of
"PING"; Slave should accept PingSlaveMessage but not "PING" message;
Posted by Klaus Ma <kl...@cguru.net>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38050/#review97434
-----------------------------------------------------------
src/master/master.cpp
<https://reviews.apache.org/r/38050/#comment153311>
The indent seems incorrect, here's the suggestion:
install<PongSlaveMessage>(
&SlaveObserver::pong);
src/slave/slave.cpp
<https://reviews.apache.org/r/38050/#comment153312>
In slave's UT case, here're at least two cases about PING/PONG package: PingTimeoutSomePings & PingTimeoutNoPings; are those UT cases impacted? If so please help to update accordingly; and please also help to check the other UT cases about PING/PONG message, I'd not go through all slave UT cases.
- Klaus Ma
On Sept. 2, 2015, 12:11 p.m., Yong Qiao Wang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38050/
> -----------------------------------------------------------
>
> (Updated Sept. 2, 2015, 12:11 p.m.)
>
>
> Review request for mesos and Vinod Kone.
>
>
> Bugs: MESOS-1831 and MESOS-1832
> https://issues.apache.org/jira/browse/MESOS-1831
> https://issues.apache.org/jira/browse/MESOS-1832
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Master should send PingSlaveMessage instead of "PING"; Slave should accept PingSlaveMessage but not "PING" message;
>
>
> Diffs
> -----
>
> src/master/master.cpp 56bcbcc08fa0f98416c5048080adb25efc588019
> src/slave/slave.hpp 09172f7ed547049b3bd169b3db9be94e14f6bc39
> src/slave/slave.cpp 5e5522e1254a5ed6084de36782753f9aad2894c4
>
> Diff: https://reviews.apache.org/r/38050/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Yong Qiao Wang
>
>
Re: Review Request 38050: Master should send PingSlaveMessage instead
of
"PING"; Slave should accept PingSlaveMessage but not "PING" message;
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38050/#review97621
-----------------------------------------------------------
Patch looks great!
Reviews applied: [38050]
All tests passed.
- Mesos ReviewBot
On Sept. 3, 2015, 7:46 a.m., Yong Qiao Wang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38050/
> -----------------------------------------------------------
>
> (Updated Sept. 3, 2015, 7:46 a.m.)
>
>
> Review request for mesos and Vinod Kone.
>
>
> Bugs: MESOS-1831
> https://issues.apache.org/jira/browse/MESOS-1831
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Master should send PingSlaveMessage instead of "PING"; Slave should accept PingSlaveMessage but not "PING" message;
>
>
> Diffs
> -----
>
> src/master/master.cpp 56bcbcc08fa0f98416c5048080adb25efc588019
> src/tests/partition_tests.cpp b7030adcfb1bfb128040aff20f12461412b5382c
> src/tests/slave_recovery_tests.cpp 4d137e0f1278fdacf71f101b1967df35bfbcdd23
> src/tests/slave_tests.cpp 24119183ef04961fe2fdac73de7672cbe5b2408c
>
> Diff: https://reviews.apache.org/r/38050/diff/
>
>
> Testing
> -------
>
> Test2: Start master and slave to debug the PingSlaveMessage. Passed!
> Test1: Run make check successfully!
>
>
> Thanks,
>
> Yong Qiao Wang
>
>
Re: Review Request 38050: Master should send PingSlaveMessage instead
of
"PING"; Slave should accept PingSlaveMessage but not "PING" message;
Posted by Yong Qiao Wang <yq...@cn.ibm.com>.
> On 九月 3, 2015, 3:45 p.m., Vinod Kone wrote:
> > src/tests/partition_tests.cpp, line 79
> > <https://reviews.apache.org/r/38050/diff/3/?file=1063131#file1063131line79>
> >
> > why the change to process::Message here and everywhere else?
Actually, namespace "using process::Message;" has been used in the header of this file, so it does not need to change to process::Message. I will update to roolback these changes.
> On 九月 3, 2015, 3:45 p.m., Vinod Kone wrote:
> > src/master/master.cpp, lines 187-195
> > <https://reviews.apache.org/r/38050/diff/3/?file=1063130#file1063130line187>
> >
> > any reason to not just call pong() here?
Update the pongOld function to just call pong().
- Yong Qiao
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38050/#review97641
-----------------------------------------------------------
On 九月 3, 2015, 7:46 a.m., Yong Qiao Wang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38050/
> -----------------------------------------------------------
>
> (Updated 九月 3, 2015, 7:46 a.m.)
>
>
> Review request for mesos and Vinod Kone.
>
>
> Bugs: MESOS-1831
> https://issues.apache.org/jira/browse/MESOS-1831
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Master should send PingSlaveMessage instead of "PING"; Slave should accept PingSlaveMessage but not "PING" message;
>
>
> Diffs
> -----
>
> src/master/master.cpp 56bcbcc08fa0f98416c5048080adb25efc588019
> src/tests/partition_tests.cpp b7030adcfb1bfb128040aff20f12461412b5382c
> src/tests/slave_recovery_tests.cpp 4d137e0f1278fdacf71f101b1967df35bfbcdd23
> src/tests/slave_tests.cpp 24119183ef04961fe2fdac73de7672cbe5b2408c
>
> Diff: https://reviews.apache.org/r/38050/diff/
>
>
> Testing
> -------
>
> Test2: Start master and slave to debug the PingSlaveMessage. Passed!
> Test1: Run make check successfully!
>
>
> Thanks,
>
> Yong Qiao Wang
>
>
Re: Review Request 38050: Master should send PingSlaveMessage instead
of
"PING"; Slave should accept PingSlaveMessage but not "PING" message;
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38050/#review97641
-----------------------------------------------------------
looks good overall. some minor issues.
src/master/master.cpp (line 146)
<https://reviews.apache.org/r/38050/#comment153650>
"Remove this handler in 0.26.0".
src/master/master.cpp (lines 149 - 150)
<https://reviews.apache.org/r/38050/#comment153648>
this fits on one line (within 80 chars)? why wrap it?
src/master/master.cpp (lines 181 - 189)
<https://reviews.apache.org/r/38050/#comment153652>
any reason to not just call pong() here?
src/tests/partition_tests.cpp (line 79)
<https://reviews.apache.org/r/38050/#comment153654>
why the change to process::Message here and everywhere else?
- Vinod Kone
On Sept. 3, 2015, 7:46 a.m., Yong Qiao Wang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38050/
> -----------------------------------------------------------
>
> (Updated Sept. 3, 2015, 7:46 a.m.)
>
>
> Review request for mesos and Vinod Kone.
>
>
> Bugs: MESOS-1831
> https://issues.apache.org/jira/browse/MESOS-1831
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Master should send PingSlaveMessage instead of "PING"; Slave should accept PingSlaveMessage but not "PING" message;
>
>
> Diffs
> -----
>
> src/master/master.cpp 56bcbcc08fa0f98416c5048080adb25efc588019
> src/tests/partition_tests.cpp b7030adcfb1bfb128040aff20f12461412b5382c
> src/tests/slave_recovery_tests.cpp 4d137e0f1278fdacf71f101b1967df35bfbcdd23
> src/tests/slave_tests.cpp 24119183ef04961fe2fdac73de7672cbe5b2408c
>
> Diff: https://reviews.apache.org/r/38050/diff/
>
>
> Testing
> -------
>
> Test2: Start master and slave to debug the PingSlaveMessage. Passed!
> Test1: Run make check successfully!
>
>
> Thanks,
>
> Yong Qiao Wang
>
>
Re: Review Request 38050: Master should send PingSlaveMessage instead
of
"PING"; Slave should accept PingSlaveMessage but not "PING" message;
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38050/#review97709
-----------------------------------------------------------
Ship it!
Ship It!
- Vinod Kone
On Sept. 4, 2015, 1:18 a.m., Yong Qiao Wang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38050/
> -----------------------------------------------------------
>
> (Updated Sept. 4, 2015, 1:18 a.m.)
>
>
> Review request for mesos and Vinod Kone.
>
>
> Bugs: MESOS-1831
> https://issues.apache.org/jira/browse/MESOS-1831
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Master should send PingSlaveMessage instead of "PING"; Slave should accept PingSlaveMessage but not "PING" message;
>
>
> Diffs
> -----
>
> src/master/master.cpp 56bcbcc
> src/tests/partition_tests.cpp b7030ad
> src/tests/slave_recovery_tests.cpp 4d137e0
> src/tests/slave_tests.cpp 2411918
>
> Diff: https://reviews.apache.org/r/38050/diff/
>
>
> Testing
> -------
>
> Test2: Start master and slave to debug the PingSlaveMessage. Passed!
> Test1: Run make check successfully!
>
>
> Thanks,
>
> Yong Qiao Wang
>
>
Re: Review Request 38050: Master should send PingSlaveMessage instead
of
"PING"; Slave should accept PingSlaveMessage but not "PING" message;
Posted by Yong Qiao Wang <yq...@cn.ibm.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38050/
-----------------------------------------------------------
(Updated 九月 4, 2015, 1:18 a.m.)
Review request for mesos and Vinod Kone.
Bugs: MESOS-1831
https://issues.apache.org/jira/browse/MESOS-1831
Repository: mesos
Description
-------
Master should send PingSlaveMessage instead of "PING"; Slave should accept PingSlaveMessage but not "PING" message;
Diffs (updated)
-----
src/master/master.cpp 56bcbcc
src/tests/partition_tests.cpp b7030ad
src/tests/slave_recovery_tests.cpp 4d137e0
src/tests/slave_tests.cpp 2411918
Diff: https://reviews.apache.org/r/38050/diff/
Testing
-------
Test2: Start master and slave to debug the PingSlaveMessage. Passed!
Test1: Run make check successfully!
Thanks,
Yong Qiao Wang
Re: Review Request 38050: Master should send PingSlaveMessage instead
of
"PING"; Slave should accept PingSlaveMessage but not "PING" message;
Posted by Yong Qiao Wang <yq...@cn.ibm.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38050/
-----------------------------------------------------------
(Updated 九月 4, 2015, 1:04 a.m.)
Review request for mesos and Vinod Kone.
Bugs: MESOS-1831
https://issues.apache.org/jira/browse/MESOS-1831
Repository: mesos
Description
-------
Master should send PingSlaveMessage instead of "PING"; Slave should accept PingSlaveMessage but not "PING" message;
Diffs (updated)
-----
3rdparty/libprocess/include/process/collect.hpp d07b6860a5b5cbed910b2e9e2ab2429bd70a3999
3rdparty/libprocess/src/tests/process_tests.cpp 435663b10c1bfce07e8e84719aa14b5867c651c6
src/Makefile.am 4b643a32eb68dbd52714e9d66451dc2b62576f6d
src/linux/perf.hpp d10968ca670eb516bae08385fd0ddde8e8ad83b5
src/linux/perf.cpp 0011482cf9d920485728798518d32af0e9627724
src/master/master.cpp 56bcbcc08fa0f98416c5048080adb25efc588019
src/slave/containerizer/isolators/cgroups/perf_event.hpp 1f722ef3ef7ab7fce5542d4affae961d6cec2406
src/slave/containerizer/isolators/cgroups/perf_event.cpp 7dc8b7a99074b74ade019ef4df296780650a2e4e
src/slave/containerizer/provisioners/backend.cpp 6560ece14d8618878a35d1bfe27db3958da64358
src/slave/containerizer/provisioners/backends/copy.hpp 2abca37ed2479d42c634c23cac8e40d515249988
src/slave/containerizer/provisioners/backends/copy.cpp b56946562525e79bef3a7387cd71f39fd0690683
src/tests/containerizer/perf_tests.cpp bef475e4b573f02b69f2c4dd6d1c6a6e10f34c38
src/tests/containerizer/provisioner_backend_tests.cpp f2498b109c910fbf753a53b4b36a88b8d779aa69
src/tests/partition_tests.cpp b7030adcfb1bfb128040aff20f12461412b5382c
src/tests/slave_recovery_tests.cpp 4d137e0f1278fdacf71f101b1967df35bfbcdd23
src/tests/slave_tests.cpp 24119183ef04961fe2fdac73de7672cbe5b2408c
Diff: https://reviews.apache.org/r/38050/diff/
Testing
-------
Test2: Start master and slave to debug the PingSlaveMessage. Passed!
Test1: Run make check successfully!
Thanks,
Yong Qiao Wang
Re: Review Request 38050: Master should send PingSlaveMessage instead
of
"PING"; Slave should accept PingSlaveMessage but not "PING" message;
Posted by Yong Qiao Wang <yq...@cn.ibm.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38050/
-----------------------------------------------------------
(Updated 九月 3, 2015, 7:46 a.m.)
Review request for mesos and Vinod Kone.
Bugs: MESOS-1831
https://issues.apache.org/jira/browse/MESOS-1831
Repository: mesos
Description
-------
Master should send PingSlaveMessage instead of "PING"; Slave should accept PingSlaveMessage but not "PING" message;
Diffs
-----
src/master/master.cpp 56bcbcc08fa0f98416c5048080adb25efc588019
src/tests/partition_tests.cpp b7030adcfb1bfb128040aff20f12461412b5382c
src/tests/slave_recovery_tests.cpp 4d137e0f1278fdacf71f101b1967df35bfbcdd23
src/tests/slave_tests.cpp 24119183ef04961fe2fdac73de7672cbe5b2408c
Diff: https://reviews.apache.org/r/38050/diff/
Testing
-------
Test2: Start master and slave to debug the PingSlaveMessage. Passed!
Test1: Run make check successfully!
Thanks,
Yong Qiao Wang
Re: Review Request 38050: Master should send PingSlaveMessage instead
of
"PING"; Slave should accept PingSlaveMessage but not "PING" message;
Posted by Yong Qiao Wang <yq...@cn.ibm.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38050/
-----------------------------------------------------------
(Updated 九月 3, 2015, 7:45 a.m.)
Review request for mesos and Vinod Kone.
Bugs: MESOS-1831 and MESOS-1832
https://issues.apache.org/jira/browse/MESOS-1831
https://issues.apache.org/jira/browse/MESOS-1832
Repository: mesos
Description
-------
Master should send PingSlaveMessage instead of "PING"; Slave should accept PingSlaveMessage but not "PING" message;
Diffs (updated)
-----
src/master/master.cpp 56bcbcc08fa0f98416c5048080adb25efc588019
src/tests/partition_tests.cpp b7030adcfb1bfb128040aff20f12461412b5382c
src/tests/slave_recovery_tests.cpp 4d137e0f1278fdacf71f101b1967df35bfbcdd23
src/tests/slave_tests.cpp 24119183ef04961fe2fdac73de7672cbe5b2408c
Diff: https://reviews.apache.org/r/38050/diff/
Testing (updated)
-------
Test2: Start master and slave to debug the PingSlaveMessage. Passed!
Test1: Run make check successfully!
Thanks,
Yong Qiao Wang
Re: Review Request 38050: Master should send PingSlaveMessage instead
of
"PING"; Slave should accept PingSlaveMessage but not "PING" message;
Posted by Yong Qiao Wang <yq...@cn.ibm.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38050/
-----------------------------------------------------------
(Updated 九月 2, 2015, 2:16 p.m.)
Review request for mesos and Vinod Kone.
Bugs: MESOS-1831 and MESOS-1832
https://issues.apache.org/jira/browse/MESOS-1831
https://issues.apache.org/jira/browse/MESOS-1832
Repository: mesos
Description
-------
Master should send PingSlaveMessage instead of "PING"; Slave should accept PingSlaveMessage but not "PING" message;
Diffs (updated)
-----
src/master/master.cpp 56bcbcc08fa0f98416c5048080adb25efc588019
src/slave/slave.hpp 09172f7ed547049b3bd169b3db9be94e14f6bc39
src/slave/slave.cpp 5e5522e1254a5ed6084de36782753f9aad2894c4
src/tests/partition_tests.cpp b7030adcfb1bfb128040aff20f12461412b5382c
src/tests/slave_recovery_tests.cpp 4d137e0f1278fdacf71f101b1967df35bfbcdd23
src/tests/slave_tests.cpp 24119183ef04961fe2fdac73de7672cbe5b2408c
Diff: https://reviews.apache.org/r/38050/diff/
Testing
-------
Thanks,
Yong Qiao Wang