Requirement to support multiple NodeStore instance in same setup (OAK-4490)

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

Hi Team,

As part of OAK-4180 feature around using another NodeStore as a local
cache for a remote Document store I would need to register another
NodeStore instance (for now a SegmentNodeStore - OAK-4490) with the
OSGi service registry.

This instance would then be used by SecondaryStoreCacheService to save
NodeState under certain paths locally and use it later for reads.

With this change we would have a situation where there would be
multiple NodeStore instance in same service registry. This can confuse
some component which have a dependency on NodeStore as a reference and
we need to ensure they bind to correct NodeStore instance.

Proposal A - Use a 'type' service property to distinguish
==========================================

Register the NodeStore with a 'type' property. For now the value can
be 'primary' or 'secondary'. When any component registers the
NodeStore it also provides the type property.

On user side the reference needs to provide which type of NodeStore it
needs to bound

This would ensure that user of NodeStore get bound to correct type.

if we use service.ranking then it can cause a race condition where the
secondary instance may get bound untill primary comes up

Looking for feedback on what approach to take

Chetan Mehrotra

Re: Requirement to support multiple NodeStore instance in same setup (OAK-4490)

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

Okie would go with SecondaryNodeStoreProvider approach and also have a
role property for that. For now this interface would live in plugins
package and exported as it needs to be used in oak-segment and
oak-segment-tar. Later we can decide if we need to move it to SPI
package as supported extension point
Chetan Mehrotra


On Wed, Jun 22, 2016 at 4:44 PM, Stefan Egli <st...@apache.org> wrote:
> On 22/06/16 12:21, "Chetan Mehrotra" <ch...@gmail.com> wrote:
>
>>On Tue, Jun 21, 2016 at 4:52 PM, Julian Sedding <js...@gmail.com>
>>wrote:
>>> Not exposing the secondary NodeStore in the service registry would be
>>> backwards compatible. Introducing the "type" property potentially
>>> breaks existing consumers, i.e. is not backwards compatible.
>>
>>I had similar concern so proposed a new interface as part of OAK-4369.
>>However later with further discussion realized that we might have
>>similar requirement going forward i.e. presence of multiple NodeStore
>>impl so might be better to make setup handle such case.
>>
>>So at this stage we have 2 options
>>
>>1. Use a new interface to expose such "secondary" NodeStore
>>2. OR Use a new service property to distinguish between different roles
>>
>>Not sure which one to go. May be we go for merged i.e. have a new
>>interface as in #1 but also mandate that it provides its "role/type"
>>as a service property to allow client to select correct one
>>
>>Thoughts?
>
> If the 'SecondaryNodeStoreProvider' is a non-public interface which can
> later 'easily' be replaced with another mechanism, then for me this would
> sound more straight forward at this stage as it would not break any
> existing consumers (as mentioned by Julian).
>
> Perhaps once those 'other use cases going forward' of multiple NodeStores
> become more clear, then it might be more obvious as to how the
> generalization into perhaps a type property should look like.
>
> my 2cents,
> Cheers,
> Stefan
>
>

Re: Requirement to support multiple NodeStore instance in same setup (OAK-4490)

[Stefan Egli <st...@apache.org>]

On 22/06/16 12:21, "Chetan Mehrotra" <ch...@gmail.com> wrote:

>On Tue, Jun 21, 2016 at 4:52 PM, Julian Sedding <js...@gmail.com>
>wrote:
>> Not exposing the secondary NodeStore in the service registry would be
>> backwards compatible. Introducing the "type" property potentially
>> breaks existing consumers, i.e. is not backwards compatible.
>
>I had similar concern so proposed a new interface as part of OAK-4369.
>However later with further discussion realized that we might have
>similar requirement going forward i.e. presence of multiple NodeStore
>impl so might be better to make setup handle such case.
>
>So at this stage we have 2 options
>
>1. Use a new interface to expose such "secondary" NodeStore
>2. OR Use a new service property to distinguish between different roles
>
>Not sure which one to go. May be we go for merged i.e. have a new
>interface as in #1 but also mandate that it provides its "role/type"
>as a service property to allow client to select correct one
>
>Thoughts?

If the 'SecondaryNodeStoreProvider' is a non-public interface which can
later 'easily' be replaced with another mechanism, then for me this would
sound more straight forward at this stage as it would not break any
existing consumers (as mentioned by Julian).

Perhaps once those 'other use cases going forward' of multiple NodeStores
become more clear, then it might be more obvious as to how the
generalization into perhaps a type property should look like.

my 2cents,
Cheers,
Stefan

Re: Requirement to support multiple NodeStore instance in same setup (OAK-4490)

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

On Tue, Jun 21, 2016 at 4:52 PM, Julian Sedding <js...@gmail.com> wrote:
> Not exposing the secondary NodeStore in the service registry would be
> backwards compatible. Introducing the "type" property potentially
> breaks existing consumers, i.e. is not backwards compatible.

I had similar concern so proposed a new interface as part of OAK-4369.
However later with further discussion realized that we might have
similar requirement going forward i.e. presence of multiple NodeStore
impl so might be better to make setup handle such case.

So at this stage we have 2 options

1. Use a new interface to expose such "secondary" NodeStore
2. OR Use a new service property to distinguish between different roles

Not sure which one to go. May be we go for merged i.e. have a new
interface as in #1 but also mandate that it provides its "role/type"
as a service property to allow client to select correct one

Thoughts?

Chetan Mehrotra

Re: Requirement to support multiple NodeStore instance in same setup (OAK-4490)

[Julian Sedding <js...@gmail.com>]

Hi Chetan

I agree that we should not rely on the service.ranking for this. A
type property makes sense IMO.

On the other hand, do we really need to expose both NodeStores in the
service registry? The secondary (cache) NodeStore could also be
treated as an implementation detail of the DocumentNodeStore and
switched on/off via configuration. Of course the devil is in the
detail then - how to configure different BlobStores, cache sizes etc
of the secondary NodeStore?

Not exposing the secondary NodeStore in the service registry would be
backwards compatible. Introducing the "type" property potentially
breaks existing consumers, i.e. is not backwards compatible.

Regards
Julian


On Tue, Jun 21, 2016 at 9:03 AM, Chetan Mehrotra
<ch...@gmail.com> wrote:
> Hi Team,
>
> As part of OAK-4180 feature around using another NodeStore as a local
> cache for a remote Document store I would need to register another
> NodeStore instance (for now a SegmentNodeStore - OAK-4490) with the
> OSGi service registry.
>
> This instance would then be used by SecondaryStoreCacheService to save
> NodeState under certain paths locally and use it later for reads.
>
> With this change we would have a situation where there would be
> multiple NodeStore instance in same service registry. This can confuse
> some component which have a dependency on NodeStore as a reference and
> we need to ensure they bind to correct NodeStore instance.
>
> Proposal A - Use a 'type' service property to distinguish
> ==========================================
>
> Register the NodeStore with a 'type' property. For now the value can
> be 'primary' or 'secondary'. When any component registers the
> NodeStore it also provides the type property.
>
> On user side the reference needs to provide which type of NodeStore it
> needs to bound
>
> This would ensure that user of NodeStore get bound to correct type.
>
> if we use service.ranking then it can cause a race condition where the
> secondary instance may get bound untill primary comes up
>
> Looking for feedback on what approach to take
>
> Chetan Mehrotra

Re: Requirement to support multiple NodeStore instance in same setup (OAK-4490)

[Bertrand Delacretaz <bd...@apache.org>]

Hi,

On Tue, Jun 21, 2016 at 9:03 AM, Chetan Mehrotra
<ch...@gmail.com> wrote:
> ...Register the NodeStore with a 'type' property. For now the value can
> be 'primary' or 'secondary'...

Just a naming nitpick, "role" might be a better name for this, with
role = secondaryCacheStore in your case.

-Bertrand