Re: [eclipselink-dev] Fix for 455683. Add support to automatically detec

[Lukas Jungmann <lukas.jungmann@xxxxxxxxxx>]

On 2/7/15 4:03 PM, andrei ilitchev wrote:

In an unrelated thread lukas.jungmann@xxxxxxxxxx wrote:
  "we're on wls, pu is coming from spring - is instance of org.springframework.jdbc.datasource.SimpleDriverDataSource or sth similar, check could be just for the package name"

this is what I've done:
-I briefly went through http://docs.spring.io/spring/docs/current/spring-framework-reference/html/orm.html#orm-jpa
-I took samples from 'https://github.com/spring-projects/spring-integration-samples'
-I configured 'basic/jpa' sample to be using eclipselink (see: https://github.com/spring-projects/spring-integration-samples/tree/master/basic/jpa)
-I run that sample under the debugger

=> I saw that in this particular case PersistenceUnitInfo implementation which gets to EL is an instance of org.springframework.orm.jpa.persistenceunit.SpringPersistenceUnitInfo and it holds an instance of org.springframework.jdbc.datasource.SimpleDriverDataSource in nonJtaDataSource field

--lukas

Could that be used to autodetect Spring?
On any ap or on wls only?

Thanks,
Andrei

On 2/6/2015 4:26 PM, andrei ilitchev wrote:

On 2/6/2015 4:00 PM, Dmitry Kornilov wrote:

If only we could somehow detect in predeploy that the persistence unit is used with Spring then we could fix autodetection (if Spring then NoServerPlatform or better serverPlatform.disableJTA()).

I like an idea of Spring detector. Implementing it will bring server platform auto detection advantages without breaking backwards compatibility. But I see a problem implementing it. Class loader approach won’t work here. 

It is possible to try instantiating org.springframework.core.SpringVersion class by reflection. SpringVersion.getVersion() can be used to get a version of Spring codebase.

But it just a proof of existence Spring in class path. Any ideas?

May be Spring appears on the stack when predeploy is called?

Otherwise alternatives are:

1. Remove server platform autodetection.

A new TargetServer.Auto value could be defined to enable use of autodetection.

1.1 Pro: Spring users need no changes in their setup;
1.2 Con: Non Sprint users must specify server platform;

1.3 Backwards compatibility

That's a big one.

   

2. Keep server platform autodetection.
2.1 Con: Spring users must specify NoServerPlatform (or better a newly defined SpringPlatform);
2.2 Pro: Non Sprint users need not specify server platform.
2.3 Pro: SpringPlatform would allow to use correct server platform (wls, was) but with disabled jta, that would allow to perform serverPlatform-specific actions correctly (like unwrapping data source connection).

It is possible to do now specifying both target-server and transaction-type=RESOURCE_LOCAL. So I see no advantage here.

That wouldn't work in JTA case because  jtaDataSource specified in persistence.xml in RESOURCE_LOCAL case would be ignored.
Instead of SpringPlatform better define a new property eclipselink.spring (false by default) - that way would work both with specified and autodetected server platform.

Thanks,

Dmitry

On 2/6/2015 12:30 PM, Dmitry Kornilov wrote:

> That time I didn't dig deep enough. This case is complicated. 

Can you explain the scenario better so I can try to understand what the problem is?

I have explained it more than once. Please scroll down the history. It's there.

> Spring is widely used. I don't know the exact statistic data but it could be the most used case. I am still not sure that it's a good idea to break it.

Sure, I get that Spring is popular. Is this perhaps a bug in Spring's usage of the EclipseLink / JPA APIs?

There is a bug? The defaults were used.

Rick, I don't think that we can convince each other. This conversation gets useless and there is no point to continue it. I would like to hear some fresh opinion from other committers.

Thanks,
Dmitry

On Fri, Feb 6, 2015 at 10:49 AM, Dmitry Kornilov <dmitry.kornilov@xxxxxxxxxx> wrote:

Rick,

> "relying on the old behavior is relying on a bug"

I'm pretty certain that you already agreed that this application is doing something wrong. As I stated previously, this application is relying on a behavior that is not spec compliant. That is a bug and it doesn't matter how long it's been a bug for.

Yes, I did. That time I didn't dig deep enough. This case is complicated. It brings some advantages and breaks the backwards compatibility at the same time. Spring is widely used. I don't know the exact statistic data but it could be the most used case. I am still not sure that it's a good idea to break it.

> Since what time we stopped caring about backwards compatibility?

Not once have I said that we don't care about compatibility. When making branch level changes, compatibility is paramount. On major release boundaries it is a slightly different story. I think a a release boundary we have the duty to fix bugs/behaviors in the correct manner for future customers yet give existing users the ability to revert to the old (and sometimes incorrect) behavior. You can't expect an application that is working on 2.3.0 to work in the exact same manner on 2.6.0. If that was the case we could never add new function, fix bugs, etc, etc.

I am pretty sure that if only standard JPA configuration is used the same code will work fine on EL 2.3.0. Even though I cannot say so if some EL specific features are used. These defaults are there since which version? Since the beginning I suppose.

> And now we changed them to achieve some questionable advantages.

I've made my case and if you feel that this functionality isn't the correct behavior for WLS, go ahead and disable the WebLogicPlatformDetector. I don't think that is the right answer, but do what you must.

I've got your point.

Dmitry

On Fri, Feb 6, 2015 at 6:54 AM, Dmitry Kornilov <dmitry.kornilov@xxxxxxxxxx> wrote:

Rick, I understand your point. I don't see any problems in server platform classes implementation.
But I don't agree that "relying on the old behavior is relying on a bug". There is no bug, where are changed defaults. These defaults were there for years. And now we changed them to achieve some questionable advantages. Since what time we stopped caring about backwards compatibility?

Thanks,
Dmitry

On 5.2.2015 20:14, Rick Curtis wrote:

> Please review the solutions above or offer your solution. We need to decide how to deal with this problem before EL 2.6.0. release.

My view is that any customer that was relying on the old behavior was relying on a bug. If they want to get back to the old behavior they can set the target-server to None. If this change was in a service branch, I would vote to disable the functionality and require a property to enable it. Since the change went into a major release, customers need to set a property to revert to the old(incorrect) behavior.

Thanks,

Rick

On Thu, Feb 5, 2015 at 11:10 AM, Dmitry Kornilov <dmitry.kornilov@xxxxxxxxxx> wrote:

Hi,

I would like to reopen a discussion about server platform auto detection. We continue to receive complains from our customers about breaking the backwards compatibility.

I briefly repeat what the problem is for people included in CC. After adding server platform auto detection in early Jan 2015 the default behavior of EclipseLink is changed. If in persistence.xml a target platform and transaction type are not defined in some cases "Cannot use an EntityTransaction while using JTA" exception is thrown. It happens because WLS platform gets auto detected and transaction type is switched to JTA. Which is not the desired behaviour in some cases. See the table below:

                         Previous Version            New Version        

Server Platform:         NoServerPlatform            Weblogic_XX_Platform
Transaction type:        ResourceLocal               JTA                

It often happens when Spring is used. Spring manages transactions itself and requires "local resource" transaction mode.

As Rick mentioned, the spec says that if the transaction-type isn't specified in an EE container it defaults to JTA. It also mentions that if a persistence unit is using RESOURCE_LOCAL that a non-jta-datasource will be provided. This is correct, but the spec doesn't say that server platform has to be detected automatically. It just says that if a JEE platform is set in persistence.xml then use JTA transaction type by default.

There are some workarounds:

1. Specify NoServerPlatform in persistence.xml: <property name="eclipselink.target-server" value="None"/>. It will switch EL to the same behaviour as it was before the server platform detection change.

2. Specify <persistence-unit name="xxx" transaction-type="RESOURCE_LOCAL"> in persistence.xml. In this case server platform will be WLS but resource local transactions are used. I am not sure that it will work though.

3. In theory, configuring Spring to use platform specific JTA transaction manager should also fix the problem.

All workarounds require some changes in customer's applications. I think it would be unacceptable to customers whose applications are broken after introducing the new version of EL.

I see the following solutions to this problem:

1. Rollback server platform auto detection code. I suppose that breaking backwards compatibility is a good reason for that.

2. Disable server platform auto detection for WLS only. It will fix it for WLS only.

3. Add a boolean system property eclipselink.server.autodetection which switches EL server platform auto detection. By default the value is false. It will make this feature 'experimental'. It will be up to customers enable it or not.

Please review the solutions above or offer your solution. We need to decide how to deal with this problem before EL 2.6.0. release.

Thanks!

Best regards,
Dmitry Kornilov

On 30.1.2015 18:41, Dmitry Kornilov wrote:

In this particular case yes.

On 30.1.2015 18:35, Rick Curtis wrote:

Dmitry -

> setting server platform to None is a right solution in this case. 

I don't entirely follow.... are you saying that the platform detection code is doing the right thing and the application needs to configure the target-server to None?

 

Thanks,

Rick

On Fri, Jan 30, 2015 at 11:12 AM, Dmitry Kornilov <dmitry.kornilov@xxxxxxxxxx> wrote:

Hi Rick,

After some investigation and checking the specs I decided that EL behaviour is correct. That application used Spring for transaction management and setting server platform to None is a right solution in this case.

Thanks,
Dmitry

On 30.1.2015 16:26, Rick Curtis wrote:

Dmitry -

Have you made any progress?

Thanks,

Rick

On Thu, Jan 29, 2015 at 10:04 AM, andrei ilitchev <andrei.ilitchev@xxxxxxxxxx> wrote:

Hi Dmitry,

I don't have the most recent Eclipselink version, but by looking at 2.5.3 (and I believe it should be pretty close to trunk) I can't so far understand how that would happen: in EntityManagerSetupImpl class:
- updateLogins method sets the boolean flag:
        login.setUsesExternalTransactionController(transactionType == PersistenceUnitTransactionType.JTA);
- right after the call to updateLogins method, if the flag is set to false then jta is disabled:
        if (!session.getDatasourceLogin().shouldUseExternalTransactionController()) {
            session.getServerPlatform().disableJTA();
        }

May be you could step through the code in debugger and see what's going on there?

Thanks,
Andrei

On 1/29/2015 6:28 AM, Dmitry Kornilov wrote:

Hi Andrei,

I got the info from Rick Curtis that you were responsible for WLS detector implementation in EL. We have some backward compatibility issues with it (see email below).

I made a simple table to help you understand the problem. Application is deployed on WLS. There are 3 use cases with different p.xml.

Use case 1:                                     Previous Version            New Version               Suggested Change

Server platform is NOT defined in p.xml         NoServerPlatform            Weblogic_XX_Platform      Weblogic_XX_Platform
Transaction type is NOT defined in p.xml        ResourceLocal               JTA                       ResourceLocal

Use case 2:

Server platform is NOT defined in p.xml         NoServerPlatform            Weblogic_XX_Platform      Weblogic_XX_Platform
Transaction type is set to JTA in p.xml         Error (JTA cannot be        JTA                       JTA
                                                with NoServerPlatform)

Use case 3:

Server platform is set to WLS in p.xml          Weblogic_XX_Platform        Weblogic_XX_Platform      Weblogic_XX_Platform
Transaction type is NOT defined in p.xml        JTA                         JTA                       JTA

Currently we have problems with the first use case. Auto detection code forces usage of JTA transaction type which breaks compatibility with previous versions (see the table above).

All other use cases are OK because the second one was not possible with the previous version and the third one doesn't change anything.

My proposed solution is:
If a server platfrom has to be detected and transaction type is not explicitly set ResourceLocal has to be used.

As Rick said it works OK with WAS. So maybe only WLS detector code has to be fixed.

Please respond asap this is quite urgent.

Thanks!

Best regards,
Dmitry Kornilov

On 29.1.2015 02:00, Rick Curtis wrote:

Dmitry -

Based off the p.xml that you sent, I tested calling Persistence.createEntityManagerFactory(...) for a persistence unit that declares only a jta-data-source / unspecified transaction-type and I'm not seeing the same problem you encountered. I tested with both WebSphere_Liberty target server and None target server. I'll note that my test fails because EclipseLink doesn't have a datasource to use and it ignores the jta-data-source. I did find an interesting warning message[1] that seems to explain why my jta-data-source isn't being used.

If I update my application to use a non-jta-datasource it works fine with the WAS platform (it failed with None server, but that is for an unrelated reason). Needless to say, I'm confused about why I am unable to recreate your failure. I assume it has something to do with a difference between the way that WAS and WLS operate. If I were to go out on a limb, I would guess it has something to do with EntityManagerFactoryProvider.emSetupImpls and the contents of that map being shared between the WLS container created EMFs and Persistence.createEntityManagerFactory(..). Anyway, I digress.

> The point is that the defaults are changing.

Fair point. I think it's also fair to consider that the old default value might not be correct? The spec mentions that if the transaction-type isn't specified in an EE container it defaults to JTA. It also mentions that if a persistence unit is using RESOURCE_LOCAL that a non-jta-datasource will be provided. That being said, the spec is vague and most of the time not breaking  a customer trumps following the spec.

> I suggest changing your code the way that if server platform is NOT defined in persistence.xml then a detected platform has to use resource local transactions.

I don't really like that approach as I'm not completely certain that is the right answer. I don't know what the right answer is, but I don't like restricting target-server detection down to resource_local. Another option is to remove WLS from target server detection all together. This is a drastic change, but this would be sure to solve the problems you're having.

If you guys haven't yet, please ping Andrei Illitchev to see if he has any input. He is the one that provided guidance on the change and specifically implemented the WLS detector.

I assume you guys are trying to get final testing in before releasing 2.6.x and there is pressure to get this fixed asap so I'll be sure to stay on top of this one.

Thanks,

Rick

[1] [EL Warning]: transaction: 2015-01-28 18:18:40.974--ServerSession(-79379177)--PersistenceUnitInfo persistent.coyote has transactionType RESOURCE_LOCAL and therefore jtaDataSource will be ignored

On Wed, Jan 28, 2015 at 4:46 PM, Dmitry Kornilov <dmitry.kornilov@xxxxxxxxxx> wrote:

Hi Rick,

I don't know what was declared there. I don't have a unit test for this issue. There were a couple of applications falling with the same error message after the new EL version (with your commit) was released. I don't have access to their source code. I'll find out and let you know.

As I see the problem is in changing a default transaction manager (JTA/local resources) in server platform auto detection. It doesn't matter what was defined there. The point is that the defaults are changing.

Thanks,
Dmitry

On 28.1.2015 22:45, Rick Curtis wrote:

Dmitry -

In the failing tests, what do you have declared for a transaction-type in the persistence.xml?

Thanks,

Rick

On Wed, Jan 28, 2015 at 6:59 AM, Dmitry Kornilov <dmitry.kornilov@xxxxxxxxxx> wrote:

Hi Rick,

I am writing you because of your EclipseLink commit "1e0a8ab4   Fix for 455683. Add support to automatically detect the target server."

This change introduced quite a few compatibility problems with existing applications.

If an application is deployed on WLS server and a server platform was not explicitly set in persistence.xml then NoServerPlatform was used by EL before your commit and Weblogic_XX_Platform is used after your commit.
NoServerPlatform switches EL to use resource local transactions, Weblogic_XX_Platform uses JTA transactions.
It causes backward compatability problems. See the exception below:

Cannot use an EntityTransaction while using JTA.
at
org.eclipse.persistence.internal.jpa.transaction.JTATransactionWrapper.getTran
saction(JTATransactionWrapper.java:73)
at
org.eclipse.persistence.internal.jpa.EntityManagerImpl.getTransaction(EntityMa
nagerImpl.java:1322)

The workaround would be is adding <property name="eclipselink.target-server" value="None"/> to persistence.xml. But it would be better if it works without workarounds.

I suggest changing your code the way that if server platform is NOT defined in persistence.xml then a detected platform has to use resource local transactions.

Please respond asap this is quite urgent.

Thanks!

Best regards,
Dmitry Kornilov

--

Rick Curtis

--

Rick Curtis

--

Rick Curtis

--

Rick Curtis

_______________________________________________
eclipselink-dev mailing list
eclipselink-dev@xxxxxxxxxxx
To change your delivery options, retrieve your password, or unsubscribe from this list, visit
https://dev.eclipse.org/mailman/listinfo/eclipselink-dev

--

Rick Curtis

_______________________________________________
eclipselink-dev mailing list
eclipselink-dev@xxxxxxxxxxx
To change your delivery options, retrieve your password, or unsubscribe from this list, visit
https://dev.eclipse.org/mailman/listinfo/eclipselink-dev

_______________________________________________
eclipselink-dev mailing list
eclipselink-dev@xxxxxxxxxxx
To change your delivery options, retrieve your password, or unsubscribe from this list, visit
https://dev.eclipse.org/mailman/listinfo/eclipselink-dev

--

Rick Curtis

_______________________________________________
eclipselink-dev mailing list
eclipselink-dev@xxxxxxxxxxx
To change your delivery options, retrieve your password, or unsubscribe from this list, visit
https://dev.eclipse.org/mailman/listinfo/eclipselink-dev

_______________________________________________
eclipselink-dev mailing list
eclipselink-dev@xxxxxxxxxxx
To change your delivery options, retrieve your password, or unsubscribe from this list, visit
https://dev.eclipse.org/mailman/listinfo/eclipselink-dev

--

Rick Curtis

_______________________________________________
eclipselink-dev mailing list
eclipselink-dev@xxxxxxxxxxx
To change your delivery options, retrieve your password, or unsubscribe from this list, visit
https://dev.eclipse.org/mailman/listinfo/eclipselink-dev

_______________________________________________
eclipselink-dev mailing list
eclipselink-dev@xxxxxxxxxxx
To change your delivery options, retrieve your password, or unsubscribe from this list, visit
https://dev.eclipse.org/mailman/listinfo/eclipselink-dev

_______________________________________________
eclipselink-dev mailing list
eclipselink-dev@xxxxxxxxxxx
To change your delivery options, retrieve your password, or unsubscribe from this list, visit
https://dev.eclipse.org/mailman/listinfo/eclipselink-dev

_______________________________________________
eclipselink-dev mailing list
eclipselink-dev@xxxxxxxxxxx
To change your delivery options, retrieve your password, or unsubscribe from this list, visit
https://dev.eclipse.org/mailman/listinfo/eclipselink-dev

_______________________________________________
eclipselink-dev mailing list
eclipselink-dev@xxxxxxxxxxx
To change your delivery options, retrieve your password, or unsubscribe from this list, visit
https://dev.eclipse.org/mailman/listinfo/eclipselink-dev

_______________________________________________
eclipselink-dev mailing list
eclipselink-dev@xxxxxxxxxxx
To change your delivery options, retrieve your password, or unsubscribe from this list, visit
https://dev.eclipse.org/mailman/listinfo/eclipselink-dev