You are viewing a plain text version of this content. The canonical link for it is here.
Posted to oak-dev@jackrabbit.apache.org by Chetan Mehrotra <ch...@gmail.com> on 2017/02/22 11:57:54 UTC

CommitEditors looking for specific child node like oak:index, rep:cugPolicy leads to lots of redundant remote calls

We have some CommitEditors in Oak which look for specific child node
upon each commit like 'oak:index' and 'rep:cugPolicy'

In most cases such child node does not exist and this leads to extra
remote calls in case of DocumentNodeStore to determine if child with
such a name exist or not. In case of updates to nodes where child data
is not cached this quickly adds up and becomes a major portion of
remote call made from Oak and something which we can avoid

We should look into approaches where such child lookup can be avoided
in critical write path.

One possible approach is to mark the parent with a specific hidden
property which has such a node upon addition. This would avoid the
negative lookup in case of updates

Chetan Mehrotra

Re: CommitEditors looking for specific child node like oak:index, rep:cugPolicy leads to lots of redundant remote calls

Posted by Thomas Mueller <mu...@adobe.com>.
Hi,

For "oak:index" of type oak:QueryIndexDefinition, what about a hidden
property ":hasOakIndex" = true. That would be
NodeBuilder.hasProperty(":hasOakIndex").

Regards,
Thomas


On 22/02/17 12:57, "Chetan Mehrotra" <ch...@gmail.com> wrote:

>We have some CommitEditors in Oak which look for specific child node
>upon each commit like 'oak:index' and 'rep:cugPolicy'
>
>In most cases such child node does not exist and this leads to extra
>remote calls in case of DocumentNodeStore to determine if child with
>such a name exist or not. In case of updates to nodes where child data
>is not cached this quickly adds up and becomes a major portion of
>remote call made from Oak and something which we can avoid
>
>We should look into approaches where such child lookup can be avoided
>in critical write path.
>
>One possible approach is to mark the parent with a specific hidden
>property which has such a node upon addition. This would avoid the
>negative lookup in case of updates
>
>Chetan Mehrotra


Re: CommitEditors looking for specific child node like oak:index, rep:cugPolicy leads to lots of redundant remote calls

Posted by Angela Schreiber <an...@adobe.com>.
hi

yes that mixin exists. but the CugValidatorProvider is performs additional
validation and doesn't allow for usage of the reserved names if the mixin
is not there... so we cannot limit the evaluation to nodes that have the
mixin type set (or inherited through super-type).
see http://jackrabbit.apache.org/oak/docs/security/authorization/cug.html
for details.

kind regards
angela

On 22/02/17 14:26, "Marcel Reutegger" <mr...@adobe.com> wrote:

>Hi,
>
>On 22/02/17 12:57, Chetan Mehrotra wrote:
>> One possible approach is to mark the parent with a specific hidden
>> property which has such a node upon addition. This would avoid the
>> negative lookup in case of updates
>
>Alternatively we could also mark the parent node with a mixin. This
>would be similar to nodes that can have a rep:policy child nodes. In
>that case the parent node is rep:AccessControllable.
>
>At least for rep:cugPolicy nodes this seems to be the case already. The
>node type registry of my Oak 1.6 repository says:
>
>[rep:CugMixin]
>   mixin
>   + rep:cugPolicy (rep:CugPolicy) protected ignore
>
>[rep:CugPolicy] > rep:Policy
>   - rep:principalNames (string) mandatory protected multiple ignore
>
>
>Regards
>  Marcel


Re: CommitEditors looking for specific child node like oak:index, rep:cugPolicy leads to lots of redundant remote calls

Posted by Marcel Reutegger <mr...@adobe.com>.
Hi,

On 22/02/17 12:57, Chetan Mehrotra wrote:
> One possible approach is to mark the parent with a specific hidden
> property which has such a node upon addition. This would avoid the
> negative lookup in case of updates

Alternatively we could also mark the parent node with a mixin. This 
would be similar to nodes that can have a rep:policy child nodes. In 
that case the parent node is rep:AccessControllable.

At least for rep:cugPolicy nodes this seems to be the case already. The 
node type registry of my Oak 1.6 repository says:

[rep:CugMixin]
   mixin
   + rep:cugPolicy (rep:CugPolicy) protected ignore

[rep:CugPolicy] > rep:Policy
   - rep:principalNames (string) mandatory protected multiple ignore


Regards
  Marcel

Re: CommitEditors looking for specific child node like oak:index, rep:cugPolicy leads to lots of redundant remote calls

Posted by Chetan Mehrotra <ch...@gmail.com>.
I realized now that I logged an issue for this recently OAK-5511 which
mentioned similar approach. So lets move this discussion there
Chetan Mehrotra


On Thu, Feb 23, 2017 at 7:06 PM, Thomas Mueller <mu...@adobe.com> wrote:
> Hi,
>
>>I like Marcel proposal for "enforcing" use of mixin on parent node to
>>indicate that it can have a child node of 'oak:index'. So we can
>>leverage mxin 'mix:indexable' (OAK-3725) to mark such parent nodes
>>(like root) and IndexUpdate would only look for 'oak:index' node if
>>current node has that mixin.
>
> Ah I didn't know about OAK-3725.
>
> I'm a bit worried that we mix different aspects together, not sure which
> is better.
>
> "oak:Indexable" is visible, so it can be added and _removed_ by the user.
> So when trying to remove that mixin, we would need to check there is no
> oak:index child node with nodetype oak:QueryIndexDefinition. We need to
> check the nodetype hierarchy. On the other hand, possibly we can enforce
> that the parent node of oak:index is oak:Indexable (can we?)
>
> I'm not saying with a hidden property hidden property ":hasOakIndex"
> (automatically set and removed) it would be painless. For example when
> moving an oak:index node to a new parent, the setting has to be changed at
> both the original and the new parents.
>
> Regards,
> Thomas
>
>

Re: CommitEditors looking for specific child node like oak:index, rep:cugPolicy leads to lots of redundant remote calls

Posted by Thomas Mueller <mu...@adobe.com>.
Hi,

>I like Marcel proposal for "enforcing" use of mixin on parent node to
>indicate that it can have a child node of 'oak:index'. So we can
>leverage mxin 'mix:indexable' (OAK-3725) to mark such parent nodes
>(like root) and IndexUpdate would only look for 'oak:index' node if
>current node has that mixin.

Ah I didn't know about OAK-3725.

I'm a bit worried that we mix different aspects together, not sure which
is better.

"oak:Indexable" is visible, so it can be added and _removed_ by the user.
So when trying to remove that mixin, we would need to check there is no
oak:index child node with nodetype oak:QueryIndexDefinition. We need to
check the nodetype hierarchy. On the other hand, possibly we can enforce
that the parent node of oak:index is oak:Indexable (can we?)

I'm not saying with a hidden property hidden property ":hasOakIndex"
(automatically set and removed) it would be painless. For example when
moving an oak:index node to a new parent, the setting has to be changed at
both the original and the new parents.

Regards,
Thomas



Re: CommitEditors looking for specific child node like oak:index, rep:cugPolicy leads to lots of redundant remote calls

Posted by Chetan Mehrotra <ch...@gmail.com>.
I like Marcel proposal for "enforcing" use of mixin on parent node to
indicate that it can have a child node of 'oak:index'. So we can
leverage mxin 'mix:indexable' (OAK-3725) to mark such parent nodes
(like root) and IndexUpdate would only look for 'oak:index' node if
current node has that mixin.

This would avoid the extra calls. For new setups we can enforce this
and for upgrade we can migrate the existing code by using nodetype
index to update all such "indexable" nodes
Chetan Mehrotra


On Thu, Feb 23, 2017 at 4:47 PM, Chetan Mehrotra
<ch...@gmail.com> wrote:
> On Wed, Feb 22, 2017 at 8:21 PM, Davide Giannella <da...@apache.org> wrote:
>> Did you mean for ALL the nodes, or only specific nodes?
>>
>> Any way you're suggesting something like the following flow:
>>
>> 1) user call nodebuilder.child(":index")
>> 2) lookup in hidden property
>> 3) if not there, leverage the existing code
>>
>> If so I guess the property has been already fetched and it does not
>> require roundtrips towards the DB. Am I right?
>
> Currently the lookup is being done for ALL nodes. So IndexUpdate class
> does following on each changed node
>
> ------
>>     @Override
>     public void enter(NodeState before, NodeState after)
>             throws CommitFailedException {
>         collectIndexEditors(builder.getChildNode(INDEX_DEFINITIONS_NAME),
> before);
> ------
>
> Which transalates into checking if the current node has a child node
> 'oak:index' and this leads to redudant calls.
>
> Chetan Mehrotra

Re: CommitEditors looking for specific child node like oak:index, rep:cugPolicy leads to lots of redundant remote calls

Posted by Chetan Mehrotra <ch...@gmail.com>.
On Wed, Feb 22, 2017 at 8:21 PM, Davide Giannella <da...@apache.org> wrote:
> Did you mean for ALL the nodes, or only specific nodes?
>
> Any way you're suggesting something like the following flow:
>
> 1) user call nodebuilder.child(":index")
> 2) lookup in hidden property
> 3) if not there, leverage the existing code
>
> If so I guess the property has been already fetched and it does not
> require roundtrips towards the DB. Am I right?

Currently the lookup is being done for ALL nodes. So IndexUpdate class
does following on each changed node

------
>     @Override
    public void enter(NodeState before, NodeState after)
            throws CommitFailedException {
        collectIndexEditors(builder.getChildNode(INDEX_DEFINITIONS_NAME),
before);
------

Which transalates into checking if the current node has a child node
'oak:index' and this leads to redudant calls.

Chetan Mehrotra

Re: CommitEditors looking for specific child node like oak:index, rep:cugPolicy leads to lots of redundant remote calls

Posted by Davide Giannella <da...@apache.org>.
On 22/02/2017 11:57, Chetan Mehrotra wrote:
> One possible approach is to mark the parent with a specific hidden
> property which has such a node upon addition. This would avoid the
> negative lookup in case of updates

Did you mean for ALL the nodes, or only specific nodes?

Any way you're suggesting something like the following flow:

1) user call nodebuilder.child(":index")
2) lookup in hidden property
3) if not there, leverage the existing code

If so I guess the property has been already fetched and it does not
require roundtrips towards the DB. Am I right?

Davide