| Re: [eclipselink-dev] Adding extension points to EclipseLink |
Hi Rick,
The patch looks good, some comments:
- I think NoServerPlatformDetector.checkPlatform should return TargetServer.None rather than TargetServer.DEFAULT;
- updateServerPlatform typically called two times (once in predeploy, once in deploy) - to detect server platform no more than once consider adding a boolean flag to EntityManagerSetupImpl;
-- actually better to define a static variable EntityManagerSetupImpl.detectedServerPlatformClass and set it the first time auto detection is done - that way detection will be performed just once (except for possible initialization concurrency)
- WLSPlatformDector:
public String checkPlatform() {
String platform = null;
try {
if(this.class.getClassLoader().getClass().getName().contains("weblogic") {
platform = ServerPlatform.WebLogic;
Class clazz = PrivilegedAccessHelper.getClassForName("weblogic.version");
Method method = PrivilegedAccessHelper.getMethod(clazz, "getReleaseBuildVersion", null, false);
String serverNameAndVersion = (String) PrivilegedAccessHelper.invokeMethod(method, null, null);
if (Helper.compareVersions(serverNameAndVersion, "9") >= 0) {
if (Helper.compareVersions(serverNameAndVersion, "10") >= 0) {
platform = ServerPlatform.WebLogic_10;
} else {
platform = ServerPlatform.WebLogic_9;
}
}
}
} catch (Throwable ex){}
return platform;
}
Thanks,
Andrei
On 1/5/2015 6:21 PM, Rick Curtis wrote:
Andrei -
Please take a look at the patch attached below and let me know what you think. If it seems to be in the right direction, could I have you write a WLSPlatformDector that could be included in the initial commit?
Thanks,Rick
On Wed, Dec 17, 2014 at 9:21 PM, Andrei Ilitchev <andrei.ilitchev@xxxxxxxxxx> wrote:
----- Original Message -----
From: curtisr7@xxxxxxxxx
To: eclipselink-dev@xxxxxxxxxxx
Sent: Wednesday, December 17, 2014 4:03:05 PM GMT -05:00 US/Canada Eastern
Subject: Re: [eclipselink-dev] Adding extension points to EclipseLink
I like the idea of having a NoPlatformDetector that is first in line, and checks the ClassLoader to see if it is a basic app classloader. This will minimize the cost for anyone that isn't running in an app server env.
> and we don't know how expensive their checkPlatform calls might beWe would need to keep a very close on eye on PlatformDetectors to ensure that they aren't doing any heavy lifting. This detection mechanism must be lightweight.
Looking at the ClassLoader won't help my case where I need to differentiate between two different WebSphere application servers. I would guess that the ClassLoader path might also have problems differentiating between WLS9 and WLS10?> If classLoader.getClas().getName() has "weblogic" in it then we immediately call WebLogicPlatformDetrector, which will decide whether its wls9 or wls10.
Thanks,Rick
On Wed, Dec 17, 2014 at 2:44 PM, Andrei Ilitchev <andrei.ilitchev@xxxxxxxxxx> wrote:The drawback is a necessity to loop through all SeverPlatformDetectors (and we don't know how expensive their checkPlatform calls might be).
Alternatively "websphere" word in a package name is a good reason to call WebSpherePlatformDetector right away.
If it's a standard JSE classloader -> NoPlatform right away.
Thanks,
Andrei
----- Original Message -----
From: curtisr7@xxxxxxxxx
To: eclipselink-dev@xxxxxxxxxxx
Sent: Wednesday, December 17, 2014 2:54:19 PM GMT -05:00 US/Canada Eastern
Subject: Re: [eclipselink-dev] Adding extension points to EclipseLink
That is the beauty of the last patch I posted. For each app server we cam do what makes the most sense.
From a WebSphere point of view, I wouldn't look at the classloader class name as those classes are internals and are subject to change at any time. Using some sort of a defined API/SPI/something is the ideal solution. The two WebSphere mechanisms (system prop and a static class) I used in the patch are documented and can't go away / change.
Thanks,Rick
On Wed, Dec 17, 2014 at 1:29 PM, Andrei Ilitchev <andrei.ilitchev@xxxxxxxxxx> wrote:Consider checking the classloader class name - just like driver name in db case that should allow to figure out the app. server vendor.
----- Original Message -----
From: curtisr7@xxxxxxxxx
To: eclipselink-dev@xxxxxxxxxxx
Sent: Wednesday, December 17, 2014 1:52:52 PM GMT -05:00 US/Canada Eastern
Subject: Re: [eclipselink-dev] Adding extension points to EclipseLink
Doug -
Thanks for the followup.
> That way it was automatically populated for us when we were bootstrapping a container managed EMF.In WebSphere we're planning the same thing. As I mentioned earlier in this thread, this detection support targets the case of application managed (Persistence.createEntityManagerFactory) when running in a container.
> Avoid incorrectly assuming you are in a container when you are not.It will be up to each Platform to figure out what makes the most sense to make this determination. It is likely that the detection won't be perfect in all cases, but it will be better than what is in place today.
> Do not override a user provided server platform at any time.100%. We only want to do something in the case where a property has not been specified.
> Allow for an EMF to be created within a container where a server platform integration is not wanted - fringe case but does come up occasionally.Indeed this is an edge case and with detection in place it would require the user to explicitly set the target-server=None
Thanks,Rick
On Wed, Dec 17, 2014 at 12:07 PM, Doug Clarke <douglas.clarke@xxxxxxxxxx> wrote:Rick,
Sorry for joining this conversation late. I do like the idea of detecting the server platform similar to how the database platform is detected. In the case of the database platform we have physical connection details from which we can make a determination. In the case of the server it has been difficult to identify that you are in the container versus on the class path of the container. Its the later case that prevented my earlier work in this area.
In the case of WebLogic the server team did some work to include the server platform if it was not already in the PU properties. That way it was automatically populated for us when we were bootstrapping a container managed EMF.
The situations I believe you need to account for are:
- Avoid incorrectly assuming you are in a container when you are not. In the end I was looking at the acquired initial context to see where EclipseLink is actually being run.
- Do not override a user provided server platform at any time. Some users want to extend a platform or provide their own for specific cases.
- Allow for an EMF to be created within a container where a server platform integration is not wanted - fringe case but does come up occasionally.
I hope this helps,
Doug
On Dec 17, 2014, at 11:31 AM, Rick Curtis <curtisr7@xxxxxxxxx> wrote:
<platform.detector.v2.patch>_______________________________________________Attached below is a prototype patch with a similar(yet lighter weight) approach. With this approach each app server can implement a lightweight detector that makes sense for a given platform. The patch includes a detector that is able to detect both Liberty and Full profile. The code is structured as such to minimize changes to EntityManagerSetupImpl to keep main line code paths the same.
Let me know what you think about this approach.
Thanks,Rick
On Mon, Dec 15, 2014 at 4:36 PM, Rick Curtis <curtisr7@xxxxxxxxx> wrote:After thinking about this some more over the weekend, I'm going to back away from the patch I posted last week. Rather using jndi names to try to determine what platform we're running on, I'm going to try another approach. I'll try to post a patch tomorrow.
Thanks,Rick--
On Fri, Dec 12, 2014 at 2:33 PM, Rick Curtis <curtisr7@xxxxxxxxx> wrote:Thanks for the idea, but that feels like more of a work around than a solution. I'd prefer to go the route where we attempt to figure out the platform.
Thanks,Rick
On Fri, Dec 12, 2014 at 1:56 PM, andrei ilitchev <andrei.ilitchev@xxxxxxxxxx> wrote:
Hi Rick,
I just realized there is a simpler solution - specify target server platform as a system property when starting the app. server.
Thanks,
Andrei
On 12/12/2014 1:45 PM, Rick Curtis wrote:
One more thing, thoughts on logging an INFO message in the event that we detected a target server and it is being used?
On Fri, Dec 12, 2014 at 12:41 PM, Rick Curtis <curtisr7@xxxxxxxxx> wrote:
Andrei -
Thanks for the pointers! I understand trying to avoid polluting core classes with externals. That was the largest concern with the patch I posted.
I don't know why I didn't look into changing the JNDIConnector in the ServerPlatform. I will update the WebSpherePlatform constructor to change that value. Simple fix.
Attached below you'll find a patch where I add support for detecting the server platform if the target-server property hasn't been specified by a user. You'll notice that this detection only works for WebSphere_7 and WebSphere_liberty as those were the platforms I've tested. Should I blindly add in some of the other app servers, or wait to do that till after someone else confirms this approach works for the other servers?
While this solution isn't the most performant, but I don't know of a better route. That being said, I could cache the value returned from ServerPlatformDetector as that will not change from one invocation to the next.
Let me know what you think.
Thanks,Rick
--
On Fri, Dec 12, 2014 at 8:51 AM, andrei ilitchev <andrei.ilitchev@xxxxxxxxxx> wrote:
I would not alter the original Eclipselink approach (core classes like JNDIConnector have no knowledge of the outside world).
Not that this approach is bad (it is not), but the original approach is not going away - so the fear is confusion arising from mixture of two opposite approaches.
It would be easy to achieve the same with the existing approach.
Currently ServerPlatform defines initializeExternalTransactionController method, which is called before connecting the session (in DatabaseSessionImpl.preConnectdatasource).
It would be easy to define a new method on ServerPlatform (say, ServerPlatform.initialize), which would be called instead and which would do all necessary server-platform specific initialization of the session:
void initialize() {
initializeExternalTransactionController();
...
}
I think that requirement for the users to specify server platform as a pu property could be removed, too:
Eclipselink used to require database platform to be specified, but now allows for db platform auto detection (see DatabaseSessionImpl.loginAndDetectDatasource).
Even without any Eclipselink changes, in case of container managed pu the correct server platform is easy for container (app. server) to set.
Thanks,
Andrei
On 12/11/2014 3:48 PM, Rick Curtis wrote:
All -
Currently I'm working on two problems when running in WebSphere that I need to come up with a solution for.
1 - When an application uses javax.persistence.Persistence.createEntityManagerFactory(...), they need to specify a WebSphere target server to get transactions (among other things) to work. If they don't specify a target server, the EclipseLink runtime defaults to using the None target server and the app won't interact with our transaction manager. Yes I could require applications to configure eclipselink.target-server, but I would like to remove that requirement.
2 - Again when using javax.persistence.Persistence.createEntityManagerFactory(...) and direct JNDI names, WebSphere users will encounter bug 260383. Numerous places it is suggested to create a SessionCustomizer and then change the JNDIConnector lookup type. While that works, I would also like to remove that requirement.
Since the changes that I'm looking to make are specific to a single application server, I'd like to add an extension point to EclipseLink so I don't need to pollute the codebase with lots of ibm specific code.
I'd like like to solve this problem by adding something to EclipseLink that would use the service loader mechanism to load a service implementation that would be able to change some of the defaults of various EclipseLink internals. At this point this only changes the values of target-server and the JNDIConnector, but I could see it be useful in other areas. Attached below is a patch of some rough prototyping that I did a few days back.
Please take a quick browse through it to see if it seems like a reasonable approach.
Thanks,Rick--
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
--
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
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
_______________________________________________
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
--
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
--
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
Attachment:
455683.v5.patch
Description: Binary data