GeoTools
  1. GeoTools
  2. GEOT-909

EPSG DefaultFactory does not work in an EJB environment

    Details

    • Type: Bug Bug
    • Status: Open Open
    • Priority: Major Major
    • Resolution: Unresolved
    • Affects Version/s: 2.2-RC2
    • Fix Version/s: 2.7.6
    • Component/s: referencing
    • Labels:
      None
    • Environment:
      Windows XP SP2, JDK 1.5.0_06, JBoss 4.0.4-GA

      Description

      The current implementation of org.geotools.referencing.factory.epsg.DefaultFactory in conjunction with org.geotools.referencing.factory.epsg.DataSource has several issues in JBoss:

      • The DATASOURCE_NAME is hardcoded to a name that is not appropriate for JBoss (it should be 'java:EPSG'). I suggest to make the name configurable, perhaps with a Hint.
      • Extending javax.sql.DataSource prevents the use of a standard J2EE JDBC data source. It is also inappropriate to do this just to create the actual factory.
      • In a multiuser-environment, it is not appropriate to permanently reserve a database connection from a pool like FactoryUsingSQL does it. A connection should just be obtained from the pool when needed and then returned by closing it.
      • The current architecture has problems with concurreny: either one uses just a single factory, then it becomes a bottleneck. Or one uses a pool of factories, but then the connection hogging (see above) becomes a serious issue.

        Issue Links

          Activity

          Hide
          Martin Desruisseaux added a comment -
          The DefaultFactory implementation was wrote by someone (myself...) with no knowledge of JEE environments. Is "java:EPSG" a more usual name for a datasource in most JEE implementations? If so, then DATASOURCE_NAME should be changed accordingly.

          A realize that extending javax.sql.DataSource was probably a bad idea. Do you have a suggestion for creating the actual factory, given that different DataSource may want to hook some custom code when creating the factory? Should we provide the factory class name as a property and create it using Java reflection?

          Will put the use of a Connection pool on the todo list.

          As for usage of a single factory or multiple instance of the factory, the issue is related to CRS caching. In current architecture, each FactoryUsingSQL will be associated with its own instance of BufferedAuthorityFactory. Two different FactoryUsingSQL instances may create the same CRS twice if the second instance is not aware that the first instance already created it and cached it. Using a single factory instance can actually improve performances because of caching, unless there is really no overlapping between the CRS created by the two factories. If we want to see performance improvement, it may require providing some way to share the same BufferedAuthorityFactory among different instances of FactoryUsingSQL. Such a change may be a significant complication to the code, so we need to make sure that the performance gain are likely to be significant.

          Thanks a lot for the feedback.
          Show
          Martin Desruisseaux added a comment - The DefaultFactory implementation was wrote by someone (myself...) with no knowledge of JEE environments. Is "java:EPSG" a more usual name for a datasource in most JEE implementations? If so, then DATASOURCE_NAME should be changed accordingly. A realize that extending javax.sql.DataSource was probably a bad idea. Do you have a suggestion for creating the actual factory, given that different DataSource may want to hook some custom code when creating the factory? Should we provide the factory class name as a property and create it using Java reflection? Will put the use of a Connection pool on the todo list. As for usage of a single factory or multiple instance of the factory, the issue is related to CRS caching. In current architecture, each FactoryUsingSQL will be associated with its own instance of BufferedAuthorityFactory. Two different FactoryUsingSQL instances may create the same CRS twice if the second instance is not aware that the first instance already created it and cached it. Using a single factory instance can actually improve performances because of caching, unless there is really no overlapping between the CRS created by the two factories. If we want to see performance improvement, it may require providing some way to share the same BufferedAuthorityFactory among different instances of FactoryUsingSQL. Such a change may be a significant complication to the code, so we need to make sure that the performance gain are likely to be significant. Thanks a lot for the feedback.
          Hide
          Martin Desruisseaux added a comment -
          Problems has also been reported in Tomcat environments. A temporary workaround is to comment out the "if (register)" block, around line 316 in org.geotools.referencing.factory.epsg.DefaultFactory.
          Show
          Martin Desruisseaux added a comment - Problems has also been reported in Tomcat environments. A temporary workaround is to comment out the "if (register)" block, around line 316 in org.geotools.referencing.factory.epsg.DefaultFactory.
          Hide
          Martin Desruisseaux added a comment -
          As of revision 23990 on trunk, we do not extend javax.sql.DataSource anymore. DataSource are now used in a direct, more usual, way.
          Show
          Martin Desruisseaux added a comment - As of revision 23990 on trunk, we do not extend javax.sql.DataSource anymore. DataSource are now used in a direct, more usual, way.
          Hide
          Andrea Aime added a comment -
          Martin, in 2.3.x we could really use comment out the if(register) stuff since the error is logged and makes user think they are having issues with Geoserver (the code is triggered in Geoserver 1.5.x which uses epsg-hsql).
          It seems to me that code serves no users right now, no?
          Show
          Andrea Aime added a comment - Martin, in 2.3.x we could really use comment out the if(register) stuff since the error is logged and makes user think they are having issues with Geoserver (the code is triggered in Geoserver 1.5.x which uses epsg-hsql). It seems to me that code serves no users right now, no?
          Hide
          Martin Desruisseaux added a comment -
          Right, the code serve no users as far as I know. I will remove the code in a few days.
          Show
          Martin Desruisseaux added a comment - Right, the code serve no users as far as I know. I will remove the code in a few days.
          Hide
          Martin Desruisseaux added a comment -
          Disabled the automatic JNDI registration on both trunk and 2.3 branch as of revision 24094.
          Show
          Martin Desruisseaux added a comment - Disabled the automatic JNDI registration on both trunk and 2.3 branch as of revision 24094.
          Hide
          Cory Horner added a comment -
          added link to GEOT-1286, since the last item is unresolvable on its own
          Show
          Cory Horner added a comment - added link to GEOT-1286 , since the last item is unresolvable on its own

            People

            • Assignee:
              Unassigned
              Reporter:
              Felix Mayer
            • Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated: