Provide a way to pass indexing related state to IndexEditorProvider (OAK-4642)

[Chetan Mehrotra <ch...@gmail.com>]

Hi Team,

As a follow up to previous mail around "Using same index definition
for both async and sync indexing" wanted to discuss the next step. We
need to provide a way to pass indexing related state to
IndexEditorProvider (OAK-4642)

Over the period of time I have seen need for extra state like

1. reindexing - Currently the index implementation use some heuristic
    like check before root state being empty to determine if they are
running in reindexing mode
2. indexing mode - sync or async
3. index path of the index (see OAK-4152)
4. CommitInfo (see OAK-4640)

For #1 and #3 we have done some kind of workaround but it would be
better to have a first class support for that.

So we would need to introduce some sort of IndexingContext and have
the api for IndexEditorProvider like below

=========
    @CheckForNull
    Editor getIndexEditor(
            @Nonnull String type, @Nonnull NodeBuilder definition,
            @Nonnull NodeState root,
            @Nonnull IndexingContext context) throws CommitFailedException;
=========

To introduce such a change I see 3 options

* O1 - Introduce a new interface which takes an {{IndexingContext}}
instance which provide access to such datapoints. This would require
some broader change
** Whereever the IndexEditorProvider is invoked it would need to check
if the instance implements new interface. If yes then new method needs
to be used

Overall it introduces noise.

* O2 - Here we can introduce such data points as part of callback
interface. With this we would need to implement such methods in places
where code constructs the callback

* O3 - Make a backward incompatible change and just modify the
existing interface and adapt the various implementation

I am in favour of going for O3 and make this backward compatible change

Thoughts?

Chetan Mehrotra

Re: Provide a way to pass indexing related state to IndexEditorProvider (OAK-4642)

[Chetan Mehrotra <ch...@gmail.com>]

I have updated OAK-4642 with one more option.

===
O4 - Similar to O2 but here instead of modifying the existing
IndexUpdateCallback we can introduce a new interface
ContextualCallback which extends IndexUpdateCallback and provide
access to IndexingContext. Editor provider implementation can then
check if the callback implements this new interface and then cast it
and access the context. So only those client which are interested in
new capability make use of this
===

So provide your feedback there or in this thread
Chetan Mehrotra


On Thu, Aug 4, 2016 at 12:35 PM, Chetan Mehrotra
<ch...@gmail.com> wrote:
> Hi Team,
>
> As a follow up to previous mail around "Using same index definition
> for both async and sync indexing" wanted to discuss the next step. We
> need to provide a way to pass indexing related state to
> IndexEditorProvider (OAK-4642)
>
> Over the period of time I have seen need for extra state like
>
> 1. reindexing - Currently the index implementation use some heuristic
>     like check before root state being empty to determine if they are
> running in reindexing mode
> 2. indexing mode - sync or async
> 3. index path of the index (see OAK-4152)
> 4. CommitInfo (see OAK-4640)
>
> For #1 and #3 we have done some kind of workaround but it would be
> better to have a first class support for that.
>
> So we would need to introduce some sort of IndexingContext and have
> the api for IndexEditorProvider like below
>
> =========
>     @CheckForNull
>     Editor getIndexEditor(
>             @Nonnull String type, @Nonnull NodeBuilder definition,
>             @Nonnull NodeState root,
>             @Nonnull IndexingContext context) throws CommitFailedException;
> =========
>
> To introduce such a change I see 3 options
>
> * O1 - Introduce a new interface which takes an {{IndexingContext}}
> instance which provide access to such datapoints. This would require
> some broader change
> ** Whereever the IndexEditorProvider is invoked it would need to check
> if the instance implements new interface. If yes then new method needs
> to be used
>
> Overall it introduces noise.
>
> * O2 - Here we can introduce such data points as part of callback
> interface. With this we would need to implement such methods in places
> where code constructs the callback
>
> * O3 - Make a backward incompatible change and just modify the
> existing interface and adapt the various implementation
>
> I am in favour of going for O3 and make this backward compatible change
>
> Thoughts?
>
> Chetan Mehrotra