Skip to main content

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [List Home]
[eclipselink-dev] Fix for Bug 312947 Committed

Patch reviewed by Andrei.

Committed to 2.1.1 in revision: 7811 
Committed to trunk in revision 7812

--Shaun

On 13/07/2010 1:55 PM, Shaun Smith wrote:
Bug 312947 - Allow EclipseLink JPA to work with Gemini in OSGi
https://bugs.eclipse.org/bugs/show_bug.cgi?id=312947

Andrei, can you review the scaled back patch: https://bugs.eclipse.org/bugs/attachment.cgi?id=174185

Thread safety of EntityManagerFactoryProvider's public static collections tracked via  https://bugs.eclipse.org/bugs/show_bug.cgi?id=319765

--Shaun

On 13/7/2010 9:27 AM, Andrei Ilitchev wrote:
Sounds good.

On 12/07/2010 6:27 PM, Shaun Smith wrote:
Andrie,

Let's separate the two issues.  First, I think you're ok with the minor change passing the properties map down to the register transformer.  I absolutely need this and I don't believe you have an issue with this, correct?

Second, the concurrent Maps issue.  Let me remove this from the patch for now so Gemini can move ahead and let's you and I look at the larger issue of initialization and concurrency in the initialization code.  I've worked around the current EclipseLink expectation of a single initialization pass but am not happy with it.  It'll work for now but EclipseLink initialization should be more flexible, especially how it obtains classloaders.

Once I have the Gemini code committed we can review what I needed to do to workaround the current limitations and perhaps use this example to drive some changes into EclipseLink?

    Shaun

On 12/7/2010 5:02 PM, Andrei Ilitchev wrote:

On 12/07/2010 4:29 PM, Shaun Smith wrote:
Hi Andrei,

    Unfortunately you haven't seen the Gemini JPA code that relies on these EclipseLink changes because Mike Keith is waiting for these changes to show up in EclipseLink before we apply the Gemini updates.
Please post these changes - otherwise it's impossible to review the current patch.
SE Initialization is a pretty sensitive (and specialized) code, let's be careful..

On 12/7/2010 4:15 PM, Andrei Ilitchev wrote:
JPAInitializer: why to add an extra parameter to registerTransformer (Map properties)? The only use of the existing parameter puInfo is to print out puName - for FINER logging.
In Gemini OSGi JPA I need to pass properties down to the transformer, specifically the Bundle name and version.
EntityManagerFactoryProvider: both initialEmSetupImpls and initialPuInfos are used only in JavaSE case (not OSGi) - why both their class and initial values are changed? They should not be.
They are referenced in PersistenceProvider which is not just SE.  The Gemini code path will hit them.
Again, let's look at Gemini code path...
HashMap is used so that get doesn't lock the map - Hashtable's locking get method would cause performance deterioration.
So we use a highly concurrent map, but truth be told this code is not highly contended.
Please point exactly at the code where the problem seems to be.
There is no concurrency concerns for initialPuInfos - it's used (if used at all) only once during initialization;
Again, referenced in PersistenceProvider and in Gemini initialization happens as bundles come and go.
Possible concurrency issues for initialEmSetupImpls resolved by synchronized (EntityManagerFactoryProvider.emSetupImpls) statement in PersistenceProvider (one for put, another for remove).
I think ConcurrentHashMap is a more concurrent solution.
What is "more concurrent solution"? Is it more efficient? Or do you see a concrete problem with the existing code - if so please post this code.

Initial values should stay null for both maps, too:
neither of the maps is used outside of JavaSE case - no reason to always initialize them.
Again, PersistenceProvider and the Gemini code is not JavaSE, it's multi-threaded OSGi.  It's easier to initialize them statically which is thread safe than to write multi-thread aware initialization code.
Even in JavaSE case initialPuInfos only used if the javaagent is used - initializing it application managed case on app. server would mean that changes made to redeployed persistence unit would be ignored.
Gemini provides an OSGi "Agent" that may run multiple times for different bundles to perform initialization and support weaving. 

But could you clarify the issue with redeployment?
initialPuInfos was introduced as a performance enhancing trick for a single very specific case - JavaSE using javaagent.
In JavaSE case that doesn't use javaagent there is a potential for redeployment: say, it's an application on app. server, it could be undeployed, then redeployed with possibly altered classes, persistence.xml etc.
If the original puInfo is cached in initialPuInfos, then the new puInfo will be ignored.

If bundles come and go, I guess OSGi would need initialPuInfos==null.

    Shaun


On 12/07/2010 3:15 PM, Shaun Smith wrote:
Bug 312947 - Allow EclipseLink JPA to work with Gemini in OSGi
https://bugs.eclipse.org/bugs/show_bug.cgi?id=312947

Minor adjustments to JPA initialization to work with Gemini JPA.  This revised patch does not remove what seemed to be dead temploader code as James advised is still necessary to support weaving legacy apps using sessions.xml.
--
Oracle
Shaun Smith | Principal Product Manager
Phone: +19055023094
Oracle Server Technologies, Oracle TopLink
ORACLE Canada | 110 Matheson Boulevard West, Suite 100, Mississauga, Ontario | L5R 3P4

Green Oracle Oracle is committed to developing practices and products that help protect the environment

_______________________________________________ eclipselink-dev mailing list eclipselink-dev@xxxxxxxxxxx https://dev.eclipse.org/mailman/listinfo/eclipselink-dev
_______________________________________________ eclipselink-dev mailing list eclipselink-dev@xxxxxxxxxxx https://dev.eclipse.org/mailman/listinfo/eclipselink-dev

--
Oracle
Shaun Smith | Principal Product Manager
Phone: +19055023094
Oracle Server Technologies, Oracle TopLink
ORACLE Canada | 110 Matheson Boulevard West, Suite 100, Mississauga, Ontario | L5R 3P4

Green Oracle Oracle is committed to developing practices and products that help protect the environment

_______________________________________________ eclipselink-dev mailing list eclipselink-dev@xxxxxxxxxxx https://dev.eclipse.org/mailman/listinfo/eclipselink-dev
_______________________________________________ eclipselink-dev mailing list eclipselink-dev@xxxxxxxxxxx https://dev.eclipse.org/mailman/listinfo/eclipselink-dev

--
Oracle
Shaun Smith | Principal Product Manager
Phone: +19055023094
Oracle Server Technologies, Oracle TopLink
ORACLE Canada | 110 Matheson Boulevard West, Suite 100, Mississauga, Ontario | L5R 3P4

Green Oracle Oracle is committed to developing practices and products that help protect the environment

_______________________________________________ eclipselink-dev mailing list eclipselink-dev@xxxxxxxxxxx https://dev.eclipse.org/mailman/listinfo/eclipselink-dev
_______________________________________________ eclipselink-dev mailing list eclipselink-dev@xxxxxxxxxxx https://dev.eclipse.org/mailman/listinfo/eclipselink-dev

--
Oracle
Shaun Smith | Principal Product Manager
Phone: +19055023094
Oracle Server Technologies, Oracle TopLink
ORACLE Canada | 110 Matheson Boulevard West, Suite 100, Mississauga, Ontario | L5R 3P4

Green
              Oracle Oracle is committed to developing practices and products that help protect the environment

_______________________________________________ eclipselink-dev mailing list eclipselink-dev@xxxxxxxxxxx https://dev.eclipse.org/mailman/listinfo/eclipselink-dev
_______________________________________________ eclipselink-dev mailing list eclipselink-dev@xxxxxxxxxxx https://dev.eclipse.org/mailman/listinfo/eclipselink-dev

--
Oracle
Shaun Smith | Principal Product Manager
Phone: +19055023094
Oracle Server Technologies, Oracle TopLink
ORACLE Canada | 110 Matheson Boulevard West, Suite 100, Mississauga, Ontario | L5R 3P4

Green
          Oracle Oracle is committed to developing practices and products that help protect the environment

Back to the top