[ 
https://issues.apache.org/jira/browse/OAK-4916?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15595433#comment-15595433
 ] 

Marcel Reutegger commented on OAK-4916:
---------------------------------------

I would also prefer a compositional approach. I already find it difficult to 
understand what exactly {{BackgroundObserver.contentChanged()}} does right now. 
Adding more logic to it will probably make it more difficult to maintain and 
understand.

I like the NOOP_CHANGED idea, but I would try to keep it an implementation 
detail. That means we'd need to introduce a new kind of observer, which allows 
filtering of some contentChanged() calls. This new type of observer would not 
just get the new root state, but also the base state it needs to compare 
against. In between we'd have our new filtering observer that supports queuing 
with the help of a BackgroundObserver, but also pre-filters incoming changes, 
collapses multiple filtered changes into a NOOP_CHANGED and passes it to the 
BackgroundObserver. The latter doesn't need to understand the semantics of 
NOOP_CHANGED. But it would be recognized on the consumer side of the new 
filtering observer.

I'll try to sketch this idea in a patch.

> Add support for excluding commits to BackgroundObserver
> -------------------------------------------------------
>
>                 Key: OAK-4916
>                 URL: https://issues.apache.org/jira/browse/OAK-4916
>             Project: Jackrabbit Oak
>          Issue Type: Technical task
>          Components: core
>    Affects Versions: 1.5.11
>            Reporter: Stefan Egli
>            Assignee: Stefan Egli
>             Fix For: 1.6
>
>         Attachments: OAK-4916.patch, OAK-4916.v2.patch
>
>
> As part of pre-filtering commits it would be useful to have support in the 
> BackgroundObserver (in general) that would allow to exclude certain commits 
> from being added to the (BackgroundObserver's) queue, thus keeping the queue 
> smaller. The actual filtering is up to subclasses.
> The suggested implementation is as follows:
> * a new method {{isExcluded}} is introduced which represents a subclass hook 
> for filtering
> * excluded commits are not added to the queue
> * when multiple commits are excluded subsequently, this is collapsed
> * the first non-excluded commit (ContentChange) added to the queue is marked 
> with the last non-excluded root state as the 'previous root'
> * downstream Observers are notified of the exclusion of a commit via a 
> special CommitInfo {{NOOP_CHANGE}}: this instructs it to exclude this change 
> while at the same time 'fast-forwarding' the root state to the new one.
> ** this extra token is one way of solving the problem that 
> {{Observer.contentChanged}} represents a diff between two states but does not 
> transport the 'from' state explicitly - that is implicitly taken from the 
> previous call to {{contentChanged}}. Thus using such a gap token 
> ({{NOOP_CHANGE}}) seems to be the only way to instruct Observers to skip a 
> change.
> To repeat: whoever extends BackgroundObserver with filtering must be aware of 
> the new {{NOOP_CHANGE}} token. Anyone not doing filtering will not get any 
> {{NOOP_CHANGE}} tokens though.
> NOTE: See [comment further 
> below|https://issues.apache.org/jira/browse/OAK-4916?focusedCommentId=15572165&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-15572165]
>  with a new suggested approach, which doesn't use NOOP_CHANGED but introduces 
> a new FilteringAwareObserver instead.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Reply via email to