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

[Hardy Ferentschik]

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

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 496 bytes
Desc: not available
Url : http://lists.jboss.org/pipermail/hibernate-dev/attachments/20160427/9833a67f/attachment-0001.bin