> 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
> 
>

Reply via email to