You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@zookeeper.apache.org by TyqITstudent <gi...@git.apache.org> on 2018/11/24 06:43:46 UTC

[GitHub] zookeeper pull request #720: add an API to get total count of recursive sub ...

GitHub user TyqITstudent opened a pull request:

    https://github.com/apache/zookeeper/pull/720

    add an API to get total count of recursive sub nodes of one node

    In production environment, there will be always a situation that there are a lot of recursive sub nodes of one node. We need to count total number of it.
    
    Now, we can only use API getChildren which returns the List of first level of sub nodes. We need to iterate every sub node to get recursive sub nodes. It will cost a lot of time.
    
    In zookeeper server side, it uses Hasp<String, DataNode> to store node. The key of the map represents the path of the node. We can iterate the map get total number of all levels of sub nodes of one node.

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/TyqITstudent/zookeeper ZOOKEEPER-3167

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/zookeeper/pull/720.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #720
    
----
commit f21dab121f255959032148e6608b84c12ed0bd68
Author: tianyiqun <89...@...>
Date:   2018-11-24T06:39:30Z

    add an API to get total count of recursive sub nodes of one node

----


---

[GitHub] zookeeper issue #720: add an API to get total count of recursive sub nodes o...

Posted by TyqITstudent <gi...@git.apache.org>.
Github user TyqITstudent commented on the issue:

    https://github.com/apache/zookeeper/pull/720
  
    > At every push CI will retest your work.
    > Alternatively you can close and reopen the PR
    
    When Jenkins check these parts, I change nothing but output are errors. Could you please help me to solve this?  (In compile area)
    [Jenkins compile result.](https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/2745/artifact/patchprocess/trunkJavacWarnings.txt/*view*/)



---

[GitHub] zookeeper issue #720: add an API to get total count of recursive sub nodes o...

Posted by eolivelli <gi...@git.apache.org>.
Github user eolivelli commented on the issue:

    https://github.com/apache/zookeeper/pull/720
  
    At every push CI will retest your work.
    Alternatively you can close and reopen the PR


---

[GitHub] zookeeper pull request #720: add an API to get total count of recursive sub ...

Posted by nkalmar <gi...@git.apache.org>.
Github user nkalmar commented on a diff in the pull request:

    https://github.com/apache/zookeeper/pull/720#discussion_r236578709
  
    --- Diff: zookeeper-server/src/main/java/org/apache/zookeeper/ZooDefs.java ---
    @@ -50,6 +50,8 @@
     
             public final int getChildren = 8;
     
    +        public final int getAllChildrenNumber = 20;
    --- End diff --
    
    20 is already taken for deleteContainer, please change it to something else (22 seems to be the first free number until 100)


---

[GitHub] zookeeper issue #720: add an API to get total count of recursive sub nodes o...

Posted by TyqITstudent <gi...@git.apache.org>.
Github user TyqITstudent commented on the issue:

    https://github.com/apache/zookeeper/pull/720
  
    > It seems you are not renaming classes/fields in every point of code
    
    Ok, thanks for your remind.


---

[GitHub] zookeeper pull request #720: add an API to get total count of recursive sub ...

Posted by TyqITstudent <gi...@git.apache.org>.
GitHub user TyqITstudent reopened a pull request:

    https://github.com/apache/zookeeper/pull/720

    add an API to get total count of recursive sub nodes of one node

    In production environment, there will be always a situation that there are a lot of recursive sub nodes of one node. We need to count total number of it.
    
    Now, we can only use API getChildren which returns the List of first level of sub nodes. We need to iterate every sub node to get recursive sub nodes. It will cost a lot of time.
    
    In zookeeper server side, it uses Hasp<String, DataNode> to store node. The key of the map represents the path of the node. We can iterate the map get total number of all levels of sub nodes of one node.

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/TyqITstudent/zookeeper ZOOKEEPER-3167

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/zookeeper/pull/720.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #720
    
----
commit f21dab121f255959032148e6608b84c12ed0bd68
Author: tianyiqun <89...@...>
Date:   2018-11-24T06:39:30Z

    add an API to get total count of recursive sub nodes of one node

commit 1b527726f52499aa943de1ec63de4ce9967300cf
Author: tianyiqun <89...@...>
Date:   2018-11-24T06:39:30Z

    add an API to get total count of recursive sub nodes of one node

commit 67760fed151fce49f29fabc577eef19216cef94b
Author: tianyiqun <89...@...>
Date:   2018-11-24T11:12:43Z

    Merge branch 'ZOOKEEPER-3167' of https://github.com/TyqITstudent/zookeeper into ZOOKEEPER-3167

----


---

[GitHub] zookeeper pull request #720: add an API to get total count of recursive sub ...

Posted by TyqITstudent <gi...@git.apache.org>.
Github user TyqITstudent commented on a diff in the pull request:

    https://github.com/apache/zookeeper/pull/720#discussion_r236039566
  
    --- Diff: zookeeper-server/src/main/java/org/apache/zookeeper/ClientCnxn.java ---
    @@ -616,6 +616,14 @@ private void processEvent(Object event) {
                           } else {
                               cb.processResult(rc, clientPath, p.ctx, null);
                           }
    +                  } else if (p.response instanceof GetAllChildrenNumberResponse) {
    --- End diff --
    
    > Also need to import GetAllChildrenNumberResponse
    
    I have added the import, and how to re-trigger the Jenkins build to check the code?


---

[GitHub] zookeeper pull request #720: add an API to get total count of recursive sub ...

Posted by nkalmar <gi...@git.apache.org>.
Github user nkalmar commented on a diff in the pull request:

    https://github.com/apache/zookeeper/pull/720#discussion_r236036475
  
    --- Diff: zookeeper-server/src/main/java/org/apache/zookeeper/ClientCnxn.java ---
    @@ -616,6 +616,14 @@ private void processEvent(Object event) {
                           } else {
                               cb.processResult(rc, clientPath, p.ctx, null);
                           }
    +                  } else if (p.response instanceof GetAllChildrenNumberResponse) {
    --- End diff --
    
    Also need to import GetAllChildrenNumberResponse 


---

[GitHub] zookeeper issue #720: add an API to get total count of recursive sub nodes o...

Posted by TyqITstudent <gi...@git.apache.org>.
Github user TyqITstudent commented on the issue:

    https://github.com/apache/zookeeper/pull/720
  
    name the API 'countChildren'
    add a flag to ask for a recursive traversal or simply have the count without listing
    add also the async version of the method
    
    1.  your name is better, I can change method name.
    2.  you means that the method returns the number of first level children (if flag is false)?
    3.  I  will add the async method.



---

[GitHub] zookeeper pull request #720: add an API to get total count of recursive sub ...

Posted by TyqITstudent <gi...@git.apache.org>.
Github user TyqITstudent closed the pull request at:

    https://github.com/apache/zookeeper/pull/720


---

[GitHub] zookeeper issue #720: add an API to get total count of recursive sub nodes o...

Posted by eolivelli <gi...@git.apache.org>.
Github user eolivelli commented on the issue:

    https://github.com/apache/zookeeper/pull/720
  
    @TyqITstudent 
    For the 'recursive' flag I mean:
    - true: recurse thw tree
    - false: count only first level
    
    About the build...does the build pass locally on your machine?
    It seems you are not renaming classes/fields in every point of code 


---

[GitHub] zookeeper pull request #720: add an API to get total count of recursive sub ...

Posted by TyqITstudent <gi...@git.apache.org>.
Github user TyqITstudent commented on a diff in the pull request:

    https://github.com/apache/zookeeper/pull/720#discussion_r236065557
  
    --- Diff: zookeeper-server/src/main/java/org/apache/zookeeper/ClientCnxn.java ---
    @@ -616,6 +616,14 @@ private void processEvent(Object event) {
                           } else {
                               cb.processResult(rc, clientPath, p.ctx, null);
                           }
    +                  } else if (p.response instanceof GetAllChildrenNumberResponse) {
    --- End diff --
    
    > Also need to import GetAllChildrenNumberResponse
    
    OK, thanks for your remind.


---

[GitHub] zookeeper issue #720: add an API to get total count of recursive sub nodes o...

Posted by TyqITstudent <gi...@git.apache.org>.
Github user TyqITstudent commented on the issue:

    https://github.com/apache/zookeeper/pull/720
  
    > At every push CI will retest your work.
    > Alternatively you can close and reopen the PR
    
    Your advice is good, I will change some parts of my code.


---

[GitHub] zookeeper pull request #720: add an API to get total count of recursive sub ...

Posted by nkalmar <gi...@git.apache.org>.
Github user nkalmar commented on a diff in the pull request:

    https://github.com/apache/zookeeper/pull/720#discussion_r236036464
  
    --- Diff: zookeeper-server/src/main/java/org/apache/zookeeper/ZooKeeper.java ---
    @@ -2495,6 +2495,30 @@ public void setACL(final String path, List<ACL> acl, int version,
             return getChildren(path, watch ? watchManager.defaultWatcher : null);
         }
     
    +    /*
    +    *  Get all children number of one node
    +    * */
    +    public int getAllChildrenNumber(final String path)
    +            throws KeeperException, InterruptedException {
    +        int totalNumber = 0;
    +        final String clientPath = path;
    +        PathUtils.validatePath(clientPath);
    +        // the watch contains the un-chroot path
    +        WatchRegistration wcb = null;
    +        final String serverPath = prependChroot(clientPath);
    +        RequestHeader h = new RequestHeader();
    +        h.setType(ZooDefs.OpCode.getAllChildrenNumber);
    +        GetAllChildrenNumberRequest request = new GetAllChildrenNumberRequest();
    --- End diff --
    
    You need to import the class GetAllChildrenNumberRequest which jute generates.


---

[GitHub] zookeeper pull request #720: add an API to get total count of recursive sub ...

Posted by TyqITstudent <gi...@git.apache.org>.
Github user TyqITstudent commented on a diff in the pull request:

    https://github.com/apache/zookeeper/pull/720#discussion_r236655892
  
    --- Diff: zookeeper-server/src/main/java/org/apache/zookeeper/ZooDefs.java ---
    @@ -50,6 +50,8 @@
     
             public final int getChildren = 8;
     
    +        public final int getAllChildrenNumber = 20;
    --- End diff --
    
    OK, I will change the number.


---