Re: Requirement to support multiple NodeStore instance in same setup (OAK-4490)
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 wrote: > On 22/06/16 12:21, "Chetan Mehrotra" wrote: > >>On Tue, Jun 21, 2016 at 4:52 PM, Julian Sedding >>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)
On 22/06/16 12:21, "Chetan Mehrotra" wrote: >On Tue, Jun 21, 2016 at 4:52 PM, Julian Sedding >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)
On Tue, Jun 21, 2016 at 4:52 PM, Julian Sedding 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)
Hi, On Tue, Jun 21, 2016 at 9:03 AM, Chetan Mehrotra 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
Re: Requirement to support multiple NodeStore instance in same setup (OAK-4490)
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 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
Requirement to support multiple NodeStore instance in same setup (OAK-4490)
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