12:04 a.m.
The "Service" and "ServiceManager" concepts in Hibernate Search have
a
specific meaning which is often misunderstood and/or abused, causing
trouble.
They also changed over time: victim of two major refactorings which
evolved the purpose and stretching its intent
So I'll change the definition again :-)
But hopefully clarifying it, so here is a draft of the rules which I
plan to both implement and document carefully on the javadoc, with
some comments to highlight what is changing:
# A service type is identified by a Class: a Service interface
Nothing new here. Yet: worth stressing that this implies that only one
implementation will be around.
# There can only be ONE IMPLEMENTATION of a service type used by the
whole Hibernate Search instance
In other words: it was not a good idea to have the
LuceneWorkSerializer to be modelled as a Service, back when we
supposedly could use a different serialization strategy for a
different index.
Yet it is a good idea nowadays to have LuceneWorkSerializer extend
Service, as we dropped that level of flexibility. This implies that
there's a single type of serializer (at most) and it's totally fine to
expose this as:
SearchIntegrator#getLuceneWorkSerializer()
[this method doesn't exist yet, but I'm thinking of adding it for our
convenience and the following other points]
P.S. we're only maintaining - and bundling - a single Serializer
implementation so it's no surprise that we can handle only one.. yet
this implies people wanting to override it have to either hack our
bootstrap or physically remove our implementation.
# A Service implementation can be provided by having it injected at
bootstrap (i.e.
org.hibernate.search.cfg.spi.SearchConfiguration.getProvidedServices()
)
Not a new rule either: repeating for clarity. We call these "provided"
services.
# If a service isn't "provided", then we attempt to create one using
java.util.ServiceLoader
Currently this expects a single implementation to be available:
there's no way to pick which one if there are multiple implementations
on the classpath.
I think we'll need to be able to pass a "hint" or similar to the
requestService to allow expression of preferences, handle shortnames,
etc.. a proposal for that will follow when there will be need: at this
point it's important to clarify the limitation, as this expresses what
a Service is not able to model today.
Currently implementations are looked up "on demand". I plan to allow
"pre-initialize" services as it removes some trouble; these components
could have convenience getters, not least to remove the concurrency
overhead.
Remember that since there's only one implementation for a given type
around, there's no reason to not do this: the intent of the Service
contract is to allow people to inject a customized implementation.
# If a Service implementation also implements Startable, or Stoppable,
we'll invoke the respective methods once at start and/or at stop of
the Search instance - unless they are provided in which case they are
ignored.
The current javadoc suggests that it's illegal for a provided
implementation to also implement Startable and/or Stoppable; I don't
remember why that was, but today it seems unfitting: people might want
to extend one of our implementations, or reuse some of the
implementations normally auto-started but reuse them "by instance" by
providing them to multiple Search instances to save memory (we
actually have a need for this for Index Affinity in Infinispan).
The important concept which will survive, is that we don't start or
stop stuff which is provided as that's clearly responsibility of
another component.
# All non-provided Services will be stopped once, and only once as
final step when the SearchIntegrator is stopped.
This is a significant difference with today's code: we expect the
Service consumers to "open / close", hopefully in a finally block, to
the point that Gunnar enhanced it to at least allow AutoClose
semantics.
Yet, I don't want runtime code to open and close these frequently as
it has been a bottleneck in the past.
It also led to the creations of issues like HSEARCH-1589 : we might
start/stop the same service frequently, and need to improve with
reference counters.
I suspect that historically the reasoning was to make sure that the
order of teardown would follow the inverse order of bootstrap as
components would cleanup after themselves, but having clarified that
Service instances are unique globally, should also imply that their
state doesn't depend on other Services. So the teardown order doesn't
matter anymore.. we'll start one for each, but only close it at
shutdown.
# Hierarchy?
We've talked about global components so far. It's clear that the
IndexManager has a central role in the overall architecture, as we
tend to allow per-index customisations. Or per-family customisations,
as suggested in my previous email.
An example which affects Service:
The "JestClient client" [the Service we use to connect to
Elasticsearch] could be considered a good fit for being a "Service" as
this allows people to override the client implementation and/or inject
a pre-configured instance.. yet it's not a good fit if we want to
allow people to connect to different hosts for different indexes.
I don't plan to implement the hierarchical ServiceManager right now,
but proposing it already so that we can agree on the above cleanups in
contract, with the perspective that there are cleaner solutions also
for the scoped use case.
Implementing these changes resolves or obsoletes at least 10 JIRA
issues in one shot..
Thanks,
Sanne
11:33 a.m.
This is an interesting sum up.
Shouldn't we keep these thoughts in a separate document for developer?
Together with some higher level diagrams of the architecture, maybe.
On Tue, Apr 26, 2016 at 11:04 PM, Sanne Grinovero <sanne(a)hibernate.org>
wrote:
The "Service" and "ServiceManager" concepts in
Hibernate Search have a
specific meaning which is often misunderstood and/or abused, causing
trouble.
They also changed over time: victim of two major refactorings which
evolved the purpose and stretching its intent
So I'll change the definition again :-)
But hopefully clarifying it, so here is a draft of the rules which I
plan to both implement and document carefully on the javadoc, with
some comments to highlight what is changing:
# A service type is identified by a Class: a Service interface
Nothing new here. Yet: worth stressing that this implies that only one
implementation will be around.
# There can only be ONE IMPLEMENTATION of a service type used by the
whole Hibernate Search instance
In other words: it was not a good idea to have the
LuceneWorkSerializer to be modelled as a Service, back when we
supposedly could use a different serialization strategy for a
different index.
Yet it is a good idea nowadays to have LuceneWorkSerializer extend
Service, as we dropped that level of flexibility. This implies that
there's a single type of serializer (at most) and it's totally fine to
expose this as:
SearchIntegrator#getLuceneWorkSerializer()
[this method doesn't exist yet, but I'm thinking of adding it for our
convenience and the following other points]
P.S. we're only maintaining - and bundling - a single Serializer
implementation so it's no surprise that we can handle only one.. yet
this implies people wanting to override it have to either hack our
bootstrap or physically remove our implementation.
# A Service implementation can be provided by having it injected at
bootstrap (i.e.
org.hibernate.search.cfg.spi.SearchConfiguration.getProvidedServices()
)
Not a new rule either: repeating for clarity. We call these "provided"
services.
# If a service isn't "provided", then we attempt to create one using
java.util.ServiceLoader
Currently this expects a single implementation to be available:
there's no way to pick which one if there are multiple implementations
on the classpath.
I think we'll need to be able to pass a "hint" or similar to the
requestService to allow expression of preferences, handle shortnames,
etc.. a proposal for that will follow when there will be need: at this
point it's important to clarify the limitation, as this expresses what
a Service is not able to model today.
Currently implementations are looked up "on demand". I plan to allow
"pre-initialize" services as it removes some trouble; these components
could have convenience getters, not least to remove the concurrency
overhead.
Remember that since there's only one implementation for a given type
around, there's no reason to not do this: the intent of the Service
contract is to allow people to inject a customized implementation.
# If a Service implementation also implements Startable, or Stoppable,
we'll invoke the respective methods once at start and/or at stop of
the Search instance - unless they are provided in which case they are
ignored.
The current javadoc suggests that it's illegal for a provided
implementation to also implement Startable and/or Stoppable; I don't
remember why that was, but today it seems unfitting: people might want
to extend one of our implementations, or reuse some of the
implementations normally auto-started but reuse them "by instance" by
providing them to multiple Search instances to save memory (we
actually have a need for this for Index Affinity in Infinispan).
The important concept which will survive, is that we don't start or
stop stuff which is provided as that's clearly responsibility of
another component.
# All non-provided Services will be stopped once, and only once as
final step when the SearchIntegrator is stopped.
This is a significant difference with today's code: we expect the
Service consumers to "open / close", hopefully in a finally block, to
the point that Gunnar enhanced it to at least allow AutoClose
semantics.
Yet, I don't want runtime code to open and close these frequently as
it has been a bottleneck in the past.
It also led to the creations of issues like HSEARCH-1589 : we might
start/stop the same service frequently, and need to improve with
reference counters.
I suspect that historically the reasoning was to make sure that the
order of teardown would follow the inverse order of bootstrap as
components would cleanup after themselves, but having clarified that
Service instances are unique globally, should also imply that their
state doesn't depend on other Services. So the teardown order doesn't
matter anymore.. we'll start one for each, but only close it at
shutdown.
# Hierarchy?
We've talked about global components so far. It's clear that the
IndexManager has a central role in the overall architecture, as we
tend to allow per-index customisations. Or per-family customisations,
as suggested in my previous email.
An example which affects Service:
The "JestClient client" [the Service we use to connect to
Elasticsearch] could be considered a good fit for being a "Service" as
this allows people to override the client implementation and/or inject
a pre-configured instance.. yet it's not a good fit if we want to
allow people to connect to different hosts for different indexes.
I don't plan to implement the hierarchical ServiceManager right now,
but proposing it already so that we can agree on the above cleanups in
contract, with the perspective that there are cleaner solutions also
for the scoped use case.
Implementing these changes resolves or obsoletes at least 10 JIRA
issues in one shot..
Thanks,
Sanne
_______________________________________________
hibernate-dev mailing list
hibernate-dev(a)lists.jboss.org
https://lists.jboss.org/mailman/listinfo/hibernate-dev
2:48 p.m.
On 27 April 2016 at 10:33, Davide D'Alto <daltodavide(a)gmail.com> wrote:
This is an interesting sum up.
Shouldn't we keep these thoughts in a separate document for developer?
Together with some higher level diagrams of the architecture, maybe.
I indeed wrote it as you expressed need for an high-level overview of Search.
I can add a distilled version to our wiki and try to incorporate the
feedback, but I was hoping we'd avoid the direction which might bring
us to UML age :) .. maybe a clarification on the javadocs and contract
methods would suffice?
12:35 p.m.
Hey Sanne,
great write-up!
2016-04-27 0:04 GMT+02:00 Sanne Grinovero <sanne(a)hibernate.org>:
The "Service" and "ServiceManager" concepts in
Hibernate Search have a
specific meaning which is often misunderstood and/or abused, causing
trouble.
They also changed over time: victim of two major refactorings which
evolved the purpose and stretching its intent
So I'll change the definition again :-)
But hopefully clarifying it, so here is a draft of the rules which I
plan to both implement and document carefully on the javadoc, with
some comments to highlight what is changing:
# A service type is identified by a Class: a Service interface
Nothing new here. Yet: worth stressing that this implies that only one
implementation will be around.
# There can only be ONE IMPLEMENTATION of a service type used by the
whole Hibernate Search instance
In other words: it was not a good idea to have the
LuceneWorkSerializer to be modelled as a Service, back when we
supposedly could use a different serialization strategy for a
different index.
Yet it is a good idea nowadays to have LuceneWorkSerializer extend
Service, as we dropped that level of flexibility. This implies that
there's a single type of serializer (at most) and it's totally fine to
expose this as:
SearchIntegrator#getLuceneWorkSerializer()
[this method doesn't exist yet, but I'm thinking of adding it for our
convenience and the following other points]
It's not clear to me when you'd provide such convenience getter and when
not. What's the criteria for it? It appears you consider different
categories of services?
Personally, I'd like if each service would obtain the other services it
depends on through its constructor. Then it can store the dependencies in
(final) fields and reference them as needed without any sort of look-up.
Services would not have to deal with the service manager anymore, they'd
just get those services they need. One thing I dislike about the current
approach is that you cannot make services fully immutable via final fields
if you depend on other services.
P.S. we're only maintaining - and bundling - a single Serializer
implementation so it's no surprise that we can handle only one.. yet
this implies people wanting to override it have to either hack our
bootstrap or physically remove our implementation.
# A Service implementation can be provided by having it injected at
bootstrap (i.e.
org.hibernate.search.cfg.spi.SearchConfiguration.getProvidedServices()
)
Not a new rule either: repeating for clarity. We call these "provided"
services.
# If a service isn't "provided", then we attempt to create one using
java.util.ServiceLoader
Currently this expects a single implementation to be available:
there's no way to pick which one if there are multiple implementations
on the classpath.
I think we'll need to be able to pass a "hint" or similar to the
requestService to allow expression of preferences, handle shortnames,
etc.. a proposal for that will follow when there will be need: at this
point it's important to clarify the limitation, as this expresses what
a Service is not able to model today.
+1 for shaping requirements around this once it's needed. Atm. it appears a
bit blurry to me.
Currently implementations are looked up "on demand". I plan to allow
"pre-initialize" services as it removes some trouble; these components
could have convenience getters, not least to remove the concurrency
overhead.
Remember that since there's only one implementation for a given type
around, there's no reason to not do this: the intent of the Service
contract is to allow people to inject a customized implementation.
Sounds good.
# If a Service implementation also implements Startable, or Stoppable,
we'll invoke the respective methods once at start and/or at stop
of
the Search instance - unless they are provided in which case they are
ignored.
The current javadoc suggests that it's illegal for a provided
implementation to also implement Startable and/or Stoppable; I don't
remember why that was, but today it seems unfitting: people might want
to extend one of our implementations, or reuse some of the
implementations normally auto-started but reuse them "by instance" by
providing them to multiple Search instances to save memory (we
actually have a need for this for Index Affinity in Infinispan).
The important concept which will survive, is that we don't start or
stop stuff which is provided as that's clearly responsibility of
another component.
Why is this? Above you say it's unfitting? I'm not quite clear on the
proposed direction.
# All non-provided Services will be stopped once, and only once as
final step when the SearchIntegrator is stopped.
This is a significant difference with today's code: we expect the
Service consumers to "open / close", hopefully in a finally block, to
the point that Gunnar enhanced it to at least allow AutoClose
semantics.
Yet, I don't want runtime code to open and close these frequently as
it has been a bottleneck in the past.
+1 for setting up the graph once and avoiding all the request/release
hassle. Ideally with "constructor injection" as described above.
It also led to the creations of issues like HSEARCH-1589 : we might
start/stop the same service frequently, and need to improve with
reference counters.
I suspect that historically the reasoning was to make sure that the
order of teardown would follow the inverse order of bootstrap as
components would cleanup after themselves, but having clarified that
Service instances are unique globally, should also imply that their
state doesn't depend on other Services. So the teardown order doesn't
matter anymore.. we'll start one for each, but only close it at
shutdown.
I'm not following on that one. Provided one service uses another and stores
it in a field, tear down order may matter indeed (if a service is using
another one in its close() logic). It'd be resolved easily if circles are
forbidden.
# Hierarchy?
We've talked about global components so far. It's clear that the
IndexManager has a central role in the overall architecture, as we
tend to allow per-index customisations. Or per-family customisations,
as suggested in my previous email.
An example which affects Service:
The "JestClient client" [the Service we use to connect to
Elasticsearch] could be considered a good fit for being a "Service" as
this allows people to override the client implementation and/or inject
a pre-configured instance.. yet it's not a good fit if we want to
allow people to connect to different hosts for different indexes.
I don't plan to implement the hierarchical ServiceManager right now,
but proposing it already so that we can agree on the above cleanups in
contract, with the perspective that there are cleaner solutions also
for the scoped use case.
It's an interesting idea. The granularity of the hierarchy would have to be
discussed. Would you e.g. have one service manager per index which inherits
the global one?
Implementing these changes resolves or obsoletes at least 10 JIRA
issues in one shot..
What is your plan for this? Which ones do you plan to address as of 5.6,
(5.7), 6 or farther in the future?
Thanks,
Sanne
_______________________________________________
hibernate-dev mailing list
hibernate-dev(a)lists.jboss.org
https://lists.jboss.org/mailman/listinfo/hibernate-dev
3:48 p.m.
On 27 April 2016 at 11:35, Gunnar Morling <gunnar(a)hibernate.org> wrote:
Hey Sanne,
great write-up!
2016-04-27 0:04 GMT+02:00 Sanne Grinovero <sanne(a)hibernate.org>:
>
> The "Service" and "ServiceManager" concepts in Hibernate Search
have a
> specific meaning which is often misunderstood and/or abused, causing
> trouble.
>
> They also changed over time: victim of two major refactorings which
> evolved the purpose and stretching its intent
>
> So I'll change the definition again :-)
> But hopefully clarifying it, so here is a draft of the rules which I
> plan to both implement and document carefully on the javadoc, with
> some comments to highlight what is changing:
>
>
>
> # A service type is identified by a Class: a Service interface
>
> Nothing new here. Yet: worth stressing that this implies that only one
> implementation will be around.
>
> # There can only be ONE IMPLEMENTATION of a service type used by the
> whole Hibernate Search instance
>
> In other words: it was not a good idea to have the
> LuceneWorkSerializer to be modelled as a Service, back when we
> supposedly could use a different serialization strategy for a
> different index.
>
> Yet it is a good idea nowadays to have LuceneWorkSerializer extend
> Service, as we dropped that level of flexibility. This implies that
> there's a single type of serializer (at most) and it's totally fine to
> expose this as:
>
> SearchIntegrator#getLuceneWorkSerializer()
>
> [this method doesn't exist yet, but I'm thinking of adding it for our
> convenience and the following other points]
It's not clear to me when you'd provide such convenience getter and when
not. What's the criteria for it? It appears you consider different
categories of services?
Indeed this bothers me: "Service" is a general purpose abstraction,
and in practice several components could be refactored to become a
Service, while also many Service(s) could be refactored to not be one.
Drawing a well defined line is not practical.
Take a look at the ServiceIntegrator: why do we have a
getErrorHandler(), and why is that not a Service? (hint: historical
reasons)
Even better, look at ImmutableSearchFactory: it's a placeholder for
many things which could be services.
Why is TimeoutExceptionFactory not a Service?
The problem is twofold:
- it's unclear
- we keep discussing these stuff, while it has very low value to the end users
So my take is going to be a pragmatic one: having getters on
`ServiceIntegrator` is an approach which worked very well so far and I
don't feel the need to be moving everything into a "Services" model.
I do like the Services abstraction as a way to allow SPIs (think
Infinispan Query, but also beyond) to customize and override internal
components.
Another benefit of Services is that one custom component might need to
lookup another custom component, and this code living outside of the
`engine` module you can't add a getter for all your custom stuff.
Example: see the services defined by the JGroups backend:
`NodeSelectorService` and `MessageSenderService` are very specific to
the design of this backend, and depend on JGroups.
Clearly something like a "ClassLoaderService" is useful to a wide
scope and having a simple getter is just simpler.
What I think was a mistake, is to allow exposing the ServiceManager to
SPI as it is prone to abuse: while we know that a FieldBridgeFactory
will need the ability to instantiate classes by name, that means it
needs access to a ClassLoaderService but today we're passing it the
whole ServiceManager ; this opens the door for lookup of other
services, and we get quickly entangled in complex dependency graphs.
There are some cases of valid inter-service dependencies, but these
are very limited (e.g. the above JGroups services).
So I'm not proposing to scrap this capability from the ServiceManager
as it is a very useful thing, yet I don't see why e.g. the
FieldBridgeFactory needs more than one trivial line of code to load
the classes it needs.
Alternatively, we could see to refactor those few Services which need
additional services to not need this anymore - yet I don't see why
since we have a beautiful ServiceManager which can do such things :)
Personally, I'd like if each service would obtain the other
services it
depends on through its constructor. Then it can store the dependencies in
(final) fields and reference them as needed without any sort of look-up.
Services would not have to deal with the service manager anymore, they'd
just get those services they need. One thing I dislike about the current
approach is that you cannot make services fully immutable via final fields
if you depend on other services.
It's a great idea. We should do that, but again since we don't have
much time I'm forced to keep it practical :-(
If you have time to propose a KISS patch (we're not in the business of
DI frameworks) I'd love to apply it but remember our priority is
Elasticsearch at this point.. I merely had to start this thread to
clarify were I'd like to go with e.g. the JestClient and the
SerializerService.
> P.S. we're only maintaining - and bundling - a single
Serializer
> implementation so it's no surprise that we can handle only one.. yet
> this implies people wanting to override it have to either hack our
> bootstrap or physically remove our implementation.
>
> # A Service implementation can be provided by having it injected at
> bootstrap (i.e.
> org.hibernate.search.cfg.spi.SearchConfiguration.getProvidedServices()
> )
>
> Not a new rule either: repeating for clarity. We call these "provided"
> services.
>
> # If a service isn't "provided", then we attempt to create one using
> java.util.ServiceLoader
>
> Currently this expects a single implementation to be available:
> there's no way to pick which one if there are multiple implementations
> on the classpath.
> I think we'll need to be able to pass a "hint" or similar to the
> requestService to allow expression of preferences, handle shortnames,
> etc.. a proposal for that will follow when there will be need: at this
> point it's important to clarify the limitation, as this expresses what
> a Service is not able to model today.
+1 for shaping requirements around this once it's needed. Atm. it appears a
bit blurry to me.
There was no request for this, but to unblur the problem: say a power
user wants to plug in its own SerializerService implementation, and
bundles it with its app to deploy on WildFly. He'll get an exception
as now there are two providers for the same service, and he can't
easily "remove" the other one as it's provided by the app server.
He can't override the provided services either, as the JPA context is
booted by the app server.
Would be nicer to have a property to allow picking which
implementation to use, rather than throw exceptions.
>
>
> Currently implementations are looked up "on demand". I plan to allow
> "pre-initialize" services as it removes some trouble; these components
> could have convenience getters, not least to remove the concurrency
> overhead.
> Remember that since there's only one implementation for a given type
> around, there's no reason to not do this: the intent of the Service
> contract is to allow people to inject a customized implementation.
Sounds good.
> # If a Service implementation also implements Startable, or Stoppable,
> we'll invoke the respective methods once at start and/or at stop of
> the Search instance - unless they are provided in which case they are
> ignored.
>
> The current javadoc suggests that it's illegal for a provided
> implementation to also implement Startable and/or Stoppable; I don't
> remember why that was, but today it seems unfitting: people might want
> to extend one of our implementations, or reuse some of the
> implementations normally auto-started but reuse them "by instance" by
> providing them to multiple Search instances to save memory (we
> actually have a need for this for Index Affinity in Infinispan).
> The important concept which will survive, is that we don't start or
> stop stuff which is provided as that's clearly responsibility of
> another component.
Why is this? Above you say it's unfitting? I'm not quite clear on the
proposed direction.
It's very simple: say I want to extend JestClient and use my custom
implementation, e.g. I am experimenting with some http pooling
capabilities which Search doesn't have.
I can ignore the start, and initialize it my own way.. but Search will
refuse this Service instance as it's also inheriting @Startable.
I'd prefer to allow people to inject their own stuff without being
overly strict; BTW the JestClient service isn't using an interface to
identify itself so users will need to extend it .. it's a limitation I
think we need to fix, we've always been very friendly to allow people
to override components.
> # All non-provided Services will be stopped once, and only once as
> final step when the SearchIntegrator is stopped.
>
> This is a significant difference with today's code: we expect the
> Service consumers to "open / close", hopefully in a finally block, to
> the point that Gunnar enhanced it to at least allow AutoClose
> semantics.
> Yet, I don't want runtime code to open and close these frequently as
> it has been a bottleneck in the past.
+1 for setting up the graph once and avoiding all the request/release
hassle. Ideally with "constructor injection" as described above.
+1
>
> It also led to the creations of issues like HSEARCH-1589 : we might
> start/stop the same service frequently, and need to improve with
> reference counters.
>
> I suspect that historically the reasoning was to make sure that the
> order of teardown would follow the inverse order of bootstrap as
> components would cleanup after themselves, but having clarified that
> Service instances are unique globally, should also imply that their
> state doesn't depend on other Services. So the teardown order doesn't
> matter anymore.. we'll start one for each, but only close it at
> shutdown.
I'm not following on that one. Provided one service uses another and stores
it in a field, tear down order may matter indeed (if a service is using
another one in its close() logic). It'd be resolved easily if circles are
forbidden.
Right those services depending on others might still need it. We might
still need reference counters, or at least differentiate for those
services which are frequently looked up and could bypass them.
> # Hierarchy?
>
> We've talked about global components so far. It's clear that the
> IndexManager has a central role in the overall architecture, as we
> tend to allow per-index customisations. Or per-family customisations,
> as suggested in my previous email.
>
> An example which affects Service:
> The "JestClient client" [the Service we use to connect to
> Elasticsearch] could be considered a good fit for being a "Service" as
> this allows people to override the client implementation and/or inject
> a pre-configured instance.. yet it's not a good fit if we want to
> allow people to connect to different hosts for different indexes.
>
> I don't plan to implement the hierarchical ServiceManager right now,
> but proposing it already so that we can agree on the above cleanups in
> contract, with the perspective that there are cleaner solutions also
> for the scoped use case.
It's an interesting idea. The granularity of the hierarchy would have to be
discussed. Would you e.g. have one service manager per index which inherits
the global one?
> Implementing these changes resolves or obsoletes at least 10 JIRA
> issues in one shot..
What is your plan for this? Which ones do you plan to address as of 5.6,
(5.7), 6 or farther in the future?
Implementing this plan has no visible benefit to end users, other than
say be able to override the JestClient but that can be achieved by
implementing just one step.
So, purely practical oriented:
1) make you all aware of the limitations of Service to get the new
code in the right direction
2) simplify things which give us benefit in new coding
3) move to the above plan iteratively, prioritizing what we need e.g.
I need https://github.com/hibernate/hibernate-search/pull/1082 for
various other reasons so I did it already.
Thanks,
Sanne
2:34 p.m.
Hi,
Yet it is a good idea nowadays to have LuceneWorkSerializer extend
Service, as we dropped that level of flexibility. This implies that
there's a single type of serializer (at most)
Ok
and it's totally fine to expose this as:
SearchIntegrator#getLuceneWorkSerializer()
How is that related to the service discussion and why do you want to offer it.
What is the benefit? The SearchIntegrator already gives access to the ServiceManager
from where the serializer can be retrieved. Why exposing an explicit getter?
Are you going to add getters for all services? Would this getter do something
different than accessing the service manager?
On a tangent, the whole serialization framework would need an overhaul, especially
the Serializer and Deserializer interfaces. They are very tightly coupled on how
Avro works and tbh they are quite convoluted.
# A Service implementation can be provided by having it injected at
bootstrap (i.e.
org.hibernate.search.cfg.spi.SearchConfiguration.getProvidedServices()
)
Not a new rule either: repeating for clarity. We call these "provided"
services.
One thing, I was never happy about was that provided services get a sopecial treatment
in the service manager. They can returned first, provided they match the requested
service
type and they are don't get "released" (meaning they don't take part in
the reference counting).
I think services which are managed by the service manager should be treated equal.
Maybe that becomes possible now that you consider dropping the reference counting, which
I think is a good idea.
Currently this expects a single implementation to be available:
there's no way to pick which one if there are multiple implementations
on the classpath.
We have been discussing to extend the implementation once the needs arises to
allow multiple service implementation to be available on the classpath in which
case the user must provide additional configuration to allow to select the correct
implementation. So far there was no need to add this, but maybe now!?
Currently implementations are looked up "on demand". I plan
to allow
"pre-initialize" services as it removes some trouble;
What exactly does that mean? Didn't we have this discussion before?
Which troubles are you talking about?
not least to remove the concurrency overhead.
This argument could of course not be missed out ;-)
# If a Service implementation also implements Startable, or
Stoppable,
we'll invoke the respective methods once at start and/or at stop of
the Search instance - unless they are provided in which case they are
ignored.
This is something I never full understood. If a provided services does not
want to get started/stopped would it then just not implement these interfaces?
The current javadoc suggests that it's illegal for a provided
implementation to also implement Startable and/or Stoppable; I don't
remember why that was, but today it seems unfitting
Illegal might be the wrong term. It probably should say that it should not
implement them if it does not want to have these callbacks invoked. Which is
pretty much obvious. AFAIR, there was a discussion that we never should start/stop
a provided service and hence this wording, but I think in code we never enforced it.
The important concept which will survive, is that we don't start
or
stop stuff which is provided as that's clearly responsibility of
another component.
Then it should not implement Startable/Stopable and if it does, it should be
noops.
# All non-provided Services will be stopped once, and only once as
final step when the SearchIntegrator is stopped.
+1
This is a significant difference with today's code: we expect
the
Service consumers to "open / close", hopefully in a finally block, to
the point that Gunnar enhanced it to at least allow AutoClose
semantics.
I think this is still a good practice. I'd keep the API like this,
but don't do reference counting and stop the service each time the
count reaches 0. Instead, as you say, stop all services at a final
shutdown.
I don't plan to implement the hierarchical ServiceManager right
now,
but proposing it already so that we can agree on the above cleanups in
contract, with the perspective that there are cleaner solutions also
for the scoped use case.
What do you even understand under a hierarchical index manager?
Implementing these changes resolves or obsoletes at least 10 JIRA
issues in one shot..
Really?
--Hardy
4:13 p.m.
I can't mention Service w/o getting your attention :)
On 27 April 2016 at 13:34, Hardy Ferentschik <hardy(a)hibernate.org> wrote:
Hi,
> Yet it is a good idea nowadays to have LuceneWorkSerializer extend
> Service, as we dropped that level of flexibility. This implies that
> there's a single type of serializer (at most)
Ok
> and it's totally fine to expose this as:
> SearchIntegrator#getLuceneWorkSerializer()
How is that related to the service discussion and why do you want to offer it.
What is the benefit? The SearchIntegrator already gives access to the ServiceManager
from where the serializer can be retrieved. Why exposing an explicit getter?
Are you going to add getters for all services? Would this getter do something
different than accessing the service manager?
I hope the reply to Gunnar qualifies here too: it's indeed a thin line
but it's equally unclear when something should (not) be a Service.
There are some cases in which one can only user the Service, so these
would never able to get a Getter.
On a tangent, the whole serialization framework would need an overhaul, especially
the Serializer and Deserializer interfaces. They are very tightly coupled on how
Avro works and tbh they are quite convoluted.
+1
I wouldn't mind allowing the "backendqueue" to make its own
arrangements, i.e. not mandate the contract anymore. We just need to
provide at least one implementation, but others - especially if not
interested in Avro - should be able to do something very different.
We're also working to remove the Apache Document as something exposed
to FieldBridge: by doing so, we could generate the actual Document
only at receive side, and not have to serialize Apache code anymore
but serialize an easier structure.
> # A Service implementation can be provided by having it injected at
> bootstrap (i.e.
> org.hibernate.search.cfg.spi.SearchConfiguration.getProvidedServices()
> )
>
> Not a new rule either: repeating for clarity. We call these "provided"
services.
One thing, I was never happy about was that provided services get a sopecial treatment
in the service manager. They can returned first, provided they match the requested
service
type and they are don't get "released" (meaning they don't take part in
the reference counting).
I think services which are managed by the service manager should be treated equal.
Maybe that becomes possible now that you consider dropping the reference counting, which
I think is a good idea.
We need a way to have people override an internal component by handing
in an instance: this was the very reason to create this Service
manager.
An example, is people running many instances of Search in the same JVM
(many apps in an app server?) they can share some instances, provided
we identify some components worth it.
I can't think of such a component consuming enough memory to warrant
doing this in our "traditional" use case (paired up with Hibernate
ORM) but there are several heavy weight components which could be
shared in the Infinispan use case.
> Currently this expects a single implementation to be available:
> there's no way to pick which one if there are multiple implementations
> on the classpath.
We have been discussing to extend the implementation once the needs arises to
allow multiple service implementation to be available on the classpath in which
case the user must provide additional configuration to allow to select the correct
implementation. So far there was no need to add this, but maybe now!?
There isn't specific need yet, I'm just pointing out to the newcomers
to be aware of this :)
> Currently implementations are looked up "on demand". I plan to allow
> "pre-initialize" services as it removes some trouble;
What exactly does that mean? Didn't we have this discussion before?
Which troubles are you talking about?
> not least to remove the concurrency overhead.
This argument could of course not be missed out ;-)
> # If a Service implementation also implements Startable, or Stoppable,
> we'll invoke the respective methods once at start and/or at stop of
> the Search instance - unless they are provided in which case they are
> ignored.
This is something I never full understood. If a provided services does not
want to get started/stopped would it then just not implement these interfaces?
Yes there are ways around, but it seems more practical to just allow it.
As mentioned in my other reply, it's not always easy to implement it
the "clean way" you suggest - not least if someone is skilled enough
to want to override components, should be able to figure the
limitations out.
> The current javadoc suggests that it's illegal for a provided
> implementation to also implement Startable and/or Stoppable; I don't
> remember why that was, but today it seems unfitting
Illegal might be the wrong term. It probably should say that it should not
implement them if it does not want to have these callbacks invoked. Which is
pretty much obvious. AFAIR, there was a discussion that we never should start/stop
a provided service and hence this wording, but I think in code we never enforced it.
It currently enforces it, and we even have unit tests to make sure of it :)
> The important concept which will survive, is that we don't start or
> stop stuff which is provided as that's clearly responsibility of
> another component.
Then it should not implement Startable/Stopable and if it does, it should be
noops.
> # All non-provided Services will be stopped once, and only once as
> final step when the SearchIntegrator is stopped.
+1
> This is a significant difference with today's code: we expect the
> Service consumers to "open / close", hopefully in a finally block, to
> the point that Gunnar enhanced it to at least allow AutoClose
> semantics.
I think this is still a good practice. I'd keep the API like this,
but don't do reference counting and stop the service each time the
count reaches 0. Instead, as you say, stop all services at a final
shutdown.
Yea, I think the API is fitting.
The main problem I'm seeing is confusion about when it's appropriate
to make one..
The cases which I don't like the open/close is when some code blocks
need a service within the scope of a single method call.
As you say, long-lived components should grab a reference and reuse it
over time, then finally release it, but e.g. ClassLoaderService is
often needed "one shot" e.g. to setup a component.. which is were I
think a simple getter - which also bypasses reference counting - would
be more useful.
> I don't plan to implement the hierarchical ServiceManager right now,
> but proposing it already so that we can agree on the above cleanups in
> contract, with the perspective that there are cleaner solutions also
> for the scoped use case.
What do you even understand under a hierarchical index manager?
> Implementing these changes resolves or obsoletes at least 10 JIRA
> issues in one shot..
Really?
Ok, just 8 after closer inspection :)
--Hardy
2920
days inactive
2921
days old
6 comments
4 participants
participants (4)
-
Davide D'Alto -
Gunnar Morling -
Hardy Ferentschik -
Sanne Grinovero