[hibernate-dev] Centralized access to "bootstrap only" resources
Steve Ebersole
steve at hibernate.org
Thu Sep 1 08:02:12 EDT 2016
In order to validate that unifying the event listener impls between
"native" and "jpa" is actually workable I applied that idea on top of 5.2.
And it works. ANd since I did the work, I created a PR from it for us to:
1. review that specific set of changes (slightly different accounting
for ongoing 6.0 work)
2. consider incorporating it into 5.2
https://github.com/hibernate/hibernate-orm/pull/1541
On Thu, Sep 1, 2016 at 1:11 AM Gunnar Morling <gunnar at hibernate.org> wrote:
> > Do we want to change JpaIntegrator#integrate signature to pass its
> context
> > as a parameter object
>
> If you think that's the better approach, I'd say 6 is the right time to do
> it. Users (or integrators) will be prepared for this sort of breakage in a
> major.
>
> > do other projects supply custom
> > org.hibernate.service.spi.SessionFactoryServiceInitiator impls
>
> Yes, OGM does. Though I'm not too concerned about such change, it should
> be easy for us to adjust and at this point we don't aim
> for compatibility with several ORM (major) versions.
>
> 2016-09-01 4:09 GMT+02:00 Steve Ebersole <steve at hibernate.org>:
>
>> One contract I would need to adjust for this
>> is
>> org.hibernate.service.spi.SessionFactoryServiceInitiator#initiateService.
>> I can find all the implementations of that in ORM, but do other projects
>> supply custom org.hibernate.service.spi.SessionFactoryServiceInitiator
>> impls?
>>
>>
>> On Wed, Aug 31, 2016 at 5:18 AM andrea boriero <andrea at hibernate.org>
>> wrote:
>>
>> > I'm fine with combining native and JPA events handling, about the second
>> > point, ideally I would change the signature but due to the problems you
>> > listed I vote for the in-line solution.
>> >
>> > On 30 August 2016 at 19:20, Steve Ebersole <steve at hibernate.org> wrote:
>> >
>> >> Any thoughts on the JpaIntegrator parts of the discussion?
>> Specifically
>> >> there are 2 main considerations:
>> >>
>> >> 1. To change the Integrator#integrate contract - ideally, in
>> >> retrospect,
>> >
>> >
>> >> #integrate probably should have taken a "parameter object" to help
>> >> insulate
>> >> from these types of changes. But I wanted to get y'alls thoughts on
>> >> this
>> >> especially since this one potentially causes upgrade problems in
>> terms
>> >> of
>> >> applications or problems supporting multiple ORM versions in terms
>> >> of integrations.
>> >>
>> > 2. The alternative I mentioned was to move the
>> JpaIntegrator#integrate
>> >
>> >
>> >> functionality in-line with the building of the SessionFactory. This
>> >> has
>> >> some really nice benefits as discussed (like JPA callback support
>> from
>> >> native bootstrapping), but it has some challenges to handle as well
>> >> mainly
>> >> in terms of seamlessly combining the different Hibernate event
>> >> listeners
>> >> used to implement the native versus JPA behavior. The simple JPA
>> >> callback/listener case is pretty easy to support regardless. The
>> more
>> >> difficult ones are event listeners that implement event handling
>> >> differently () or the ones that cascade different actions depending
>> on
>> >> native/jpa bootstrapping (). I think even the latter bucket may be
>> >> easy to
>> >> handle leveraging SessionFactoryOptions#isJpaBootstrap inside the
>> >> listeners. The former bucket is really the one I am more concerned
>> >> with.
>> >> So let's look at this as 2 distinct questions:
>> >>
>> > 1. Do we want to combine event listeners for native and JPA
>> handling
>> >> of events?
>> >> 2. Do we want to change JpaIntegrator#integrate signature to pass
>> >> its
>> >
>> >
>> >> context as a parameter object in order to facilitate this? Or
>> do we
>> >> in-line the decisions/actions done in JpaIntegrator into
>> >> SessionFactory
>> >> init?
>> >>
>> >
>> >>
>> >> On Tue, Aug 30, 2016 at 8:50 AM Steve Ebersole <steve at hibernate.org>
>> >> wrote:
>> >>
>> >> > On Tue, Aug 30, 2016 at 6:27 AM Sanne Grinovero <sanne at hibernate.org
>> >
>> >> > wrote:
>> >> >
>> >> >> On 30 August 2016 at 10:09, Emmanuel Bernard <
>> emmanuel at hibernate.org>
>> >> >> wrote:
>> >> >> > I am not sure if that is still relevant but in the past, either
>> >> HSEARCH
>> >> >> > or HV were keeping the ReflectionManager around to use it at
>> runtime
>> >> >> > (either because metadata was loaded lazily or because of a reboot
>> of
>> >> the
>> >> >> > factories due to a configuration change.
>> >> >> >
>> >> >> > So we need to check that losing access to ReflectionManager after
>> SF
>> >> is
>> >> >> > created won't be problematic for these projects.
>> >> >>
>> >> >> In the "dynamic reconfiguration" case we create our own
>> >> >> ReflectionManager instance:
>> >> >> -
>> >> >>
>> >>
>> https://github.com/hibernate/hibernate-search/blob/fd4acb5d8f396201f5dccc89ba3cbc07becea08a/engine/src/main/java/org/hibernate/search/engine/impl/IncrementalSearchConfiguration.java#L26-L35
>> >> >
>> >> >
>> >> > Interesting that y'all do not specify classloading behavior there
>> (the
>> >> > ClassLoaderDelegate stuff I added to HCANN)...
>> >> >
>> >> >
>> >> >
>> >> >> Steve, we had a similar notion of "boot only available components"
>> in
>> >> >> Search but over time we started to have various "special needs" of
>> >> >> various other components holding a reference on these.
>> >> >> When I later tried to re-instate order, it was too late and we got
>> in
>> >> >> arguments like the API's intent not having been clear enough and too
>> >> >> much entanglement had happened.
>> >> >>
>> >> >
>> >> > Hard to say without specifics. I hate "general rules" :)
>> >> >
>> >> > So let's look at the specifics in terms of things I have moved to
>> >> > BootstrapContext...
>> >> >
>> >> >
>> >>
>> > > 1. HCANN ReflectionManager - as you said, y'all create your own for
>> >>
>> >
>> >> > your use case. You'd own the lifecycle of that one you create. I
>> >> see no
>> >> > conflict there. Also we know that in 7.0 HCANN use will go away
>> >> and we
>> >> > will move to Jandex. The Jandex IndexView reference is only valid
>> >> for a
>> >> > limited period of time when WF hands it to us.
>> >>
>> > > 2. JPA "temp ClassLoader" - I think this one is self-evident. JPA
>> >>
>> >
>> >> > states that this ClassLoader (if one) is available for only a
>> >> limited time.
>> >>
>> > > 3. ClassmateContext - I centralized this so that we did not have to
>> >>
>> >
>> >> > keep "priming" the classmate caches each time we needed to use
>> >> classmate.
>> >> > Aside from a possible performance hit, there really is nothing
>> >> special here
>> >> > versus creating a new ClassmateContext each time you need it. For
>> >> ORM we
>> >> > currently never use classmate outside of bootstrap. Could that
>> >> change?
>> >> > Maybe, and we'd deal with that if/when it does.
>> >>
>> > > 4. scanning components
>> >>
>> >
>> >> > (ArchiveDescriptorFactory, ScanOptions, ScanEnvironment, Scanner)
>> -
>> >> maybe
>> >> > going back to your "dynamic reconfiguration" scenario this makes
>> >> sense. No
>> >> > idea. But in ORM holding on to these after bootstrap makes no
>> sense.
>> >>
>> > > 5. I've also started making BootstrapContext the holder for
>> bootstrap
>> >>
>> >
>> >> > metadata-related collectors. Here we collect
>> >> > SQLFunctions, AuxiliaryDatabaseObjects,
>> >> AttributeConverterDefinitions,
>> >> > and CacheRegionDefinitions.
>> >>
>> > > 6. There are 2 other (new in 6.0) delegates that I keep here too.
>> >>
>> >
>> >> > Interestingly, one is fully intended to be held beyond bootstrap.
>> >> But I
>> >> > think that these intentions just need to be documented.
>> >> >
>> >> >
>> >> > Overall I'd view a "dynamic reconfiguration" scenario very much like
>> a
>> >> > limite bootstrap scenario. Personally I'd expect to have to maker
>> many
>> >> of
>> >> > these "boot only resources" available to that process. Not
>> necessarily
>> >> the
>> >> > same ones as used during the primary bootstrap though. I personally
>> >> would
>> >> > prefer to not hold reference to these "just in case" we have a
>> "dynamic
>> >> > reconfiguration" situation later; I'd just rebuild them. Granted
>> things
>> >> > like a WF-handed Jandex IndexView would be difficult to handle in
>> there,
>> >> > but that is the case regardless of whether we hold reference to it or
>> >> not;
>> >> > that has to do with WF eventually invalidating that reference it
>> handed
>> >> us.
>> >> >
>> >> >
>> >> > So while I think it's a good idea, and also Search should try this
>> >> >> again, I think we'd need to design it from day 1 to be defensive
>> >> >> against future code attempting to hold on these services.
>> >> >> Not sure what would be the best approach for ORM, but I guess that
>> >> >> simply invalidating/closing these components after bootstrap and
>> >> >> having these throw an exception after that would be a good start.
>> >> >>
>> >> >
>> >> > That is roughly what I do. There is a BootstrapContext#release
>> method.
>> >> > It in turn releases the delegates it holds. I can add some defensive
>> >> > checking for throwing some "unavailable" exceptions in case stuff
>> holds
>> >> > references to these. That's a good idea.
>> >> >
>> >> >
>> >> > However, please allow some flexibility for the case in which someone
>> >> >> really needs one of the services you're dooming at runtime.
>> >> >> For example Search might need to re-read configuration properties at
>> >> >> runtime; we can of course make a copy, but then we'd need a way to
>> be
>> >> >> able to make such a copy (We currently actually make such a copy of
>> >> >> the cfg Properties).
>> >> >> Configuration properties being just an example, maybe we need a
>> >> >> generic way to be able to declare which services should not be
>> cleaned
>> >> >> up after bootstrap?
>> >> >>
>> >> >
>> >> > We already hold on to configuration properties into the SF. See
>> >> > ConfigurationService.
>> >> >
>> >> >
>> >> >
>> >> >> In practice, the services you've listed should be fine today but the
>> >> >> need for us to make a copy (or to invoke some API to ask for a life
>> >> >> extension) might show up in future.
>> >> >>
>> >> >> Rough proposal :
>> >> >>
>> >> >> interface BootService {
>> >> >> void flagForUsageBeyondBootstrap();
>> >> >> }
>> >> >>
>> >> >
>> >> > -1 I think the BootstrapContext is not the right place for this. It
>> is
>> >> > not the BootstrapContext itself that needs to remain valid, it is the
>> >> > delegates it exposes. That is where the "extension" should be
>> >> allowed. If
>> >> > that is voted as generally worthwhile, I can see 2 options:
>> >> >
>> >>
>> > > 1. Expose #allowExtendedAccess (or somesuch method name) to the
>> actual
>> >>
>> >
>> >> > delegates. This would be an indicator to not release its
>> resources
>> >> when
>> >> > the BootstrapContext#release method tells the delegate to release
>> >> itself.
>> >>
>> > > 2. Allow OGM, Search, etc to specify specific impls for these
>> >>
>> >
>> >> > delegates. It could handle the delegate's #release method
>> however it
>> >> > wanted.
>> >> >
>> >> > However, realize that if these things are not released by
>> >> > BootstrapContext#release then ORM washes its hands of cleaning them
>> up
>> >> (it
>> >> > would have no "scope" to do that).
>> >> >
>> >>
>> > _______________________________________________
>> >> hibernate-dev mailing list
>> >> hibernate-dev at lists.jboss.org
>> >> https://lists.jboss.org/mailman/listinfo/hibernate-dev
>> >>
>> >
>> _______________________________________________
>> hibernate-dev mailing list
>> hibernate-dev at lists.jboss.org
>> https://lists.jboss.org/mailman/listinfo/hibernate-dev
>>
>
>
More information about the hibernate-dev
mailing list