Re: [hibernate-dev] Centralized access to "bootstrap only" resources

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(a)hibernate.org&gt; 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(a)hibernate.org&gt; > wrote: > > > On Tue, Aug 30, 2016 at 6:27 AM Sanne Grinovero <sanne(a)hibernate.org&gt; > > wrote: > > > >> On 30 August 2016 at 10:09, Emmanuel Bernard <emmanuel(a)hibernate.org&gt; > >> 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/fd4acb5d8f396201f5dccc... > > > > > > 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(a)lists.jboss.org > https://lists.jboss.org/mailman/listinfo/hibernate-dev >