[hibernate-dev] "Service" in Hibernate Search: history, lessons learned and rewrite

[Davide D'Alto]

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 at 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 at lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/hibernate-dev
>