Home | About | Sematext search-lucene.com search-hadoop.com
 Search Hadoop and all its subprojects:

Switch to Threaded View
Zookeeper >> mail # dev >> Re: Review Request: Add zk.updateServerList(newServerList)


Copy link to this message
-
Re: Review Request: Add zk.updateServerList(newServerList)


> On Oct. 15, 2012, 11:46 p.m., michim wrote:
> > /src/c/Makefile.am, line 75
> > <https://reviews.apache.org/r/3781/diff/4/?file=176404#file176404line75>
> >
> >     nitpick: a trailing space

Fixed.
> On Oct. 15, 2012, 11:46 p.m., michim wrote:
> > /src/c/src/zookeeper.c, line 435
> > <https://reviews.apache.org/r/3781/diff/4/?file=176409#file176409line435>
> >
> >     Do we need to call both srandom and srand48?

No, definitely not. Removed call to srand48. Nice catch.
> On Oct. 15, 2012, 11:46 p.m., michim wrote:
> > /src/c/src/zookeeper.c, line 1105
> > <https://reviews.apache.org/r/3781/diff/4/?file=176409#file176409line1105>
> >
> >     typo: st to false -> set to false

Fixed.
> On Oct. 15, 2012, 11:46 p.m., michim wrote:
> > /src/c/src/zookeeper.c, line 1572
> > <https://reviews.apache.org/r/3781/diff/4/?file=176409#file176409line1572>
> >
> >     Is there a tab character here?

Yes, and there were a few others in the file that I'd incorrectly added (I don't know why my company uses tabs, but I often forget to switch settings when I go work on zookeeper!).

Fixed.
> On Oct. 15, 2012, 11:46 p.m., michim wrote:
> > /src/c/src/zookeeper.c, line 1925
> > <https://reviews.apache.org/r/3781/diff/4/?file=176409#file176409line1925>
> >
> >     Was the number 60 chosen arbitrarily? What happens if zh->recv_timeout is less than 60?

Somewhat arbitrarily. It is designed just to prevent *thrashing* so we don't try to reconnect too quickly in the event that we couldn't connect to *any* of the servers in the ensemble.

recv_timeout is measured in milliseconds, so having a value less than 60 seems unrealistic to me.

If you have a better idea here, please let me know :).
> On Oct. 15, 2012, 11:46 p.m., michim wrote:
> > /src/docs/src/documentation/content/xdocs/zookeeperProgrammers.xml, line 561
> > <https://reviews.apache.org/r/3781/diff/4/?file=176413#file176413line561>
> >
> >     typo: and not jus -> and not just

fixed.
- Marshall
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3781/#review12460
-----------------------------------------------------------
On Oct. 13, 2012, 7:56 a.m., Alexander Shraer wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/3781/
> -----------------------------------------------------------
>
> (Updated Oct. 13, 2012, 7:56 a.m.)
>
>
> Review request for zookeeper.
>
>
> Description
> -------
>
> https://issues.apache.org/jira/browse/ZOOKEEPER-1355
>
>
> Diffs
> -----
>
>   /src/c/Makefile.am 1397800
>   /src/c/include/zookeeper.h 1397800
>   /src/c/src/mt_adaptor.c 1397800
>   /src/c/src/st_adaptor.c 1397800
>   /src/c/src/zk_adaptor.h 1397800
>   /src/c/src/zookeeper.c 1397800
>   /src/c/tests/TestZookeeperClose.cc 1397800
>   /src/c/tests/TestZookeeperInit.cc 1397800
>   /src/c/tests/ZKMocks.cc 1397800
>   /src/docs/src/documentation/content/xdocs/zookeeperProgrammers.xml 1397800
>   /src/java/main/org/apache/zookeeper/ZooKeeper.java 1397800
>   /src/java/main/org/apache/zookeeper/client/HostProvider.java 1397800
>   /src/java/main/org/apache/zookeeper/client/StaticHostProvider.java 1397800
>   /src/java/test/org/apache/zookeeper/test/StaticHostProviderTest.java 1397800
>
> Diff: https://reviews.apache.org/r/3781/diff/
>
>
> Testing
> -------
>
> new tests included as part of the patch
>
>
> Thanks,
>
> Alexander Shraer
>
>