Hi Marina,
comments in line...
On 10/12/06, Marina Vatkina <Marina.Vatkina_at_sun.com> wrote:
>
> Tom, Wonseok,
>
> Please review the changes.
>
> Major points are described below in the IT note. Other diffs:
>
> a) renamed initServerSession() to setServerSessionName(), removed from it
> the
> call to assignCMP3Policy(), and changed the javadoc comments as they
> didn't
> reflect the actual things happening in the method anyway;
>
> b) changed to call assignCMP3Policy() directly in place of the previous
> initServerSession() call;
>
> c) moved the call to setServerSessionName() to the beginning of predeploy
> (see
> the reason #3 below).
>
> Tom, do you see any problem with registering the session by name so early?
> I
> couldn't find any code in the EntityManagerSetupImpl that would suggest
> that
> it can be.
1. Original initServerSession() does another things. Those calls
session.getDescriptors() but this is filled in predeploy phase. Therefore I
think this should not be called before predeploy finishes.
How about making a new method "initSessionName()" by extracting only the
following code?
// use default session name if none is provided
String name = EntityManagerFactoryProvider.getConfigPropertyAsString
(TopLinkProperties.SESSION_NAME, m);
if(name == null) {
if (persistenceUnitInfo.getPersistenceUnitRootUrl() != null){
name =
persistenceUnitInfo.getPersistenceUnitRootUrl().toString()
+ "-" + persistenceUnitInfo.getPersistenceUnitName();
} else {
name = persistenceUnitInfo.getPersistenceUnitName();
}
session.setName(name);
// session.log(SessionLog.FINEST, SessionLog.PROPERTIES,
"property_value_default", new Object[]{TopLinkProperties.SESSION_NAME,
name});
addSessionToGlobalSessionManager();
}
2. (To Tom) Is it safe to call addSessionToGlobalSessionManager() in this
early step (with uninitialized session)?
3. What if SESSION_NAME is set by user explicitly? Now user-specified name
is set later by updateSessionName() which is called from
updateServerSession() in deploy phase. I think we need to set this
user-specified name in "initSessionName()" also. But current code appears to
allow changing session name in deploy(). Then should logger should change?
I'm confused.
4. (To Tom) This is another concern but related to #3. What is the behaviour
of session name property? If I specify the same session name of two
applications or two persistence units, could I share ServerSession? But it
doesn't seem to do it, because in EntityManagerSetupImpl.predeploy() session
is always newly created like below regardless of the session name.
session = new ServerSession(...);
If session name property has no effect in JPA, I think it should be removed
and then there will be no confusion in #3.
d) I added an api to the SessionLog to allow for #1 below. Another option
> would
> be to check for the log level being OFF, but it's more time consuming to
> go up
> the logging chain every time.
Then in Java SE, the default level of JavaLogger becomes INFO while CONFIG
is a default for DefaultLogger. It appears to be inconsistent for the
default level. I don't know why the default level has been CONFIG for
TopLink logger, how about changing it to INFO consistently. Then we don't
need to set default level explicitly and to introduce new method
"userParentLevel()" to SessionLog interface.
protected void initOrUpdateLogging(boolean init, Map m, SessionLog log)
{
String logLevelString = PropertiesHandler.getPropertyValueLogDebug(
TopLinkProperties.LOGGING_LEVEL, m, session);
// Don't need to set default level. It will follow the default level policy
of the logger. INFO for DefaultSessionLog. Parent logger's level(INFO
normally) for JavaLog.
/*
if(logLevelString == null && init) {
logLevelString =
PropertiesHandler.getDefaultPropertyValueLogDebug(
TopLinkProperties.LOGGING_LEVEL, session);
}
*/
if (logLevelString != null) {
log.setLevel(AbstractSessionLog.translateStringToLoggingLevel
(logLevelString));
}
Regards
- Wonseok
Let me know if you see any problems with the proposed changes. I tested with
> 2 PUs being deployed, one with the explicit log level, and another without
> it.
> I also tested dynamic change of the log levels, and server restart. All
> seem
> to work correctly.
>
> thanks,
> -marina
>
> -------- Original Message --------
> Subject: [Issue 1107] Logger defined by a property doesn't work
> Date: Thu, 12 Oct 2006 01:56:44 +0000
> From: mvatkina_at_dev.java.net
> To: mvatkina_at_dev.java.net
>
> https://glassfish.dev.java.net/issues/show_bug.cgi?id=1107
>
>
>
> User mvatkina changed the following:
>
> What |Old value |New value
>
> ================================================================================
> Status|NEW |STARTED
>
> --------------------------------------------------------------------------------
>
>
>
>
> ------- Additional comments from mvatkina_at_dev.java.net Thu Oct 12 01:56:44
> +0000
> 2006 -------
> There are apparently several issues to address -
> 1.Do not set log level explicitly if not specified in the properties - use
> the
> parent logger's level (set by the container) instead.
> 2. The default logger namespace should be a child of TopLink namespace,
> e.g.
> "oracle.toplink.essentials.default", otherwise AbstractSessionLog will set
> the
> explicit level for all TopLink loggers (overriding the container settings)
> in
> the VM on the first PU with the specified log level.
> 3. ServerSession name must be set prior to setting the logger for that
> session,
> otherwise the logger is registered as the default logger, and its level
> will
> apply to all PUs in this VM.
>
> A note: the problem that can't be solved so far is switching between
> explicitly
> specified log level for a PU and the defalt app server level for this
> namespace.
> As there is no way to unset log level after it's set, the only work around
> will
> be to change the property value or restart the server.
>
>
>