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

Chetan Mehrotra commented on OAK-4916:
--------------------------------------

Somehow not very comfortable with {{NOOP_CHANGE}} based protocol for 
communicating filtering.

May be we introduce new interfaces

# ContentChangeFilter - This would be passed to {{BackgroundObserver}} and 
would be responsible for determining if given change should be included in the 
queue or not. By default {{BackgroundObserver}} would 
{{ContentChangeFilter#DEFAULT}}. Observers which support prefiltering would 
provide there own implementation. Kind of composition over inheritence
{code}
public interface ContentChangeFilter {
    ContentChangeFilter DEFAULT = new ContentChangeFilter() {
        @Override
        public boolean includeChange(CommitInfo info) {
            return true
        }
    };
    
    boolean includeChange(CommitInfo info);
}
{code}
# FilteringAwareObserver - Any observer which allows prefiltering would also 
implement this interface. And instead of {{NOOP_CHANGE}} based convention 
{{BackgroundObserver}} would invoke the {{resetPreviousRoot}} to indicate that 
base root state needs to be adjusted. 
{code}
public interface FilteringAwareObserver extends Observer{
    /**
     * Invoked to enable such observers to reset there previous root
     * to given NodeState
     * 
     * @param root previous NodeState root
     */
    void resetPreviousRoot(NodeState root);
}
{code}

Another approach would be to make such observer state less i.e. they do not 
track the previous state and we pass in previous state and 
{{BackgroundObserver}} maintains a tuple 

{code}
void contentChanged(@Nullable NodeState before, @Nonnull NodeState after, 
@Nullable CommitInfo info);
{code}

[~mduerig] Would have better thoughts here ;)

> 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
>
>
> 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.



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

Reply via email to