You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@zookeeper.apache.org by Rakesh R <ra...@huawei.com> on 2015/05/06 11:22:49 UTC
Re: Review Request 33713: ZOOKEEPER-2163 - Complete implementation
and doc
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33713/#review82648
-----------------------------------------------------------
CHANGES.txt
<https://reviews.apache.org/r/33713/#comment133418>
This is not required, please revert this changes
bin/zkServer.sh
<https://reviews.apache.org/r/33713/#comment133419>
This is not required, please revert this changes
src/java/main/org/apache/zookeeper/ZooKeeper.java
<https://reviews.apache.org/r/33713/#comment133438>
Minor suggestion:
I could see many places the import statements has been modified to .*
I prefer not to change the existing imports it may show many changes in diff that may deviate reviewers focus. We can do import the necessary classess/items required for the implementation.
src/java/main/org/apache/zookeeper/ZooKeeper.java
<https://reviews.apache.org/r/33713/#comment133420>
I assume you are referring to jute.maxbuffer. It is not good to hard code value 1MB, right?
src/java/main/org/apache/zookeeper/ZooKeeper.java
<https://reviews.apache.org/r/33713/#comment133437>
One general thought:
Instead of providing a new API for creating the container, can we think of "CreateMode.CONTAINER" a new znode type? Also, I feel this may help us to support multiTxn/batch apis.
I haven't done detailed analysis, this is just a rough thought that comes in my mind.
src/java/main/org/apache/zookeeper/ZooKeeper.java
<https://reviews.apache.org/r/33713/#comment133421>
I assume you are referring to jute.maxbuffer. It is not good to hard code value 1MB, right?
src/java/main/org/apache/zookeeper/server/ContainerManager.java
<https://reviews.apache.org/r/33713/#comment133422>
Please replace System.currentMillis() with org.apache.zookeeper.common.Time.currentElapsedTime()
src/java/main/org/apache/zookeeper/server/ContainerManager.java
<https://reviews.apache.org/r/33713/#comment133423>
Please use {} instead of + for string concatenation in logs
src/java/main/org/apache/zookeeper/server/ContainerManager.java
<https://reviews.apache.org/r/33713/#comment133424>
Please use {} instead of + for string concatenation in logs
src/java/main/org/apache/zookeeper/server/ContainerManager.java
<https://reviews.apache.org/r/33713/#comment133425>
Can we modify System.currentTimeMillis() using org.apache.zookeeper.common.Time ?
src/java/main/org/apache/zookeeper/server/ContainerManager.java
<https://reviews.apache.org/r/33713/#comment133426>
Please format this line, its more than >80 chars
src/java/main/org/apache/zookeeper/server/ContainerManager.java
<https://reviews.apache.org/r/33713/#comment133428>
Could you please tell me the reson for the checks
node.stat.getCversion() > 0 ?
Probably you can add a comment over there, so that others also will understand.
src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java
<https://reviews.apache.org/r/33713/#comment133431>
Please modify string concatenation in logs using {}
src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java
<https://reviews.apache.org/r/33713/#comment133432>
Could you tell me the behaviour if there occurs NoNodeException?
src/java/test/org/apache/zookeeper/server/CreateContainerTest.java
<https://reviews.apache.org/r/33713/#comment133434>
Please add timeout args in tests like,
@Test(timeout=60000)
src/java/test/org/apache/zookeeper/server/CreateContainerTest.java
<https://reviews.apache.org/r/33713/#comment133435>
Can we think of some better logic instead of Thread.sleep(1000) in tests. It may be flaky in some env, right?
src/java/test/org/apache/zookeeper/server/CreateContainerTest.java
<https://reviews.apache.org/r/33713/#comment133436>
Can we think of some better logic instead of Thread.sleep(1000) in tests. It may be flaky in some env, right?
I could see there are few more occurances of Thread.sleep(xx) similar to this, please do the changes. Thanks!
- Rakesh R
On April 30, 2015, 4:08 p.m., Jordan Zimmerman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33713/
> -----------------------------------------------------------
>
> (Updated April 30, 2015, 4:08 p.m.)
>
>
> Review request for zookeeper, michim, Rakesh R, and Raul Gutierrez Segales.
>
>
> Bugs: ZOOKEEPER-2163
> https://issues.apache.org/jira/browse/ZOOKEEPER-2163
>
>
> Repository: zookeeper-git
>
>
> Description
> -------
>
> Introduce new ZNode type: container
>
>
> Diffs
> -----
>
> CHANGES.txt 51ec65d
> bin/zkServer.sh dae3ce2
> src/docs/src/documentation/content/xdocs/zookeeperAdmin.xml c12c2ca
> src/docs/src/documentation/content/xdocs/zookeeperProgrammers.xml 223cf8e
> src/java/main/org/apache/zookeeper/ZooDefs.java a4fc331
> src/java/main/org/apache/zookeeper/ZooKeeper.java dd8ecf4
> src/java/main/org/apache/zookeeper/server/ContainerManager.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/DataNode.java b341a69
> src/java/main/org/apache/zookeeper/server/DataTree.java 78cddb1
> src/java/main/org/apache/zookeeper/server/FinalRequestProcessor.java 7e3c29f
> src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java 4911acf
> src/java/main/org/apache/zookeeper/server/Request.java bed9b13
> src/java/main/org/apache/zookeeper/server/TraceFormatter.java 582383d
> src/java/main/org/apache/zookeeper/server/ZooKeeperServerMain.java 63daea0
> src/java/main/org/apache/zookeeper/server/quorum/CommitProcessor.java cf0900b
> src/java/main/org/apache/zookeeper/server/quorum/FollowerRequestProcessor.java 4d061f4
> src/java/main/org/apache/zookeeper/server/quorum/LeaderZooKeeperServer.java 6434d02
> src/java/main/org/apache/zookeeper/server/quorum/ObserverRequestProcessor.java 36a23ee
> src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyRequestProcessor.java a49319c
> src/java/main/org/apache/zookeeper/server/util/SerializeUtils.java 1a45c5e
> src/java/test/org/apache/zookeeper/server/CreateContainerTest.java PRE-CREATION
> src/zookeeper.jute 709e935
>
> Diff: https://reviews.apache.org/r/33713/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Jordan Zimmerman
>
>
Re: Review Request 33713: ZOOKEEPER-2163 - Complete implementation
and doc
Posted by Jordan Zimmerman <jo...@jordanzimmerman.com>.
> On May 6, 2015, 9:22 a.m., Rakesh R wrote:
> > src/java/main/org/apache/zookeeper/ZooKeeper.java, line 1404
> > <https://reviews.apache.org/r/33713/diff/1/?file=946048#file946048line1404>
> >
> > One general thought:
> >
> > Instead of providing a new API for creating the container, can we think of "CreateMode.CONTAINER" a new znode type? Also, I feel this may help us to support multiTxn/batch apis.
> >
> > I haven't done detailed analysis, this is just a rough thought that comes in my mind.
At this point, I don't remember why I chose to add a new API. Other committers haven't asked for this. But, if you like I can change it a new CreateMode. Supporting multi/trans is a good point. Please advise.
> On May 6, 2015, 9:22 a.m., Rakesh R wrote:
> > src/java/main/org/apache/zookeeper/server/ContainerManager.java, line 132
> > <https://reviews.apache.org/r/33713/diff/1/?file=946049#file946049line132>
> >
> > Could you please tell me the reson for the checks
> > node.stat.getCversion() > 0 ?
> >
> > Probably you can add a comment over there, so that others also will understand.
It's to keep newly created containers from being deleted before any children have been added. If you were to create the container just before a container cleaning period the container would be immediately be deleted. However, we could change to only deleting a container once it's n ticks old. Thoughts?
- Jordan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33713/#review82648
-----------------------------------------------------------
On April 30, 2015, 4:08 p.m., Jordan Zimmerman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33713/
> -----------------------------------------------------------
>
> (Updated April 30, 2015, 4:08 p.m.)
>
>
> Review request for zookeeper, michim, Rakesh R, and Raul Gutierrez Segales.
>
>
> Bugs: ZOOKEEPER-2163
> https://issues.apache.org/jira/browse/ZOOKEEPER-2163
>
>
> Repository: zookeeper-git
>
>
> Description
> -------
>
> Introduce new ZNode type: container
>
>
> Diffs
> -----
>
> CHANGES.txt 51ec65d
> bin/zkServer.sh dae3ce2
> src/docs/src/documentation/content/xdocs/zookeeperAdmin.xml c12c2ca
> src/docs/src/documentation/content/xdocs/zookeeperProgrammers.xml 223cf8e
> src/java/main/org/apache/zookeeper/ZooDefs.java a4fc331
> src/java/main/org/apache/zookeeper/ZooKeeper.java dd8ecf4
> src/java/main/org/apache/zookeeper/server/ContainerManager.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/DataNode.java b341a69
> src/java/main/org/apache/zookeeper/server/DataTree.java 78cddb1
> src/java/main/org/apache/zookeeper/server/FinalRequestProcessor.java 7e3c29f
> src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java 4911acf
> src/java/main/org/apache/zookeeper/server/Request.java bed9b13
> src/java/main/org/apache/zookeeper/server/TraceFormatter.java 582383d
> src/java/main/org/apache/zookeeper/server/ZooKeeperServerMain.java 63daea0
> src/java/main/org/apache/zookeeper/server/quorum/CommitProcessor.java cf0900b
> src/java/main/org/apache/zookeeper/server/quorum/FollowerRequestProcessor.java 4d061f4
> src/java/main/org/apache/zookeeper/server/quorum/LeaderZooKeeperServer.java 6434d02
> src/java/main/org/apache/zookeeper/server/quorum/ObserverRequestProcessor.java 36a23ee
> src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyRequestProcessor.java a49319c
> src/java/main/org/apache/zookeeper/server/util/SerializeUtils.java 1a45c5e
> src/java/test/org/apache/zookeeper/server/CreateContainerTest.java PRE-CREATION
> src/zookeeper.jute 709e935
>
> Diff: https://reviews.apache.org/r/33713/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Jordan Zimmerman
>
>
Re: Review Request 33713: ZOOKEEPER-2163 - Complete implementation
and doc
Posted by Jordan Zimmerman <jo...@jordanzimmerman.com>.
> On May 6, 2015, 9:22 a.m., Rakesh R wrote:
> > src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java, line 497
> > <https://reviews.apache.org/r/33713/diff/1/?file=946053#file946053line497>
> >
> > Could you tell me the behaviour if there occurs NoNodeException?
This was copied from OpCode.create2 and OpCode.create. Do I need to do something different?
- Jordan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33713/#review82648
-----------------------------------------------------------
On May 7, 2015, 5:56 p.m., Jordan Zimmerman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33713/
> -----------------------------------------------------------
>
> (Updated May 7, 2015, 5:56 p.m.)
>
>
> Review request for zookeeper, michim, Rakesh R, and Raul Gutierrez Segales.
>
>
> Bugs: ZOOKEEPER-2163
> https://issues.apache.org/jira/browse/ZOOKEEPER-2163
>
>
> Repository: zookeeper-git
>
>
> Description
> -------
>
> Introduce new ZNode type: container
>
>
> Diffs
> -----
>
> src/docs/src/documentation/content/xdocs/zookeeperAdmin.xml c12c2ca
> src/docs/src/documentation/content/xdocs/zookeeperProgrammers.xml 223cf8e
> src/java/main/org/apache/zookeeper/CreateMode.java d87f410
> src/java/main/org/apache/zookeeper/MultiTransactionRecord.java ea913b4
> src/java/main/org/apache/zookeeper/Op.java 97d3d7b
> src/java/main/org/apache/zookeeper/ZooDefs.java a4fc331
> src/java/main/org/apache/zookeeper/ZooKeeper.java dd8ecf4
> src/java/main/org/apache/zookeeper/cli/CreateCommand.java c6de7c6
> src/java/main/org/apache/zookeeper/server/ContainerManager.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/DataNode.java b341a69
> src/java/main/org/apache/zookeeper/server/DataTree.java 78cddb1
> src/java/main/org/apache/zookeeper/server/FinalRequestProcessor.java 7e3c29f
> src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java 4911acf
> src/java/main/org/apache/zookeeper/server/Request.java bed9b13
> src/java/main/org/apache/zookeeper/server/TraceFormatter.java 582383d
> src/java/main/org/apache/zookeeper/server/ZooKeeperServerMain.java 63daea0
> src/java/main/org/apache/zookeeper/server/quorum/CommitProcessor.java cf0900b
> src/java/main/org/apache/zookeeper/server/quorum/FollowerRequestProcessor.java 4d061f4
> src/java/main/org/apache/zookeeper/server/quorum/LeaderZooKeeperServer.java 6434d02
> src/java/main/org/apache/zookeeper/server/quorum/ObserverRequestProcessor.java 36a23ee
> src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyRequestProcessor.java a49319c
> src/java/main/org/apache/zookeeper/server/util/SerializeUtils.java 1a45c5e
> src/java/test/org/apache/zookeeper/server/CreateContainerTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/test/CreateModeTest.java 9db01bb
> src/zookeeper.jute 709e935
>
> Diff: https://reviews.apache.org/r/33713/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Jordan Zimmerman
>
>
Re: Review Request 33713: ZOOKEEPER-2163 - Complete implementation
and doc
Posted by Rakesh R <ra...@huawei.com>.
> On May 6, 2015, 9:22 a.m., Rakesh R wrote:
> > src/java/main/org/apache/zookeeper/server/ContainerManager.java, line 132
> > <https://reviews.apache.org/r/33713/diff/1/?file=946049#file946049line132>
> >
> > Could you please tell me the reson for the checks
> > node.stat.getCversion() > 0 ?
> >
> > Probably you can add a comment over there, so that others also will understand.
>
> Jordan Zimmerman wrote:
> It's to keep newly created containers from being deleted before any children have been added. If you were to create the container just before a container cleaning period the container would be immediately be deleted. However, we could change to only deleting a container once it's n ticks old. Thoughts?
OK. I got it. If you get a chance to add one more patch, it would great to add java comments in the logic saying these details, so that others also will understand it clearly.
- Rakesh
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33713/#review82648
-----------------------------------------------------------
On June 1, 2015, 6:17 p.m., Jordan Zimmerman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33713/
> -----------------------------------------------------------
>
> (Updated June 1, 2015, 6:17 p.m.)
>
>
> Review request for zookeeper, michim, Rakesh R, and Raul Gutierrez Segales.
>
>
> Bugs: ZOOKEEPER-2163
> https://issues.apache.org/jira/browse/ZOOKEEPER-2163
>
>
> Repository: zookeeper-git
>
>
> Description
> -------
>
> Introduce new ZNode type: container
>
>
> Diffs
> -----
>
> src/docs/src/documentation/content/xdocs/zookeeperAdmin.xml 5401157
> src/docs/src/documentation/content/xdocs/zookeeperProgrammers.xml 218baf3
> src/java/main/org/apache/zookeeper/CreateMode.java d87f410
> src/java/main/org/apache/zookeeper/MultiTransactionRecord.java ca7dd98
> src/java/main/org/apache/zookeeper/Op.java 97d3d7b
> src/java/main/org/apache/zookeeper/ZooDefs.java a4fc331
> src/java/main/org/apache/zookeeper/ZooKeeper.java fdee4e6
> src/java/main/org/apache/zookeeper/cli/CreateCommand.java c6de7c6
> src/java/main/org/apache/zookeeper/server/ContainerManager.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/DataNode.java b341a69
> src/java/main/org/apache/zookeeper/server/DataTree.java 78cddb1
> src/java/main/org/apache/zookeeper/server/FinalRequestProcessor.java 7e3c29f
> src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java 0e8133e
> src/java/main/org/apache/zookeeper/server/Request.java bed9b13
> src/java/main/org/apache/zookeeper/server/TraceFormatter.java 582383d
> src/java/main/org/apache/zookeeper/server/ZooKeeperServerMain.java 63daea0
> src/java/main/org/apache/zookeeper/server/quorum/CommitProcessor.java cf0900b
> src/java/main/org/apache/zookeeper/server/quorum/FollowerRequestProcessor.java 4d061f4
> src/java/main/org/apache/zookeeper/server/quorum/LeaderZooKeeperServer.java 6434d02
> src/java/main/org/apache/zookeeper/server/quorum/ObserverRequestProcessor.java 36a23ee
> src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyRequestProcessor.java a49319c
> src/java/main/org/apache/zookeeper/server/util/SerializeUtils.java 1a45c5e
> src/java/test/org/apache/zookeeper/server/CreateContainerTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/test/CreateModeTest.java 9db01bb
> src/zookeeper.jute 921f658
>
> Diff: https://reviews.apache.org/r/33713/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Jordan Zimmerman
>
>
Re: Review Request 33713: ZOOKEEPER-2163 - Complete implementation
and doc
Posted by Jordan Zimmerman <jo...@jordanzimmerman.com>.
> On May 6, 2015, 9:22 a.m., Rakesh R wrote:
> > src/java/test/org/apache/zookeeper/server/CreateContainerTest.java, line 90
> > <https://reviews.apache.org/r/33713/diff/1/?file=946063#file946063line90>
> >
> > Can we think of some better logic instead of Thread.sleep(1000) in tests. It may be flaky in some env, right?
I have no other idea. A record is added into ZK's processor and there are no testing hooks for that. I need to wait for it to get processed. If you have a suggestion, please let me know. NOTE: ZK's own tests have these timers.
> On May 6, 2015, 9:22 a.m., Rakesh R wrote:
> > src/java/test/org/apache/zookeeper/server/CreateContainerTest.java, line 131
> > <https://reviews.apache.org/r/33713/diff/1/?file=946063#file946063line131>
> >
> > Can we think of some better logic instead of Thread.sleep(1000) in tests. It may be flaky in some env, right?
> >
> > I could see there are few more occurances of Thread.sleep(xx) similar to this, please do the changes. Thanks!
I have no other idea. A record is added into ZK's processor and there are no testing hooks for that. I need to wait for it to get processed. If you have a suggestion, please let me know. NOTE: ZK's own tests have these timers.
- Jordan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33713/#review82648
-----------------------------------------------------------
On May 7, 2015, 11:33 p.m., Jordan Zimmerman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33713/
> -----------------------------------------------------------
>
> (Updated May 7, 2015, 11:33 p.m.)
>
>
> Review request for zookeeper, michim, Rakesh R, and Raul Gutierrez Segales.
>
>
> Bugs: ZOOKEEPER-2163
> https://issues.apache.org/jira/browse/ZOOKEEPER-2163
>
>
> Repository: zookeeper-git
>
>
> Description
> -------
>
> Introduce new ZNode type: container
>
>
> Diffs
> -----
>
> src/docs/src/documentation/content/xdocs/zookeeperAdmin.xml c12c2ca
> src/docs/src/documentation/content/xdocs/zookeeperProgrammers.xml 223cf8e
> src/java/main/org/apache/zookeeper/CreateMode.java d87f410
> src/java/main/org/apache/zookeeper/MultiTransactionRecord.java ea913b4
> src/java/main/org/apache/zookeeper/Op.java 97d3d7b
> src/java/main/org/apache/zookeeper/ZooDefs.java a4fc331
> src/java/main/org/apache/zookeeper/ZooKeeper.java dd8ecf4
> src/java/main/org/apache/zookeeper/cli/CreateCommand.java c6de7c6
> src/java/main/org/apache/zookeeper/server/ContainerManager.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/DataNode.java b341a69
> src/java/main/org/apache/zookeeper/server/DataTree.java 78cddb1
> src/java/main/org/apache/zookeeper/server/FinalRequestProcessor.java 7e3c29f
> src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java 4911acf
> src/java/main/org/apache/zookeeper/server/Request.java bed9b13
> src/java/main/org/apache/zookeeper/server/TraceFormatter.java 582383d
> src/java/main/org/apache/zookeeper/server/ZooKeeperServerMain.java 63daea0
> src/java/main/org/apache/zookeeper/server/quorum/CommitProcessor.java cf0900b
> src/java/main/org/apache/zookeeper/server/quorum/FollowerRequestProcessor.java 4d061f4
> src/java/main/org/apache/zookeeper/server/quorum/LeaderZooKeeperServer.java 6434d02
> src/java/main/org/apache/zookeeper/server/quorum/ObserverRequestProcessor.java 36a23ee
> src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyRequestProcessor.java a49319c
> src/java/main/org/apache/zookeeper/server/util/SerializeUtils.java 1a45c5e
> src/java/test/org/apache/zookeeper/server/CreateContainerTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/test/CreateModeTest.java 9db01bb
> src/zookeeper.jute 709e935
>
> Diff: https://reviews.apache.org/r/33713/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Jordan Zimmerman
>
>
Re: Review Request 33713: ZOOKEEPER-2163 - Complete implementation
and doc
Posted by Jordan Zimmerman <jo...@jordanzimmerman.com>.
> On May 6, 2015, 9:22 a.m., Rakesh R wrote:
> > src/java/main/org/apache/zookeeper/ZooKeeper.java, line 34
> > <https://reviews.apache.org/r/33713/diff/1/?file=946048#file946048line34>
> >
> > Minor suggestion:
> > I could see many places the import statements has been modified to .*
> >
> > I prefer not to change the existing imports it may show many changes in diff that may deviate reviewers focus. We can do import the necessary classess/items required for the implementation.
Sorry - IntelliJ does this automatically. I'll try to revert these.
> On May 6, 2015, 9:22 a.m., Rakesh R wrote:
> > src/java/main/org/apache/zookeeper/ZooKeeper.java, line 1392
> > <https://reviews.apache.org/r/33713/diff/1/?file=946048#file946048line1392>
> >
> > I assume you are referring to jute.maxbuffer. It is not good to hard code value 1MB, right?
I copied this doc verbatim from the create() method. It seems to me that this should be globally changed in a separate issue.
> On May 6, 2015, 9:22 a.m., Rakesh R wrote:
> > src/java/main/org/apache/zookeeper/ZooKeeper.java, line 1433
> > <https://reviews.apache.org/r/33713/diff/1/?file=946048#file946048line1433>
> >
> > I assume you are referring to jute.maxbuffer. It is not good to hard code value 1MB, right?
I copied this doc verbatim from the create() method. It seems to me that this should be globally changed in a separate issue.
- Jordan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33713/#review82648
-----------------------------------------------------------
On April 30, 2015, 4:08 p.m., Jordan Zimmerman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33713/
> -----------------------------------------------------------
>
> (Updated April 30, 2015, 4:08 p.m.)
>
>
> Review request for zookeeper, michim, Rakesh R, and Raul Gutierrez Segales.
>
>
> Bugs: ZOOKEEPER-2163
> https://issues.apache.org/jira/browse/ZOOKEEPER-2163
>
>
> Repository: zookeeper-git
>
>
> Description
> -------
>
> Introduce new ZNode type: container
>
>
> Diffs
> -----
>
> CHANGES.txt 51ec65d
> bin/zkServer.sh dae3ce2
> src/docs/src/documentation/content/xdocs/zookeeperAdmin.xml c12c2ca
> src/docs/src/documentation/content/xdocs/zookeeperProgrammers.xml 223cf8e
> src/java/main/org/apache/zookeeper/ZooDefs.java a4fc331
> src/java/main/org/apache/zookeeper/ZooKeeper.java dd8ecf4
> src/java/main/org/apache/zookeeper/server/ContainerManager.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/DataNode.java b341a69
> src/java/main/org/apache/zookeeper/server/DataTree.java 78cddb1
> src/java/main/org/apache/zookeeper/server/FinalRequestProcessor.java 7e3c29f
> src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java 4911acf
> src/java/main/org/apache/zookeeper/server/Request.java bed9b13
> src/java/main/org/apache/zookeeper/server/TraceFormatter.java 582383d
> src/java/main/org/apache/zookeeper/server/ZooKeeperServerMain.java 63daea0
> src/java/main/org/apache/zookeeper/server/quorum/CommitProcessor.java cf0900b
> src/java/main/org/apache/zookeeper/server/quorum/FollowerRequestProcessor.java 4d061f4
> src/java/main/org/apache/zookeeper/server/quorum/LeaderZooKeeperServer.java 6434d02
> src/java/main/org/apache/zookeeper/server/quorum/ObserverRequestProcessor.java 36a23ee
> src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyRequestProcessor.java a49319c
> src/java/main/org/apache/zookeeper/server/util/SerializeUtils.java 1a45c5e
> src/java/test/org/apache/zookeeper/server/CreateContainerTest.java PRE-CREATION
> src/zookeeper.jute 709e935
>
> Diff: https://reviews.apache.org/r/33713/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Jordan Zimmerman
>
>
Re: Review Request 33713: ZOOKEEPER-2163 - Complete implementation
and doc
Posted by Rakesh R <ra...@huawei.com>.
> On May 6, 2015, 9:22 a.m., Rakesh R wrote:
> > src/java/main/org/apache/zookeeper/ZooKeeper.java, line 1404
> > <https://reviews.apache.org/r/33713/diff/1/?file=946048#file946048line1404>
> >
> > One general thought:
> >
> > Instead of providing a new API for creating the container, can we think of "CreateMode.CONTAINER" a new znode type? Also, I feel this may help us to support multiTxn/batch apis.
> >
> > I haven't done detailed analysis, this is just a rough thought that comes in my mind.
>
> Jordan Zimmerman wrote:
> At this point, I don't remember why I chose to add a new API. Other committers haven't asked for this. But, if you like I can change it a new CreateMode. Supporting multi/trans is a good point. Please advise.
Yes, since this is a design change it would be good to listen others thought also. IMO will add this point in the jira so that will get more attention.
- Rakesh
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33713/#review82648
-----------------------------------------------------------
On April 30, 2015, 4:08 p.m., Jordan Zimmerman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33713/
> -----------------------------------------------------------
>
> (Updated April 30, 2015, 4:08 p.m.)
>
>
> Review request for zookeeper, michim, Rakesh R, and Raul Gutierrez Segales.
>
>
> Bugs: ZOOKEEPER-2163
> https://issues.apache.org/jira/browse/ZOOKEEPER-2163
>
>
> Repository: zookeeper-git
>
>
> Description
> -------
>
> Introduce new ZNode type: container
>
>
> Diffs
> -----
>
> CHANGES.txt 51ec65d
> bin/zkServer.sh dae3ce2
> src/docs/src/documentation/content/xdocs/zookeeperAdmin.xml c12c2ca
> src/docs/src/documentation/content/xdocs/zookeeperProgrammers.xml 223cf8e
> src/java/main/org/apache/zookeeper/ZooDefs.java a4fc331
> src/java/main/org/apache/zookeeper/ZooKeeper.java dd8ecf4
> src/java/main/org/apache/zookeeper/server/ContainerManager.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/DataNode.java b341a69
> src/java/main/org/apache/zookeeper/server/DataTree.java 78cddb1
> src/java/main/org/apache/zookeeper/server/FinalRequestProcessor.java 7e3c29f
> src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java 4911acf
> src/java/main/org/apache/zookeeper/server/Request.java bed9b13
> src/java/main/org/apache/zookeeper/server/TraceFormatter.java 582383d
> src/java/main/org/apache/zookeeper/server/ZooKeeperServerMain.java 63daea0
> src/java/main/org/apache/zookeeper/server/quorum/CommitProcessor.java cf0900b
> src/java/main/org/apache/zookeeper/server/quorum/FollowerRequestProcessor.java 4d061f4
> src/java/main/org/apache/zookeeper/server/quorum/LeaderZooKeeperServer.java 6434d02
> src/java/main/org/apache/zookeeper/server/quorum/ObserverRequestProcessor.java 36a23ee
> src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyRequestProcessor.java a49319c
> src/java/main/org/apache/zookeeper/server/util/SerializeUtils.java 1a45c5e
> src/java/test/org/apache/zookeeper/server/CreateContainerTest.java PRE-CREATION
> src/zookeeper.jute 709e935
>
> Diff: https://reviews.apache.org/r/33713/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Jordan Zimmerman
>
>
Re: Review Request 33713: ZOOKEEPER-2163 - Complete implementation
and doc
Posted by Rakesh R <ra...@huawei.com>.
> On May 6, 2015, 9:22 a.m., Rakesh R wrote:
> > src/java/test/org/apache/zookeeper/server/CreateContainerTest.java, line 90
> > <https://reviews.apache.org/r/33713/diff/1/?file=946063#file946063line90>
> >
> > Can we think of some better logic instead of Thread.sleep(1000) in tests. It may be flaky in some env, right?
>
> Jordan Zimmerman wrote:
> I have no other idea. A record is added into ZK's processor and there are no testing hooks for that. I need to wait for it to get processed. If you have a suggestion, please let me know. NOTE: ZK's own tests have these timers.
Yes, this we can put on hold. Right now, am not getting any idea.
> On May 6, 2015, 9:22 a.m., Rakesh R wrote:
> > src/java/test/org/apache/zookeeper/server/CreateContainerTest.java, line 131
> > <https://reviews.apache.org/r/33713/diff/1/?file=946063#file946063line131>
> >
> > Can we think of some better logic instead of Thread.sleep(1000) in tests. It may be flaky in some env, right?
> >
> > I could see there are few more occurances of Thread.sleep(xx) similar to this, please do the changes. Thanks!
>
> Jordan Zimmerman wrote:
> I have no other idea. A record is added into ZK's processor and there are no testing hooks for that. I need to wait for it to get processed. If you have a suggestion, please let me know. NOTE: ZK's own tests have these timers.
Yes, this we can put on hold. Right now, am not getting any idea.
- Rakesh
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33713/#review82648
-----------------------------------------------------------
On May 16, 2015, 4:18 p.m., Jordan Zimmerman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33713/
> -----------------------------------------------------------
>
> (Updated May 16, 2015, 4:18 p.m.)
>
>
> Review request for zookeeper, michim, Rakesh R, and Raul Gutierrez Segales.
>
>
> Bugs: ZOOKEEPER-2163
> https://issues.apache.org/jira/browse/ZOOKEEPER-2163
>
>
> Repository: zookeeper-git
>
>
> Description
> -------
>
> Introduce new ZNode type: container
>
>
> Diffs
> -----
>
> src/docs/src/documentation/content/xdocs/zookeeperAdmin.xml c12c2ca
> src/docs/src/documentation/content/xdocs/zookeeperProgrammers.xml 223cf8e
> src/java/main/org/apache/zookeeper/CreateMode.java d87f410
> src/java/main/org/apache/zookeeper/MultiTransactionRecord.java ea913b4
> src/java/main/org/apache/zookeeper/Op.java 97d3d7b
> src/java/main/org/apache/zookeeper/ZooDefs.java a4fc331
> src/java/main/org/apache/zookeeper/ZooKeeper.java dd8ecf4
> src/java/main/org/apache/zookeeper/cli/CreateCommand.java c6de7c6
> src/java/main/org/apache/zookeeper/server/ContainerManager.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/DataNode.java b341a69
> src/java/main/org/apache/zookeeper/server/DataTree.java 78cddb1
> src/java/main/org/apache/zookeeper/server/FinalRequestProcessor.java 7e3c29f
> src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java 4911acf
> src/java/main/org/apache/zookeeper/server/Request.java bed9b13
> src/java/main/org/apache/zookeeper/server/TraceFormatter.java 582383d
> src/java/main/org/apache/zookeeper/server/ZooKeeperServerMain.java 63daea0
> src/java/main/org/apache/zookeeper/server/quorum/CommitProcessor.java cf0900b
> src/java/main/org/apache/zookeeper/server/quorum/FollowerRequestProcessor.java 4d061f4
> src/java/main/org/apache/zookeeper/server/quorum/LeaderZooKeeperServer.java 6434d02
> src/java/main/org/apache/zookeeper/server/quorum/ObserverRequestProcessor.java 36a23ee
> src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyRequestProcessor.java a49319c
> src/java/main/org/apache/zookeeper/server/util/SerializeUtils.java 1a45c5e
> src/java/test/org/apache/zookeeper/server/CreateContainerTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/test/CreateModeTest.java 9db01bb
> src/zookeeper.jute 709e935
>
> Diff: https://reviews.apache.org/r/33713/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Jordan Zimmerman
>
>