Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 0.7
    • Component/s: mk
    • Labels:
      None

      Issue Links

        Activity

        Dominique Pfister created issue -
        Dominique Pfister made changes -
        Field Original Value New Value
        Summary Support for large child node lists. Support for large child node lists
        Michael Dürig made changes -
        Link This issue relates to OAK-586 [ OAK-586 ]
        Hide
        Michael Dürig added a comment -

        OAK-586 is the related issue for the MongoMk

        Show
        Michael Dürig added a comment - OAK-586 is the related issue for the MongoMk
        Hide
        Marcel Reutegger added a comment -

        As mentioned in OAK-586, the child count is IMO more problematic because it must be exact. This either means the implementation needs to maintain the value somewhere and must update it whenever a node is added or removed. This requires synchronization on this value and causes contention. On the other hand an implementation, which choses to not store the child node count is forced to count the potentially many child nodes. I think it would be better to get rid of the child node count.

        The JCR API specifically allows an implementation to return -1 for the size of a RangeIterator. I think we should make use of this in Oak as well. That is, :childNodeCount may return -1 if the implementation does not know the exact number of child nodes and there exist more than the requested number of child nodes.

        Show
        Marcel Reutegger added a comment - As mentioned in OAK-586 , the child count is IMO more problematic because it must be exact. This either means the implementation needs to maintain the value somewhere and must update it whenever a node is added or removed. This requires synchronization on this value and causes contention. On the other hand an implementation, which choses to not store the child node count is forced to count the potentially many child nodes. I think it would be better to get rid of the child node count. The JCR API specifically allows an implementation to return -1 for the size of a RangeIterator. I think we should make use of this in Oak as well. That is, :childNodeCount may return -1 if the implementation does not know the exact number of child nodes and there exist more than the requested number of child nodes.
        Hide
        Michael Dürig added a comment -

        From the top of my head and without tracing all the usages through oak-core I think this should be doable. There are some places where the child node count is used for optimising common cases (e.g. 0 and 1 children, equality test) but I think these could be easily adapted to cope with the unknown case.
        Would it be possible to provide the child node count by best effort only? I.e. for small child node lists?

        For the respective JCR methods, the child node count does not work anyway since access control information needs to be considered.

        Also, instead of returning -1, why not just leave the :childNodeCount property entirely away from the returned JSON?

        Finally, there is also MicroKernel.getChildNodeCount. Should this method stay to "force" counting the child nodes, should we remove it, or should its semantic change along the line of the :childNodeCount property?

        Show
        Michael Dürig added a comment - From the top of my head and without tracing all the usages through oak-core I think this should be doable. There are some places where the child node count is used for optimising common cases (e.g. 0 and 1 children, equality test) but I think these could be easily adapted to cope with the unknown case. Would it be possible to provide the child node count by best effort only? I.e. for small child node lists? For the respective JCR methods, the child node count does not work anyway since access control information needs to be considered. Also, instead of returning -1, why not just leave the :childNodeCount property entirely away from the returned JSON? Finally, there is also MicroKernel.getChildNodeCount . Should this method stay to "force" counting the child nodes, should we remove it, or should its semantic change along the line of the :childNodeCount property?
        Hide
        Marcel Reutegger added a comment -

        there is also MicroKernel.getChildNodeCount

        The method is not used in oak-core (except for MicroKernel wrapper implementations) and the JavaDoc of the method says it's a convenience method. I'd say we remove it.

        Show
        Marcel Reutegger added a comment - there is also MicroKernel.getChildNodeCount The method is not used in oak-core (except for MicroKernel wrapper implementations) and the JavaDoc of the method says it's a convenience method. I'd say we remove it.
        Hide
        Mete Atamel added a comment -

        I should also mention that :childNodeCount prevents to efficiently implement MicroKernel#getNodes in MongoMK especially when maxChildNode is specified. Currently, when maxChildNode is specified, MongoMK still fetches all the children under path and then it applies maxChildNode in JsonUtil#toJson call in MicroKernel. Instead, it could easily apply maxChildNode when it talks to MongoDB but it cannot do that because the exact childNodeCount is needed, hence it needs to grab all children no matter what maxChildNode is specified.

        Show
        Mete Atamel added a comment - I should also mention that :childNodeCount prevents to efficiently implement MicroKernel#getNodes in MongoMK especially when maxChildNode is specified. Currently, when maxChildNode is specified, MongoMK still fetches all the children under path and then it applies maxChildNode in JsonUtil#toJson call in MicroKernel. Instead, it could easily apply maxChildNode when it talks to MongoDB but it cannot do that because the exact childNodeCount is needed, hence it needs to grab all children no matter what maxChildNode is specified.
        Hide
        Dominique Pfister added a comment -

        Guys, may I suggest you create a new issue to track this discussion about removing :childNodeCount and/or MicroKernel.getChildNodeCount, so it is clearly visible for everybody else not watching this issue?

        Show
        Dominique Pfister added a comment - Guys, may I suggest you create a new issue to track this discussion about removing :childNodeCount and/or MicroKernel.getChildNodeCount , so it is clearly visible for everybody else not watching this issue?
        Dominique Pfister made changes -
        Status Open [ 1 ] In Progress [ 3 ]
        Hide
        Dominique Pfister added a comment - - edited

        Initial implementation added in r1368834, fix invalid cast in r1436464.

        Show
        Dominique Pfister added a comment - - edited Initial implementation added in r1368834, fix invalid cast in r1436464.
        Dominique Pfister made changes -
        Status In Progress [ 3 ] Resolved [ 5 ]
        Fix Version/s 0.7 [ 12323981 ]
        Resolution Fixed [ 1 ]
        Jukka Zitting made changes -
        Status Resolved [ 5 ] Closed [ 6 ]

          People

          • Assignee:
            Dominique Pfister
            Reporter:
            Dominique Pfister
          • Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development