You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Chris Chen <cc...@nougat.org> on 2015/09/28 20:17:32 UTC

Review Request 38815: Add Zookeeper patch from ZOOKEEPER-2253

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

Review request for mesos.


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


Repository: mesos


Description
-------

Problem:
The Zookeeeper C client does makes certain assertions about the ordering
of ping packets that the Java client does not. An alternate implementation
of the Zookeeper server would then break the C client while working
correctly with the Java client.

Solution:
Port change from ZOOKEEPER-2253 to make C client handle pings similarly
to the Java client.

Result:
C client and Java clients behave the same when dealing with an out-of-order
ping packet.


Diffs
-----

  3rdparty/Makefile.am 7c244a053044ae347f9505cb78a71a6636e26586 
  3rdparty/zookeeper-3.4.5.patch 3ca180d0c81f5de521ada7fb6c1c248a871ab2da 

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


Testing
-------

Tested from upstream; Running mesos check suite currently.


Thanks,

Chris Chen


Re: Review Request 38815: Add Zookeeper patch from ZOOKEEPER-2253

Posted by Chris Chen <cc...@nougat.org>.

> On Sept. 28, 2015, 8:43 p.m., Alex Clemmer wrote:
> > Can you add this comment to the CMake build system as well?
> > 
> > The file you need to change is here: https://github.com/apache/mesos/blob/master/3rdparty/CMakeLists.txt Should be line 40, above `PATCH_CMD`.

I don't see that file...


- Chris


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


On Sept. 28, 2015, 6:17 p.m., Chris Chen wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38815/
> -----------------------------------------------------------
> 
> (Updated Sept. 28, 2015, 6:17 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Bugs: MESOS-3391
>     https://issues.apache.org/jira/browse/MESOS-3391
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Problem:
> The Zookeeeper C client does makes certain assertions about the ordering
> of ping packets that the Java client does not. An alternate implementation
> of the Zookeeper server would then break the C client while working
> correctly with the Java client.
> 
> Solution:
> Port change from ZOOKEEPER-2253 to make C client handle pings similarly
> to the Java client.
> 
> Result:
> C client and Java clients behave the same when dealing with an out-of-order
> ping packet.
> 
> 
> Diffs
> -----
> 
>   3rdparty/Makefile.am 7c244a053044ae347f9505cb78a71a6636e26586 
>   3rdparty/zookeeper-3.4.5.patch 3ca180d0c81f5de521ada7fb6c1c248a871ab2da 
> 
> Diff: https://reviews.apache.org/r/38815/diff/
> 
> 
> Testing
> -------
> 
> Tested from upstream; Running mesos check suite currently.
> 
> 
> Thanks,
> 
> Chris Chen
> 
>


Re: Review Request 38815: Add Zookeeper patch from ZOOKEEPER-2253

Posted by Alex Clemmer <cl...@gmail.com>.

> On Sept. 28, 2015, 8:43 p.m., Alex Clemmer wrote:
> > Can you add this comment to the CMake build system as well?
> > 
> > The file you need to change is here: https://github.com/apache/mesos/blob/master/3rdparty/CMakeLists.txt Should be line 40, above `PATCH_CMD`.
> 
> Chris Chen wrote:
>     I don't see that file...

Hmm, I'm not 100% sure what you mean. Do you mean that you don't see it in apache/master? This file was checked in yesterday (along with ~20 other reviews relevant to the CMake stuff), perhaps it is the case that you haven't pulled since then?


- Alex


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


On Sept. 28, 2015, 6:17 p.m., Chris Chen wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38815/
> -----------------------------------------------------------
> 
> (Updated Sept. 28, 2015, 6:17 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Bugs: MESOS-3391
>     https://issues.apache.org/jira/browse/MESOS-3391
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Problem:
> The Zookeeeper C client does makes certain assertions about the ordering
> of ping packets that the Java client does not. An alternate implementation
> of the Zookeeper server would then break the C client while working
> correctly with the Java client.
> 
> Solution:
> Port change from ZOOKEEPER-2253 to make C client handle pings similarly
> to the Java client.
> 
> Result:
> C client and Java clients behave the same when dealing with an out-of-order
> ping packet.
> 
> 
> Diffs
> -----
> 
>   3rdparty/Makefile.am 7c244a053044ae347f9505cb78a71a6636e26586 
>   3rdparty/zookeeper-3.4.5.patch 3ca180d0c81f5de521ada7fb6c1c248a871ab2da 
> 
> Diff: https://reviews.apache.org/r/38815/diff/
> 
> 
> Testing
> -------
> 
> Tested from upstream; Running mesos check suite currently.
> 
> 
> Thanks,
> 
> Chris Chen
> 
>


Re: Review Request 38815: Add Zookeeper patch from ZOOKEEPER-2253

Posted by Chris Chen <cc...@nougat.org>.

> On Sept. 28, 2015, 8:43 p.m., Alex Clemmer wrote:
> > Can you add this comment to the CMake build system as well?
> > 
> > The file you need to change is here: https://github.com/apache/mesos/blob/master/3rdparty/CMakeLists.txt Should be line 40, above `PATCH_CMD`.
> 
> Chris Chen wrote:
>     I don't see that file...
> 
> Alex Clemmer wrote:
>     Hmm, I'm not 100% sure what you mean. Do you mean that you don't see it in apache/master? This file was checked in yesterday (along with ~20 other reviews relevant to the CMake stuff), perhaps it is the case that you haven't pulled since then?

Lovely. My remote was set to the github copy. Reset origin to apache.org and I see it. Thx.


- Chris


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


On Sept. 28, 2015, 6:17 p.m., Chris Chen wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38815/
> -----------------------------------------------------------
> 
> (Updated Sept. 28, 2015, 6:17 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Bugs: MESOS-3391
>     https://issues.apache.org/jira/browse/MESOS-3391
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Problem:
> The Zookeeeper C client does makes certain assertions about the ordering
> of ping packets that the Java client does not. An alternate implementation
> of the Zookeeper server would then break the C client while working
> correctly with the Java client.
> 
> Solution:
> Port change from ZOOKEEPER-2253 to make C client handle pings similarly
> to the Java client.
> 
> Result:
> C client and Java clients behave the same when dealing with an out-of-order
> ping packet.
> 
> 
> Diffs
> -----
> 
>   3rdparty/Makefile.am 7c244a053044ae347f9505cb78a71a6636e26586 
>   3rdparty/zookeeper-3.4.5.patch 3ca180d0c81f5de521ada7fb6c1c248a871ab2da 
> 
> Diff: https://reviews.apache.org/r/38815/diff/
> 
> 
> Testing
> -------
> 
> Tested from upstream; Running mesos check suite currently.
> 
> 
> Thanks,
> 
> Chris Chen
> 
>


Re: Review Request 38815: Add Zookeeper patch from ZOOKEEPER-2253

Posted by Alex Clemmer <cl...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38815/#review100860
-----------------------------------------------------------


Can you add this comment to the CMake build system as well?

The file you need to change is here: https://github.com/apache/mesos/blob/master/3rdparty/CMakeLists.txt Should be line 40, above `PATCH_CMD`.

- Alex Clemmer


On Sept. 28, 2015, 6:17 p.m., Chris Chen wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38815/
> -----------------------------------------------------------
> 
> (Updated Sept. 28, 2015, 6:17 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Bugs: MESOS-3391
>     https://issues.apache.org/jira/browse/MESOS-3391
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Problem:
> The Zookeeeper C client does makes certain assertions about the ordering
> of ping packets that the Java client does not. An alternate implementation
> of the Zookeeper server would then break the C client while working
> correctly with the Java client.
> 
> Solution:
> Port change from ZOOKEEPER-2253 to make C client handle pings similarly
> to the Java client.
> 
> Result:
> C client and Java clients behave the same when dealing with an out-of-order
> ping packet.
> 
> 
> Diffs
> -----
> 
>   3rdparty/Makefile.am 7c244a053044ae347f9505cb78a71a6636e26586 
>   3rdparty/zookeeper-3.4.5.patch 3ca180d0c81f5de521ada7fb6c1c248a871ab2da 
> 
> Diff: https://reviews.apache.org/r/38815/diff/
> 
> 
> Testing
> -------
> 
> Tested from upstream; Running mesos check suite currently.
> 
> 
> Thanks,
> 
> Chris Chen
> 
>


Re: Review Request 38815: Add Zookeeper patch from ZOOKEEPER-2253

Posted by Chris Chen <cc...@nougat.org>.

> On Sept. 28, 2015, 11:18 p.m., Mesos ReviewBot wrote:
> > Bad review!
> > 
> > Reviews applied: []
> > 
> > Error:
> >  No reviewers specified. Please find a reviewer by asking on JIRA or the mailing list.

FINE.


- Chris


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


On Sept. 28, 2015, 11:24 p.m., Chris Chen wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38815/
> -----------------------------------------------------------
> 
> (Updated Sept. 28, 2015, 11:24 p.m.)
> 
> 
> Review request for mesos and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-3391
>     https://issues.apache.org/jira/browse/MESOS-3391
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Problem:
> The Zookeeeper C client does makes certain assertions about the ordering
> of ping packets that the Java client does not. An alternate implementation
> of the Zookeeper server would then break the C client while working
> correctly with the Java client.
> 
> Solution:
> Port change from ZOOKEEPER-2253 to make C client handle pings similarly
> to the Java client.
> 
> Result:
> C client and Java clients behave the same when dealing with an out-of-order
> ping packet.
> 
> 
> Diffs
> -----
> 
>   3rdparty/Makefile.am 7c244a053044ae347f9505cb78a71a6636e26586 
>   3rdparty/zookeeper-3.4.5.patch 3ca180d0c81f5de521ada7fb6c1c248a871ab2da 
> 
> Diff: https://reviews.apache.org/r/38815/diff/
> 
> 
> Testing
> -------
> 
> Tested from upstream; Running mesos check succeeds.
> 
> 
> Thanks,
> 
> Chris Chen
> 
>


Re: Review Request 38815: Add Zookeeper patch from ZOOKEEPER-2253

Posted by Jiang Yan Xu <ya...@jxu.me>.

> On Sept. 28, 2015, 4:18 p.m., Mesos ReviewBot wrote:
> > Bad review!
> > 
> > Reviews applied: []
> > 
> > Error:
> >  No reviewers specified. Please find a reviewer by asking on JIRA or the mailing list.
> 
> Chris Chen wrote:
>     FINE.

Doesn't look like the change to 3rdparty/CMakeLists.txt is attached?


- Jiang Yan


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


On Sept. 28, 2015, 4:24 p.m., Chris Chen wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38815/
> -----------------------------------------------------------
> 
> (Updated Sept. 28, 2015, 4:24 p.m.)
> 
> 
> Review request for mesos and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-3391
>     https://issues.apache.org/jira/browse/MESOS-3391
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Problem:
> The Zookeeeper C client does makes certain assertions about the ordering
> of ping packets that the Java client does not. An alternate implementation
> of the Zookeeper server would then break the C client while working
> correctly with the Java client.
> 
> Solution:
> Port change from ZOOKEEPER-2253 to make C client handle pings similarly
> to the Java client.
> 
> Result:
> C client and Java clients behave the same when dealing with an out-of-order
> ping packet.
> 
> 
> Diffs
> -----
> 
>   3rdparty/Makefile.am 7c244a053044ae347f9505cb78a71a6636e26586 
>   3rdparty/zookeeper-3.4.5.patch 3ca180d0c81f5de521ada7fb6c1c248a871ab2da 
> 
> Diff: https://reviews.apache.org/r/38815/diff/
> 
> 
> Testing
> -------
> 
> Tested from upstream; Running mesos check succeeds.
> 
> 
> Thanks,
> 
> Chris Chen
> 
>


Re: Review Request 38815: Add Zookeeper patch from ZOOKEEPER-2253

Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38815/#review100888
-----------------------------------------------------------


Bad review!

Reviews applied: []

Error:
 No reviewers specified. Please find a reviewer by asking on JIRA or the mailing list.

- Mesos ReviewBot


On Sept. 28, 2015, 8:59 p.m., Chris Chen wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38815/
> -----------------------------------------------------------
> 
> (Updated Sept. 28, 2015, 8:59 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Bugs: MESOS-3391
>     https://issues.apache.org/jira/browse/MESOS-3391
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Problem:
> The Zookeeeper C client does makes certain assertions about the ordering
> of ping packets that the Java client does not. An alternate implementation
> of the Zookeeper server would then break the C client while working
> correctly with the Java client.
> 
> Solution:
> Port change from ZOOKEEPER-2253 to make C client handle pings similarly
> to the Java client.
> 
> Result:
> C client and Java clients behave the same when dealing with an out-of-order
> ping packet.
> 
> 
> Diffs
> -----
> 
>   3rdparty/Makefile.am 7c244a053044ae347f9505cb78a71a6636e26586 
>   3rdparty/zookeeper-3.4.5.patch 3ca180d0c81f5de521ada7fb6c1c248a871ab2da 
> 
> Diff: https://reviews.apache.org/r/38815/diff/
> 
> 
> Testing
> -------
> 
> Tested from upstream; Running mesos check succeeds.
> 
> 
> Thanks,
> 
> Chris Chen
> 
>


Re: Review Request 38815: Add Zookeeper patch from ZOOKEEPER-2253

Posted by Chris Chen <cc...@nougat.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38815/
-----------------------------------------------------------

(Updated Sept. 28, 2015, 8:59 p.m.)


Review request for mesos.


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


Repository: mesos


Description
-------

Problem:
The Zookeeeper C client does makes certain assertions about the ordering
of ping packets that the Java client does not. An alternate implementation
of the Zookeeper server would then break the C client while working
correctly with the Java client.

Solution:
Port change from ZOOKEEPER-2253 to make C client handle pings similarly
to the Java client.

Result:
C client and Java clients behave the same when dealing with an out-of-order
ping packet.


Diffs (updated)
-----

  3rdparty/Makefile.am 7c244a053044ae347f9505cb78a71a6636e26586 
  3rdparty/zookeeper-3.4.5.patch 3ca180d0c81f5de521ada7fb6c1c248a871ab2da 

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


Testing (updated)
-------

Tested from upstream; Running mesos check succeeds.


Thanks,

Chris Chen


Re: Review Request 38815: Add Zookeeper patch from ZOOKEEPER-2253

Posted by Jiang Yan Xu <ya...@jxu.me>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38815/#review100851
-----------------------------------------------------------

Ship it!


LGTM. Will commit it after reviewbot gives it a pass.

- Jiang Yan Xu


On Sept. 28, 2015, 11:17 a.m., Chris Chen wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38815/
> -----------------------------------------------------------
> 
> (Updated Sept. 28, 2015, 11:17 a.m.)
> 
> 
> Review request for mesos.
> 
> 
> Bugs: MESOS-3391
>     https://issues.apache.org/jira/browse/MESOS-3391
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Problem:
> The Zookeeeper C client does makes certain assertions about the ordering
> of ping packets that the Java client does not. An alternate implementation
> of the Zookeeper server would then break the C client while working
> correctly with the Java client.
> 
> Solution:
> Port change from ZOOKEEPER-2253 to make C client handle pings similarly
> to the Java client.
> 
> Result:
> C client and Java clients behave the same when dealing with an out-of-order
> ping packet.
> 
> 
> Diffs
> -----
> 
>   3rdparty/Makefile.am 7c244a053044ae347f9505cb78a71a6636e26586 
>   3rdparty/zookeeper-3.4.5.patch 3ca180d0c81f5de521ada7fb6c1c248a871ab2da 
> 
> Diff: https://reviews.apache.org/r/38815/diff/
> 
> 
> Testing
> -------
> 
> Tested from upstream; Running mesos check suite currently.
> 
> 
> Thanks,
> 
> Chris Chen
> 
>