You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@openjpa.apache.org by Kevin Sutter <kw...@gmail.com> on 2007/02/15 21:20:32 UTC

Re: svn commit: r507643 - in /incubator/openjpa/trunk: openjpa-kernel/src/main/java/org/apache/openjpa/kernel/AbstractBrokerFactory.java openjpa-lib/src/main/java/org/apache/openjpa/lib/conf/ConfigurationImpl.java

Abe,
This change is causing problems in an application server environment.  I
pulled in this change this morning, re-built, and tried our perf benchmark
(SunOne with OpenJPA) and I get the following exception:

Caused by: <0|false|0.9.7-incubating-SNAPSHOT>
org.apache.openjpa.persistence.PersistenceException: There were errors
initializing your configuration:
org.apache.commons.lang.exception.NestableRuntimeException: An instance of
the class "class
org.apache.openjpa.persistence.PersistenceProviderImpl$ClassTransformerImpl$1"
could not be instantiated.  Make sure the class has a public no-args
constructor.
        at org.apache.openjpa.lib.conf.Configurations.newInstance(
Configurations.java:183)
        at org.apache.openjpa.lib.conf.ObjectValue.newInstance(
ObjectValue.java:99)
        at org.apache.openjpa.lib.conf.PluginValue.instantiate(
PluginValue.java:98)
        at org.apache.openjpa.lib.conf.ObjectValue.instantiate(
ObjectValue.java:76)
        at
org.apache.openjpa.conf.OpenJPAConfigurationImpl.getClassResolverInstance(
OpenJPAConfigurationImpl.java:534)
        at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
        at sun.reflect.NativeMethodAccessorImpl.invoke(
NativeMethodAccessorImpl.java:39)
        at sun.reflect.DelegatingMethodAccessorImpl.invoke(
DelegatingMethodAccessorImpl.java:25)
        at java.lang.reflect.Method.invoke(Method.java:597)
        at org.apache.openjpa.lib.conf.ConfigurationImpl.instantiateAll(
ConfigurationImpl.java:280)
        at org.apache.openjpa.conf.OpenJPAConfigurationImpl.instantiateAll(
OpenJPAConfigurationImpl.java:1411)
        at org.apache.openjpa.kernel.AbstractBrokerFactory.makeReadOnly(
AbstractBrokerFactory.java:542)
        at org.apache.openjpa.kernel.AbstractBrokerFactory.newBroker(
AbstractBrokerFactory.java:152)
        at org.apache.openjpa.kernel.DelegatingBrokerFactory.newBroker(
DelegatingBrokerFactory.java:139)
        at
org.apache.openjpa.persistence.EntityManagerFactoryImpl.createEntityManager(
EntityManagerFactoryImpl.java:187)
        at
org.apache.openjpa.persistence.EntityManagerFactoryImpl.createEntityManager(
EntityManagerFactoryImpl.java:52)
        at com.sun.enterprise.util.EntityManagerWrapper._getDelegate(
EntityManagerWrapper.java:166)
        at com.sun.enterprise.util.EntityManagerWrapper.find(
EntityManagerWrapper.java:239)
        at com.ibm.performance.primitives.jee5.ejb.AccountejbFacade.find(
AccountejbFacade.java:50)
        at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
        at sun.reflect.NativeMethodAccessorImpl.invoke(
NativeMethodAccessorImpl.java:39)
        at sun.reflect.DelegatingMethodAccessorImpl.invoke(
DelegatingMethodAccessorImpl.java:25)
        at java.lang.reflect.Method.invoke(Method.java:597)
        at
com.sun.enterprise.security.application.EJBSecurityManager.runMethod(
EJBSecurityManager.java:1050)
        at com.sun.enterprise.security.SecurityUtil.invoke(SecurityUtil.java
:165)
        at com.sun.ejb.containers.BaseContainer.invokeTargetBeanMethod(
BaseContainer.java:2766)
        at com.sun.ejb.containers.BaseContainer.intercept(BaseContainer.java
:3847)
        at com.sun.ejb.containers.EJBLocalObjectInvocationHandler.invoke(
EJBLocalObjectInvocationHandler.java:184)
        at
com.sun.ejb.containers.EJBLocalObjectInvocationHandlerDelegate.invoke(
EJBLocalObjectInvocationHandlerDelegate.java:71)
        at $Proxy36.find(Unknown Source)
        at
com.ibm.performance.primitives.jee5.web.PingServlet2Session2Entity.processRequest
(PingServlet2Session2Entity.java:45)
        at
com.ibm.performance.primitives.jee5.web.PingServlet2Session2Entity.doGet(
PingServlet2Session2Entity.java:60)
        at javax.servlet.http.HttpServlet.service(HttpServlet.java:707)
        at javax.servlet.http.HttpServlet.service(HttpServlet.java:820)

When I back out these two file changes, everything's back to normal.  The
OpenJPA test bucket runs just fine, with or without these changes.  It's the
app server environment that is broke with these changes.  Nothing is jumping
out at me as to what caused the problem.  Can we back out these changes
until we get to the bottom of the problem?

Thanks,
Kevin


On 2/14/07, awhite@apache.org <aw...@apache.org> wrote:
>
> Author: awhite
> Date: Wed Feb 14 09:53:13 2007
> New Revision: 507643
>
> URL: http://svn.apache.org/viewvc?view=rev&rev=507643
> Log:
> Compare Configuration instances on their full properties, including
> defaults.
> Also serialize the full properties because product derivations aren't
> re-run
> on deserialize, so we need to know the complete defaults, etc.  We still
> track
> the user-given properties separately so they can be used in factory
> pooling.
>
>
> Modified:
>
>     incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/AbstractBrokerFactory.java
>
>     incubator/openjpa/trunk/openjpa-lib/src/main/java/org/apache/openjpa/lib/conf/ConfigurationImpl.java
>
> Modified:
> incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/AbstractBrokerFactory.java
> URL:
> http://svn.apache.org/viewvc/incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/AbstractBrokerFactory.java?view=diff&rev=507643&r1=507642&r2=507643
>
> ==============================================================================
> ---
> incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/AbstractBrokerFactory.java
> (original)
> +++
> incubator/openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/AbstractBrokerFactory.java
> Wed Feb 14 09:53:13 2007
> @@ -58,8 +58,7 @@
>          (AbstractBrokerFactory.class);
>
>      // static mapping of configurations to pooled broker factories
> -    private static final Map _pool = Collections.synchronizedMap
> -        (new HashMap());
> +    private static final Map _pool = Collections.synchronizedMap(new
> HashMap());
>
>      // configuration
>      private final OpenJPAConfiguration _conf;
> @@ -95,6 +94,11 @@
>          return (AbstractBrokerFactory) _pool.get(toPoolKey(conf));
>      }
>
> +    /**
> +     * Return an internal factory pool key for the given configuration.
> +     * We use the conf properties as given by the user because that is
> what's
> +     * passed to {@link #getPooledFactory} when looking for an existing
> factory.
> +     */
>      private static Map toPoolKey(OpenJPAConfiguration conf) {
>          return conf.toProperties(false);
>      }
>
> Modified:
> incubator/openjpa/trunk/openjpa-lib/src/main/java/org/apache/openjpa/lib/conf/ConfigurationImpl.java
> URL:
> http://svn.apache.org/viewvc/incubator/openjpa/trunk/openjpa-lib/src/main/java/org/apache/openjpa/lib/conf/ConfigurationImpl.java?view=diff&rev=507643&r1=507642&r2=507643
>
> ==============================================================================
> ---
> incubator/openjpa/trunk/openjpa-lib/src/main/java/org/apache/openjpa/lib/conf/ConfigurationImpl.java
> (original)
> +++
> incubator/openjpa/trunk/openjpa-lib/src/main/java/org/apache/openjpa/lib/conf/ConfigurationImpl.java
> Wed Feb 14 09:53:13 2007
> @@ -107,6 +107,7 @@
>      private String _product = null;
>      private boolean _readOnly = false;
>      private Map _props = null;
> +    private Map _fullProps = null;
>      private boolean _globals = false;
>      private String _auto = null;
>      private final List _vals = new ArrayList();
> @@ -310,7 +311,7 @@
>      }
>
>      public void valueChanged(Value val) {
> -        if (_changeSupport == null && _props == null)
> +        if (_changeSupport == null && _props == null && _fullProps ==
> null)
>              return;
>
>          String newString = val.getString();
> @@ -319,12 +320,15 @@
>                  newString);
>
>          // keep cached props up to date
> -        if (_props != null) {
> -            if (newString == null)
> -                Configurations.removeProperty(val.getProperty(), _props);
> -            else if (Configurations.containsProperty(val.getProperty(),
> _props)
> -                || val.getDefault() == null
> +        if (newString == null) {
> +            Configurations.removeProperty(val.getProperty(), _props);
> +            Configurations.removeProperty(val.getProperty(), _fullProps);
> +        } else {
> +            put(_fullProps, val, newString);
> +            if (_props != null && _props != _fullProps
> +                && (val.getDefault() == null
>                  || !val.getDefault().equals(newString))
> +                || Configurations.containsProperty(val.getProperty(),
> _props))
>                  put(_props, val, newString);
>          }
>      }
> @@ -566,33 +570,30 @@
>          // clone properties before making any modifications; we need to
> keep
>          // the internal properties instance consistent to maintain equals
> and
>          // hashcode contracts
> +        Map map = (storeDefaults) ? _fullProps : _props;
>          Map clone;
> -        if (_props == null)
> +        if (map == null)
>              clone = new HashMap();
> -        else if (_props instanceof Properties)
> -            clone = (Map) ((Properties) _props).clone();
> +        else if (map instanceof Properties)
> +            clone = (Map) ((Properties) map).clone();
>          else
> -            clone = new HashMap(_props);
> +            clone = new HashMap(map);
>
> -        // if no existing properties or the properties should contain
> entries
> -        // with default values, add values to properties
> -        if (_props == null || storeDefaults) {
> +        if (map == null) {
>              Value val;
>              String str;
>              for (int i = 0; i < _vals.size(); i++) {
> -                // if key in existing properties, we already know value
> is up
> -                // to date
>                  val = (Value) _vals.get(i);
> -                if (_props != null && Configurations.containsProperty
> -                    (val.getProperty(), _props))
> -                    continue;
> -
>                  str = val.getString();
>                  if (str != null && (storeDefaults
>                      || !str.equals(val.getDefault())))
>                      put(clone, val, str);
>              }
> -            if (_props == null)
> +            if (storeDefaults) {
> +                _fullProps = new HashMap(clone);
> +                if (_props == null)
> +                    _props = _fullProps;
> +            } else
>                  _props = new HashMap(clone);
>          }
>          return clone;
> @@ -605,15 +606,13 @@
>
>          // if the only previous call was to load defaults, forget them.
>          // this way we preserve the original formatting of the user's
> props
> -        // instead of the defaults.  this is important for caching on
> -        // configuration objects
> +        // instead of the defaults.
>          if (_globals) {
>              _props = null;
>              _globals = false;
>          }
>
>          Map remaining = new HashMap(map);
> -        boolean ser = true;
>          Value val;
>          Object o;
>          for (int i = 0; i < _vals.size(); i++) {
> @@ -625,10 +624,8 @@
>              if (o instanceof String) {
>                  if (!StringUtils.equals((String) o, val.getString()))
>                      val.setString((String) o);
> -            } else {
> -                ser &= o instanceof Serializable;
> +            } else
>                  val.setObject(o);
> -            }
>              Configurations.removeProperty(val.getProperty(), remaining);
>          }
>
> @@ -639,16 +636,15 @@
>
>          // now warn if there are any remaining properties that there
>          // is an unhandled prop
> -        Map.Entry entry;
> -        for (Iterator itr = remaining.entrySet().iterator(); itr.hasNext();)
> {
> -            entry = (Map.Entry) itr.next();
> -            if (entry.getKey() != null)
> -                warnInvalidProperty((String) entry.getKey());
> -            ser &= entry.getValue() instanceof Serializable;
> +        String key;
> +        for (Iterator itr = remaining.keySet().iterator(); itr.hasNext();)
> {
> +            key = (String) itr.next();
> +            if (key != null)
> +                warnInvalidProperty(key);
>          }
>
> -        // cache properties
> -        if (_props == null && ser)
> +        // cache user-formatted properties
> +        if (_props == null || _props == _fullProps)
>              _props = map;
>      }
>
> @@ -658,6 +654,8 @@
>       * this will account for the property prefix.
>       */
>      private void put(Map map, Value val, Object o) {
> +        if (map == null)
> +            return;
>          Object key = val.getLoadKey();
>          if (key == null)
>              key = "openjpa." + val.getProperty();
> @@ -668,6 +666,8 @@
>       * Look up the given value, testing all available prefixes.
>       */
>      private Object get(Map map, Value val, boolean setLoadKey) {
> +        if (map == null)
> +            return null;
>          String key = ProductDerivations.getConfigurationKey(
>              val.getProperty(), map);
>          if (map.containsKey(key) && setLoadKey)
> @@ -791,8 +791,9 @@
>
>          // compare properties
>          ConfigurationImpl conf = (ConfigurationImpl) other;
> -        Map p1 = (_props == null) ? toProperties(false) : _props;
> -        Map p2 = (conf._props == null) ? conf.toProperties(false) :
> conf._props;
> +        Map p1 = (_fullProps == null) ? toProperties(true) : _fullProps;
> +        Map p2 = (conf._fullProps == null) ? conf.toProperties(true)
> +            : conf._fullProps;
>          return p1.equals(p2);
>      }
>
> @@ -801,9 +802,9 @@
>       * {@link #toProperties}.
>       */
>      public int hashCode() {
> -        if (_props != null)
> -            return _props.hashCode();
> -        return toProperties(false).hashCode();
> +        if (_fullProps != null)
> +            return _fullProps.hashCode();
> +        return toProperties(true).hashCode();
>      }
>
>      /**
> @@ -849,7 +850,11 @@
>       */
>      public void readExternal(ObjectInput in)
>          throws IOException, ClassNotFoundException {
> -        fromProperties((Map) in.readObject());
> +        Map fullProps = (Map) in.readObject();
> +        Map props = (Map) in.readObject();
> +        fromProperties(fullProps);
> +        _fullProps = fullProps;
> +        _props = props;
>          _globals = in.readBoolean();
>      }
>
> @@ -858,10 +863,11 @@
>       * the properties returned by {@link #toProperties}.
>       */
>      public void writeExternal(ObjectOutput out) throws IOException {
> -        if (_props != null)
> -            out.writeObject(_props);
> +        if (_fullProps != null)
> +            out.writeObject(_fullProps);
>          else
> -            out.writeObject(toProperties(false));
> +            out.writeObject(toProperties(true));
> +        out.writeObject(_props);
>          out.writeBoolean(_globals);
>      }
>
> @@ -876,7 +882,9 @@
>              ConfigurationImpl clone = (ConfigurationImpl)
> cons.newInstance
>                  (new Object[]{ Boolean.FALSE });
>              clone._globals = _globals;
> -            clone.fromProperties(toProperties(true));
> +            Map map = new HashMap(toProperties(true));
> +            clone.fromProperties(map);
> +            clone._fullProps = map;
>              return clone;
>          } catch (RuntimeException re) {
>              throw re;
>
>
>

Re: svn commit: r507643 - in /incubator/openjpa/trunk: openjpa-kernel/src/main/java/org/apache/openjpa/kernel/AbstractBrokerFactory.java openjpa-lib/src/main/java/org/apache/openjpa/lib/conf/ConfigurationImpl.java

Posted by Abe White <aw...@bea.com>.
> This change is causing problems in an application server  
> environment.  I
> pulled in this change this morning, re-built, and tried our perf  
> benchmark
> (SunOne with OpenJPA) and I get the following exception:

Sorry about that.  I'll revert it ASAP.  I have a vague notion of  
what might be causing it -- I'll try to reproduce it with a  
standalone test case and fix it before checking in that code again. 
  
_______________________________________________________________________
Notice:  This email message, together with any attachments, may contain
information  of  BEA Systems,  Inc.,  its subsidiaries  and  affiliated
entities,  that may be confidential,  proprietary,  copyrighted  and/or
legally privileged, and is intended solely for the use of the individual
or entity named in this message. If you are not the intended recipient,
and have received this message in error, please immediately return this
by email and then delete it.