Application Server 7
  1. Application Server 7
  2. AS7-4552

JBoss AS 7 produces non-transactional (autocommit) EntityManager within transactional EJB methods when using 3rd party javax.sql.DataSource via @DataSourceDefinition

    Details

    • Type: Bug Bug
    • Status: Resolved Resolved (View Workflow)
    • Priority: Major Major
    • Resolution: Done
    • Affects Version/s: 7.1.1.Final
    • Fix Version/s: 7.1.2.Final (EAP)
    • Environment:
    • Steps to Reproduce:
      Hide

      unpack each test case

      read README for each test for details. Usually "mvn test" and go to URL.

      Show
      unpack each test case read README for each test for details. Usually "mvn test" and go to URL.
    • Similar Issues:
      Show 10 results 

      Description

      When using a javax.sql.DataSource via @DataSourceDefinition to create the JTA datasource for a persistence unit, transactional EJB business methods run without transactions. No warning or error is emitted.

      Business method calls annotated @TransactionAttribute(TransactionAttributeType.REQUIRED) receive an EntityManager that is in autocommit mode, ie is not in a transaction.

      This violates the EJB3 spec and is a nasty problem.

      I discovered this when testing some code against PostgreSQL that uses deferred constraints to create two interdependent database objects; record A must have at least one record B referencing it, but record B also has a foreign key reference to record A. This can be satisfied only with deferred constraints, and works fine in SQL-level testing. When testing with Arquillian at the JBoss AS 7 / Hibernate / JPA level, though, it was breaking.

      Further investigation showed that the entity manager was in autocommit despite the method being transactional, as demonstrated by a test that tries to create and fetch from a cursor.

      I've now verified that the issue exists when using PostgreSQL or H2 as the database, so it's not specific to PostgreSQL. I've attached test cases for both databases.

      JBoss AS 7 clearly has some validation and checking to do because it must not allow an autocommit entity manager to be injected for transactional business methods. That's a really critical error, as it effectively means that transaction isolation is always at DIRTY_READ (which most DBs don't even support) rather than the requested level, and it's impossible to roll back work!

      As a workaround, it should be possible to deploy an archive with an embedded jboss-ds.xml instead of using @DataSourceDefinition . I haven't tested this - struggling to find documentation on in-archive deployment of jboss-ds.xml or equivalent jboss-specific descriptor like a datasource definition for jboss-web.xml .

      Using the jboss admin cli, or deploying a jboss datasource definition xml file to the deployments folder separately to the program archive, isn't subject to the problem. That's a PITA when unit testing, though.

        Activity

        Hide
        Craig Ringer
        added a comment -

        First test case: annotation defined datasource with @DataSourceDefinition(className="org.postgresql.ds.PGSimpleDataSource", ...)

        Show
        Craig Ringer
        added a comment - First test case: annotation defined datasource with @DataSourceDefinition(className="org.postgresql.ds.PGSimpleDataSource", ...)
        Hide
        Craig Ringer
        added a comment -

        Uploaded an update to the first test case (JBossAS7ContainerTransactionsWith3rdPtyDataSourceFixed.zip) with a corrected query. This had no effect on the test because the issue with the query only appeared after the test failed, but it makes the query the same as in the succeeding test case.

        The second file is the succeeding test case, JBossAS7ContainerTransactionsWithJBossDataSource.zip. This test demonstrates that the same code and same database driver, run against a data source defined via jboss-admin.cli instead of using @DataSourceDefinition, works exactly as it should.

        Show
        Craig Ringer
        added a comment - Uploaded an update to the first test case (JBossAS7ContainerTransactionsWith3rdPtyDataSourceFixed.zip) with a corrected query. This had no effect on the test because the issue with the query only appeared after the test failed, but it makes the query the same as in the succeeding test case. The second file is the succeeding test case, JBossAS7ContainerTransactionsWithJBossDataSource.zip. This test demonstrates that the same code and same database driver, run against a data source defined via jboss-admin.cli instead of using @DataSourceDefinition, works exactly as it should.
        Hide
        Stuart Douglas
        added a comment -

        Does this still happen if you put transactional="true" on your @DatasourceDefinition annotation?

        Show
        Stuart Douglas
        added a comment - Does this still happen if you put transactional="true" on your @DatasourceDefinition annotation?
        Hide
        Craig Ringer
        added a comment -

        Two more test cases, demonstrating that the bug is reproducible with the H2 database too; it's not just a PostgreSQL JDBC driver issue.

        There are two tests within JBossAS7H2Tests.zip.

        JBossAS7ContainerTransactionsWithH2ContainerDS uses a container-provided datasource to, within a transactional EJB method, create a SAVEPOINT then ROLLBACK TO that savepoint. It works as expected.

        JBossAS7ContainerTransactionsWithH2DataSourceDefinition does exactly the same thing but uses a datasource provided by @DataSourceDefinition instead of a container provided data source. It fails to rollback to the savepoint, because the savepoint has ceased to exist when the transaction (auto)committed after the `SAVEPOINT' statement ran. It malfunctions in exactly the same way the PostgreSQL tests do.

        If you rewrote the PostgreSQL tests to use `SAVEPOINT' and `ROLLBACK TO SAVEPOINT' you'd get the same behavior from the PostgreSQL tests; the container-managed datasource would work, but the @DataSourceDefinition one wouldn't.

        Show
        Craig Ringer
        added a comment - Two more test cases, demonstrating that the bug is reproducible with the H2 database too; it's not just a PostgreSQL JDBC driver issue. There are two tests within JBossAS7H2Tests.zip. JBossAS7ContainerTransactionsWithH2ContainerDS uses a container-provided datasource to, within a transactional EJB method, create a SAVEPOINT then ROLLBACK TO that savepoint. It works as expected. JBossAS7ContainerTransactionsWithH2DataSourceDefinition does exactly the same thing but uses a datasource provided by @DataSourceDefinition instead of a container provided data source. It fails to rollback to the savepoint, because the savepoint has ceased to exist when the transaction (auto)committed after the `SAVEPOINT' statement ran. It malfunctions in exactly the same way the PostgreSQL tests do. If you rewrote the PostgreSQL tests to use `SAVEPOINT' and `ROLLBACK TO SAVEPOINT' you'd get the same behavior from the PostgreSQL tests; the container-managed datasource would work, but the @DataSourceDefinition one wouldn't.
        Hide
        Craig Ringer
        added a comment -

        Found this old forum thread that seems related. Looks like JBoss's JTA may be assuming a JBoss javax.sql.DataSource and borking horribly (and silently) if it gets anything else? http://forum.springsource.org/showthread.php?30017-DataSources-auto-commit-and-the-HibernateTransactionManager

        Show
        Craig Ringer
        added a comment - Found this old forum thread that seems related. Looks like JBoss's JTA may be assuming a JBoss javax.sql.DataSource and borking horribly (and silently) if it gets anything else? http://forum.springsource.org/showthread.php?30017-DataSources-auto-commit-and-the-HibernateTransactionManager
        Hide
        Craig Ringer
        added a comment - - edited

        Stuart: "transactional=true" is the default. I just re-tested the failing H2 case, where explicitly adding transactional=true had no effect.

        Show
        Craig Ringer
        added a comment - - edited Stuart: "transactional=true" is the default. I just re-tested the failing H2 case, where explicitly adding transactional=true had no effect.
        Hide
        Craig Ringer
        added a comment -

        Updated comment to reflect that I've now reproduced the bug against both H2 and PostgreSQL.

        Show
        Craig Ringer
        added a comment - Updated comment to reflect that I've now reproduced the bug against both H2 and PostgreSQL.
        Hide
        Craig Ringer
        added a comment -

        Another reference to something that looks awfully similar to this issue, this one from 2012 but re JBoss AS 5: https://community.jboss.org/message/726706

        Show
        Craig Ringer
        added a comment - Another reference to something that looks awfully similar to this issue, this one from 2012 but re JBoss AS 5: https://community.jboss.org/message/726706
        Hide
        Craig Ringer
        added a comment -

        If you can reproduce this, please elevate to "critical". It's a severe data integrity issue.

        Show
        Craig Ringer
        added a comment - If you can reproduce this, please elevate to "critical". It's a severe data integrity issue.
        Hide
        jaikiran pai
        added a comment -

        I haven't tested this - struggling to find documentation on in-archive deployment of jboss-ds.xml

        Place the *-ds.xml under the META-INF folder of the application. If it's a .war application, then the *-ds.xml is expected to be right under the WEB-INF folder of the .war. This might help http://www.jaitechwriteups.blogspot.in/2012/02/jboss-as-710final-thunder-released-java.html

        Show
        jaikiran pai
        added a comment - I haven't tested this - struggling to find documentation on in-archive deployment of jboss-ds.xml Place the *-ds.xml under the META-INF folder of the application. If it's a .war application, then the *-ds.xml is expected to be right under the WEB-INF folder of the .war. This might help http://www.jaitechwriteups.blogspot.in/2012/02/jboss-as-710final-thunder-released-java.html
        Hide
        Craig Ringer
        added a comment -

        Thanks jaikiran. Know if the JDBC driver and jboss module definition for it can be bundled in the app archive too, or does that have to be deployed separately? For use with Arquillian and ShrinkWrap it's really handy to be able to produce a completely self-contained deployment.

        Either way, for my own code I think I'll just be avoiding defining datasources in my app archives entirely; I'll just create and use container-provided datasources via jboss-cli for testing too. It requires a bit of container setup, but nothing a shell script or two can't fix.

        Show
        Craig Ringer
        added a comment - Thanks jaikiran. Know if the JDBC driver and jboss module definition for it can be bundled in the app archive too, or does that have to be deployed separately? For use with Arquillian and ShrinkWrap it's really handy to be able to produce a completely self-contained deployment. Either way, for my own code I think I'll just be avoiding defining datasources in my app archives entirely; I'll just create and use container-provided datasources via jboss-cli for testing too. It requires a bit of container setup, but nothing a shell script or two can't fix.
        Hide
        Scott Marlow
        added a comment - - edited

        https://community.jboss.org/message/730085?tstart=0 (corrected link) mentions this problem also with @DataSourceDefinition.

        Could you enable TRACE logging for com.arjuna + org.jboss.as.jpa (see https://docs.jboss.org/author/display/AS71/JPA+Reference+Guide#JPAReferenceGuide-Troubleshooting) and attach the resulting as7/standalone/log/server.log after reproducing the issue.

        Show
        Scott Marlow
        added a comment - - edited https://community.jboss.org/message/730085?tstart=0 (corrected link) mentions this problem also with @DataSourceDefinition. Could you enable TRACE logging for com.arjuna + org.jboss.as.jpa (see https://docs.jboss.org/author/display/AS71/JPA+Reference+Guide#JPAReferenceGuide-Troubleshooting ) and attach the resulting as7/standalone/log/server.log after reproducing the issue.
        Hide
        jaikiran pai
        added a comment -
        Show
        jaikiran pai
        added a comment - I think Scott meant this thread https://community.jboss.org/message/730085?tstart=0
        Hide
        Scott Marlow
        added a comment -

        yep, thanks Jaikiran!

        Show
        Scott Marlow
        added a comment - yep, thanks Jaikiran!
        Hide
        Craig Ringer
        added a comment - - edited

        Logs attached as requested. I had some TRACE level logging for Hibernate in there that I disabled for the test; if you want traces of the SQL issued etc I can re-enable and re-test.

        There are two files in h2-test-logs.zip. Both were produced by:

        • Undeploying all except the Pg jdbc drivers (should've undeployed them too but forgot)
        • Stopping the server
        • Deleting all logs
        • Starting the server
        • Deploying the test case
        • Visiting the test cases's jsf page
        • Copying the log file

        h2-fail.log is the result of running JBossAS7ContainerTransactionsWithH2DataSourceDefinition. It is the failing test case that demonstrates the problem.

        h2-success.log is the result of running JBossAS7ContainerTransactionsWithH2ContainerDS. It is included for comparison with h2-fail.log.

        Show
        Craig Ringer
        added a comment - - edited Logs attached as requested. I had some TRACE level logging for Hibernate in there that I disabled for the test; if you want traces of the SQL issued etc I can re-enable and re-test. There are two files in h2-test-logs.zip. Both were produced by: Undeploying all except the Pg jdbc drivers (should've undeployed them too but forgot) Stopping the server Deleting all logs Starting the server Deploying the test case Visiting the test cases's jsf page Copying the log file h2-fail.log is the result of running JBossAS7ContainerTransactionsWithH2DataSourceDefinition. It is the failing test case that demonstrates the problem. h2-success.log is the result of running JBossAS7ContainerTransactionsWithH2ContainerDS. It is included for comparison with h2-fail.log.
        Hide
        Stuart Douglas
        added a comment -

        I think you need to use an XA datasource for this to work.

        Show
        Stuart Douglas
        added a comment - I think you need to use an XA datasource for this to work.
        Hide
        Craig Ringer
        added a comment -

        Stuart Douglas: org.h2.jdbcx.JdbcDataSource is an XA data source. It implements both javax.sql.DataSource and javax.sql.XADataSource . The test case with H2 still fails.

        Not only that, but if you try to specify an XA datasource that implements javax.sql.XADataSource and doesn't implement javax.sql.DataSource (like org.postgresql.xa.PGXADataSource) as the datasource for a @DataSourceDefinition, you'll get a ClassCastException at deploy time. The supplied class must be castable to javax.sql.DataSource, at least for the JBoss implementation of @DataSourceDefinition. There is no separate annotation for an XA datasource definition, nor do there appear to be any attributes to mark a datasource as an XA datasource.

        Even if an XA datasource were required, JBoss AS 7 is still silently permitting a non-transactional EntityManager in EJB business methods that require transactions. If an XA DS were required it should be throwing an EJB exception when an attempt to enter a transactional method is made.

        Separately, in JBoss AS 7 it does seem to be impossible to specify an XA datasource as the data source for @DataSourceDefinition unless it also implements javax.sql.DataSource. I suspect that shouldn't be the case, but don't know enough to be sure.

        Show
        Craig Ringer
        added a comment - Stuart Douglas: org.h2.jdbcx.JdbcDataSource is an XA data source. It implements both javax.sql.DataSource and javax.sql.XADataSource . The test case with H2 still fails. Not only that, but if you try to specify an XA datasource that implements javax.sql.XADataSource and doesn't implement javax.sql.DataSource (like org.postgresql.xa.PGXADataSource) as the datasource for a @DataSourceDefinition, you'll get a ClassCastException at deploy time. The supplied class must be castable to javax.sql.DataSource, at least for the JBoss implementation of @DataSourceDefinition. There is no separate annotation for an XA datasource definition, nor do there appear to be any attributes to mark a datasource as an XA datasource. Even if an XA datasource were required, JBoss AS 7 is still silently permitting a non-transactional EntityManager in EJB business methods that require transactions. If an XA DS were required it should be throwing an EJB exception when an attempt to enter a transactional method is made. Separately, in JBoss AS 7 it does seem to be impossible to specify an XA datasource as the data source for @DataSourceDefinition unless it also implements javax.sql.DataSource. I suspect that shouldn't be the case, but don't know enough to be sure.
        Hide
        jaikiran pai
        added a comment -

        Reading through some specs and other documentation, I think this might be more of a persistence provider implementation (Hibernate in this case) thing. One part of the spec (EJB 3.1 section 13.3.4) states that:

        13.3.4Enterprise Beans Using Container-Managed Transaction Demarcation
        ...
        The enterprise bean methods must not use the following methods of the java.sql.Connection interface: commit, setAutoCommit ...

        which is a sign that (obviously) the enclosing transaction demarcation should be allowed to handle the commit.
        Now, I don't see anything in the spec which talks about how a JPA implementation (ex: Hibernate) should deal with connections which already have autocommit set to true, in the context of a CMT bean. Looking at the Hibernate code, I see that in case of non-JTA access, Hibernate sets the autoCommit on the connection to false https://github.com/hibernate/hibernate-orm/blob/master/hibernate-core/src/main/java/org/hibernate/engine/transaction/internal/jdbc/JdbcIsolationDelegate.java#L70 but in case of JTA access, I don't see it checking the autocommit status on the connection https://github.com/hibernate/hibernate-orm/blob/master/hibernate-core/src/main/java/org/hibernate/engine/transaction/internal/jta/JtaIsolationDelegate.java#L91. I am not sure what the behaviour here should be - perhaps check the autocommit status and throw an error?

        Show
        jaikiran pai
        added a comment - Reading through some specs and other documentation, I think this might be more of a persistence provider implementation (Hibernate in this case) thing. One part of the spec (EJB 3.1 section 13.3.4) states that: 13.3.4Enterprise Beans Using Container-Managed Transaction Demarcation ... The enterprise bean methods must not use the following methods of the java.sql.Connection interface: commit, setAutoCommit ... which is a sign that (obviously) the enclosing transaction demarcation should be allowed to handle the commit. Now, I don't see anything in the spec which talks about how a JPA implementation (ex: Hibernate) should deal with connections which already have autocommit set to true, in the context of a CMT bean. Looking at the Hibernate code, I see that in case of non-JTA access, Hibernate sets the autoCommit on the connection to false https://github.com/hibernate/hibernate-orm/blob/master/hibernate-core/src/main/java/org/hibernate/engine/transaction/internal/jdbc/JdbcIsolationDelegate.java#L70 but in case of JTA access, I don't see it checking the autocommit status on the connection https://github.com/hibernate/hibernate-orm/blob/master/hibernate-core/src/main/java/org/hibernate/engine/transaction/internal/jta/JtaIsolationDelegate.java#L91 . I am not sure what the behaviour here should be - perhaps check the autocommit status and throw an error?
        Hide
        Craig Ringer
        added a comment -

        I just read that part of the spec as saying that the EJB its self must keep its hands off the autocommit state and must not try to commit or roll the transaction back its self. JBoss enforces this just fine.

        One avenue worth investigating is why this happens with 3rd party datasources, but not with JBoss datasources. What's different between a JBoss datasource wrapping an org.postgresql.Driver and an org.postgresql.ds.PGSimpleDataSource wrapping an org.postgresql.Driver? Is it as simple as one defaulting to autocommit and the other not, or is there some interaction between JBoss-provided data sources happening that isn't happening for 3rd party data sources?

        Show
        Craig Ringer
        added a comment - I just read that part of the spec as saying that the EJB its self must keep its hands off the autocommit state and must not try to commit or roll the transaction back its self. JBoss enforces this just fine. One avenue worth investigating is why this happens with 3rd party datasources, but not with JBoss datasources. What's different between a JBoss datasource wrapping an org.postgresql.Driver and an org.postgresql.ds.PGSimpleDataSource wrapping an org.postgresql.Driver? Is it as simple as one defaulting to autocommit and the other not, or is there some interaction between JBoss-provided data sources happening that isn't happening for 3rd party data sources?
        Hide
        Scott Marlow
        added a comment -

        I'll start with a look at attached JBossAS7H2Tests.zip. One question that I have is about the use of jta-data-source to specify the @DataSourceDefinition DataSource. I don't think obtained JDBC connections are being associated with the JTA transaction.

        Show
        Scott Marlow
        added a comment - I'll start with a look at attached JBossAS7H2Tests.zip. One question that I have is about the use of jta-data-source to specify the @DataSourceDefinition DataSource. I don't think obtained JDBC connections are being associated with the JTA transaction.
        Hide
        Scott Marlow
        added a comment -

        The first time that DataSourceTransactionProxyHandler is invoked with an active JTA transaction, we get a new XAConnection from the datasource, enlist it into the tx and return the XAConnection.getConnection() (autocommit is off for this connection).

        The next call that comes in for the same jta transaction, we repeat this, enlisting the newly obtained XAConnection in the jta transaction. However, the XAConnection.getConnection() connection has autocommit enabled.

        Show
        Scott Marlow
        added a comment - The first time that DataSourceTransactionProxyHandler is invoked with an active JTA transaction, we get a new XAConnection from the datasource, enlist it into the tx and return the XAConnection.getConnection() (autocommit is off for this connection). The next call that comes in for the same jta transaction, we repeat this, enlisting the newly obtained XAConnection in the jta transaction. However, the XAConnection.getConnection() connection has autocommit enabled.
        Hide
        Scott Marlow
        added a comment -

        As a start, will try disabling autocommit.

        Show
        Scott Marlow
        added a comment - As a start, will try disabling autocommit.
        Hide
        Scott Marlow
        added a comment -

        Seems that turning autocommit off isn't enough. Returning separate connections is probably screwing up this particular test (and its use of SAVEPOINT).

        Show
        Scott Marlow
        added a comment - Seems that turning autocommit off isn't enough. Returning separate connections is probably screwing up this particular test (and its use of SAVEPOINT).
        Hide
        Scott Marlow
        added a comment -

        Craig,

        Did you try deploying the separate -ds.xml yet?

        I'm thinking that we should fix the autocommit part of this bug but as far as the other part, which would be caching the db connection with the JTA transaction (like a pool manager would do). I'm thinking that we should integrate with a pool manager for that (which may be a bigger change than we want to make at the moment).

        Fixing the autocommit to always be off in a jta transaction will make this less dangerous. I'll start with that.

        Scott

        Show
        Scott Marlow
        added a comment - Craig, Did you try deploying the separate -ds.xml yet? I'm thinking that we should fix the autocommit part of this bug but as far as the other part, which would be caching the db connection with the JTA transaction (like a pool manager would do). I'm thinking that we should integrate with a pool manager for that (which may be a bigger change than we want to make at the moment). Fixing the autocommit to always be off in a jta transaction will make this less dangerous. I'll start with that. Scott
        Hide
        Scott Marlow
        added a comment -

        changes to disable autocommit are here https://github.com/scottmarlow/jboss-as/commit/070cf0b74f22edc8676ba074aa93e9ec77b156b9

        I think the other change to talk about making, would be handling DataSourceDefinition differently. Similar to how deployment ds.xml are handled (DsXmlDeploymentParsingProcessor + DsXmlDeploymentInstallProcessor). With perhaps some way to hook in an external pool manager if a certain DataSourceDefinition property is specified.

        Show
        Scott Marlow
        added a comment - changes to disable autocommit are here https://github.com/scottmarlow/jboss-as/commit/070cf0b74f22edc8676ba074aa93e9ec77b156b9 I think the other change to talk about making, would be handling DataSourceDefinition differently. Similar to how deployment ds.xml are handled (DsXmlDeploymentParsingProcessor + DsXmlDeploymentInstallProcessor). With perhaps some way to hook in an external pool manager if a certain DataSourceDefinition property is specified.
        Hide
        Stuart Douglas
        added a comment -

        For 7.2 we should definitely change @DataSourceDefinition to use the same mechanism as -ds.xml deployments.

        Show
        Stuart Douglas
        added a comment - For 7.2 we should definitely change @DataSourceDefinition to use the same mechanism as -ds.xml deployments.
        Hide
        Scott Marlow
        added a comment -

        Makes sense to me, should I submit a pull request against https://github.com/scottmarlow/jboss-as/tree/AS7-4552_datasourcedef_3 (contains the commit above). This will disable autocommit (if we are in an active jta tx).

        Show
        Scott Marlow
        added a comment - Makes sense to me, should I submit a pull request against https://github.com/scottmarlow/jboss-as/tree/AS7-4552_datasourcedef_3 (contains the commit above). This will disable autocommit (if we are in an active jta tx).
        Hide
        Scott Marlow
        added a comment -

        Also note a related issue is reported here https://community.jboss.org/message/731192

        Show
        Scott Marlow
        added a comment - Also note a related issue is reported here https://community.jboss.org/message/731192
        Hide
        Stuart Douglas
        added a comment -

        Can you post more details about the ClassCastException you get when use org.postgresql.xa.PGXADataSource? I had a look through the code but I can't see where a ClassCastException would result.

        Show
        Stuart Douglas
        added a comment - Can you post more details about the ClassCastException you get when use org.postgresql.xa.PGXADataSource? I had a look through the code but I can't see where a ClassCastException would result.
        Hide
        Craig Ringer
        added a comment -

        Scott Marlow: Might be worth noting in the commit message title that it's "AS-4552 mitigation" or "AS-4552 workaround".

        Different databases have different behavior when autocommit is off and a connection is closed without an explicit commit or rollback. Most DBs rollback to be safe, but Oracle at least likes to commit the transaction. Is this a concern with this change?

        Is there any risk of statements silently being rolled back without any exception with this change? Or a sequence of operations not committing atomically?

        Thanks so much for taking the time to look at this. I was only using the @DataSourceDefinition for testing purposes so I was at low risk of data loss, and it doesn't affect my main app, but others are still at risk. Thanks for the prompt examination of the problem.

        Show
        Craig Ringer
        added a comment - Scott Marlow: Might be worth noting in the commit message title that it's "AS-4552 mitigation" or "AS-4552 workaround". Different databases have different behavior when autocommit is off and a connection is closed without an explicit commit or rollback. Most DBs rollback to be safe, but Oracle at least likes to commit the transaction. Is this a concern with this change? Is there any risk of statements silently being rolled back without any exception with this change? Or a sequence of operations not committing atomically? Thanks so much for taking the time to look at this. I was only using the @DataSourceDefinition for testing purposes so I was at low risk of data loss, and it doesn't affect my main app, but others are still at risk. Thanks for the prompt examination of the problem.
        Hide
        Stuart Douglas
        added a comment -

        I have done up a prototype that moves the @DataSourceDefinintion handling code into line datasources subsystem, so its data sources are handled by the same code as any other datasource:

        https://github.com/stuartwdouglas/jboss-as/compare/AS-4552

        This does have some user visible changes however, the most obvious one is that you now have to specify the driver in the 'properties' section (i.e. @DataSourceDefinition(properties=

        {"driver=h2"}

        ) )

        Show
        Stuart Douglas
        added a comment - I have done up a prototype that moves the @DataSourceDefinintion handling code into line datasources subsystem, so its data sources are handled by the same code as any other datasource: https://github.com/stuartwdouglas/jboss-as/compare/AS-4552 This does have some user visible changes however, the most obvious one is that you now have to specify the driver in the 'properties' section (i.e. @DataSourceDefinition(properties= {"driver=h2"} ) )
        Hide
        Scott Marlow
        added a comment -

        Maybe we could have a "default DataSourceDefinition properties" configured in standalone.xml. That might be helpful with pre-written code that can't be altered (like compliance tests).

        Show
        Scott Marlow
        added a comment - Maybe we could have a "default DataSourceDefinition properties" configured in standalone.xml. That might be helpful with pre-written code that can't be altered (like compliance tests).
        Hide
        Craig Ringer
        added a comment -

        With this change, AS7 can't support the use of a 3rd party DataSource, so DataSourceDefinition has to specify a java.sql.Driver, right? If so, I'm not sure that's a big improvement over declaring @DataSourceDefinition unsupported/broken and requiring the use of a bundled -ds.xml file instead. It still makes @DataSourceDefinition non-portable and possibly non-compliant.

        Also, what influence will that have on the classpath search for the driver? If an app bundles a driver in its archive that is not also deployed separately to the server, will it be found after this change is applied? It is now. Certainly one can't use an in-war driver with a separately deployed -ds.xml or a jboss-cli created datasource, but I don't know about the case of a bundled -ds.xml .

        It seems like it might be enough to break code that currently works (ish), but not a full fix that allows direct use of a javax.sql.DataSource .

        I'm not complaining, mind you; I appreciate the fact that you folks are looking into this. I'm just trying to help by looking at possible effects of any proposed fix/change.

        If I'm understanding the discussion correctly, it sounds like JBoss AS 7 is relying on the use of its own internal connection pool implementation to co-ordinate with the JTA transaction manager. A DataSource that doesn't use that pool breaks internal assumptions, causing the above confusion and breakage. Correct?

        Show
        Craig Ringer
        added a comment - With this change, AS7 can't support the use of a 3rd party DataSource, so DataSourceDefinition has to specify a java.sql.Driver, right? If so, I'm not sure that's a big improvement over declaring @DataSourceDefinition unsupported/broken and requiring the use of a bundled -ds.xml file instead. It still makes @DataSourceDefinition non-portable and possibly non-compliant. Also, what influence will that have on the classpath search for the driver? If an app bundles a driver in its archive that is not also deployed separately to the server, will it be found after this change is applied? It is now. Certainly one can't use an in-war driver with a separately deployed -ds.xml or a jboss-cli created datasource, but I don't know about the case of a bundled -ds.xml . It seems like it might be enough to break code that currently works (ish), but not a full fix that allows direct use of a javax.sql.DataSource . I'm not complaining, mind you; I appreciate the fact that you folks are looking into this. I'm just trying to help by looking at possible effects of any proposed fix/change. If I'm understanding the discussion correctly, it sounds like JBoss AS 7 is relying on the use of its own internal connection pool implementation to co-ordinate with the JTA transaction manager. A DataSource that doesn't use that pool breaks internal assumptions, causing the above confusion and breakage. Correct?
        Hide
        Craig Ringer
        added a comment -

        Also: I haven't tried a separate -ds.xml yet. On the cards, got side tracked by a blog post the arquillian guys asked for.

        Show
        Craig Ringer
        added a comment - Also: I haven't tried a separate -ds.xml yet. On the cards, got side tracked by a blog post the arquillian guys asked for.
        Hide
        Craig Ringer
        added a comment -

        Stuart: Sorry, missed your comment about the ClassCastException. Here you go:

        21:28:23,794 ERROR [org.jboss.msc.service.fail] (MSC service thread 1-5) MSC00001: Failed to start service jboss.persistenceunit."AS7TxTest-1.0-SNAPSHOT.war#test-PU": org.jboss.msc.service.StartException in service jboss.persistenceunit."AS7TxTest-1.0-SNAPSHOT.war#test-PU": Failed to start service
                at org.jboss.msc.service.ServiceControllerImpl$StartTask.run(ServiceControllerImpl.java:1767) [jboss-msc-1.0.2.GA.jar:1.0.2.GA]
                at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1110) [rt.jar:1.7.0_01]
                at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:603) [rt.jar:1.7.0_01]
                at java.lang.Thread.run(Thread.java:722) [rt.jar:1.7.0_01]
        Caused by: java.lang.ClassCastException: org.postgresql.xa.PGXADataSource$$DataSourceProxy2 cannot be cast to javax.sql.DataSource
                at org.jboss.as.jpa.processor.PersistenceUnitDeploymentProcessor$ManagedReferenceFactoryInjector.inject(PersistenceUnitDeploymentProcessor.java:624)
                at org.jboss.as.jpa.processor.PersistenceUnitDeploymentProcessor$ManagedReferenceFactoryInjector.inject(PersistenceUnitDeploymentProcessor.java:613)
                at org.jboss.msc.inject.CastingInjector.inject(CastingInjector.java:55) [jboss-msc-1.0.2.GA.jar:1.0.2.GA]
                at org.jboss.msc.service.ServiceControllerImpl.doInject(ServiceControllerImpl.java:1549) [jboss-msc-1.0.2.GA.jar:1.0.2.GA]
                at org.jboss.msc.service.ServiceControllerImpl.access$1900(ServiceControllerImpl.java:49) [jboss-msc-1.0.2.GA.jar:1.0.2.GA]
                at org.jboss.msc.service.ServiceControllerImpl$StartTask.performInjections(ServiceControllerImpl.java:1780) [jboss-msc-1.0.2.GA.jar:1.0.2.GA]
                at org.jboss.msc.service.ServiceControllerImpl$StartTask.run(ServiceControllerImpl.java:1741) [jboss-msc-1.0.2.GA.jar:1.0.2.GA]
                ... 3 more
        
        Show
        Craig Ringer
        added a comment - Stuart: Sorry, missed your comment about the ClassCastException. Here you go: 21:28:23,794 ERROR [org.jboss.msc.service.fail] (MSC service thread 1-5) MSC00001: Failed to start service jboss.persistenceunit. "AS7TxTest-1.0-SNAPSHOT.war#test-PU" : org.jboss.msc.service.StartException in service jboss.persistenceunit. "AS7TxTest-1.0-SNAPSHOT.war#test-PU" : Failed to start service at org.jboss.msc.service.ServiceControllerImpl$StartTask.run(ServiceControllerImpl.java:1767) [jboss-msc-1.0.2.GA.jar:1.0.2.GA] at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1110) [rt.jar:1.7.0_01] at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:603) [rt.jar:1.7.0_01] at java.lang. Thread .run( Thread .java:722) [rt.jar:1.7.0_01] Caused by: java.lang.ClassCastException: org.postgresql.xa.PGXADataSource$$DataSourceProxy2 cannot be cast to javax.sql.DataSource at org.jboss.as.jpa.processor.PersistenceUnitDeploymentProcessor$ManagedReferenceFactoryInjector.inject(PersistenceUnitDeploymentProcessor.java:624) at org.jboss.as.jpa.processor.PersistenceUnitDeploymentProcessor$ManagedReferenceFactoryInjector.inject(PersistenceUnitDeploymentProcessor.java:613) at org.jboss.msc.inject.CastingInjector.inject(CastingInjector.java:55) [jboss-msc-1.0.2.GA.jar:1.0.2.GA] at org.jboss.msc.service.ServiceControllerImpl.doInject(ServiceControllerImpl.java:1549) [jboss-msc-1.0.2.GA.jar:1.0.2.GA] at org.jboss.msc.service.ServiceControllerImpl.access$1900(ServiceControllerImpl.java:49) [jboss-msc-1.0.2.GA.jar:1.0.2.GA] at org.jboss.msc.service.ServiceControllerImpl$StartTask.performInjections(ServiceControllerImpl.java:1780) [jboss-msc-1.0.2.GA.jar:1.0.2.GA] at org.jboss.msc.service.ServiceControllerImpl$StartTask.run(ServiceControllerImpl.java:1741) [jboss-msc-1.0.2.GA.jar:1.0.2.GA] ... 3 more
        Hide
        Craig Ringer
        added a comment - - edited

        I've tested -ds.xml deployment with the postgresql tests without success so far. Creating the module and loading it was fuss-free*. I now have:

        [standalone@localhost:9999 /] /subsystem=datasources/jdbc-driver=postgresql-driver:read-resource
        {
            "outcome" => "success",
            "result" => {
                "driver-class-name" => "org.postgresql.Driver",
                "driver-module-name" => "org.postgresql",
                "driver-name" => "postgresql-driver"
            }
        }
        

        If I remove the @DataSourceDefinition from JBossAS7ContainerTransactionsWith3rdPtyDataSource and instead place the following contents in src/main/webapp/WEB-INF/postgresql-ds.xml:

        <?xml version="1.0" encoding="UTF-8"?>
        <datasources>
          <datasource jndi-name="java:app/test-ds" enabled="true" use-java-context="true"  
                pool-name="test-ds-pool">
            <connection-url>jdbc:postgresql:regress</connection-url>
            <driver>postgresql-driver</driver>
            <security>
              <user-name>regress</user-name>
              <password>regress</password>
            </security>
          </datasource>
        </datasources>
        

        where persistence.xml is:

        <?xml version="1.0" encoding="UTF-8"?>
        <persistence version="2.0" xmlns="http://java.sun.com/xml/ns/persistence" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://java.sun.com/xml/ns/persistence http://java.sun.com/xml/ns/persistence/persistence_2_0.xsd">
          <persistence-unit name="test-PU" transaction-type="JTA">
            <jta-data-source>java:app/test-ds</jta-data-source>
            <exclude-unlisted-classes>false</exclude-unlisted-classes>
            <properties/>
          </persistence-unit>
        </persistence>
        

        then the deployment succeeds and so does the transaction test. So it looks like a bundled *-ds.xml file is working, though it's non-portable and requires the manual installation and enabling of a new server module. It's another approach, anyway.

        I'm guessing there's no way for a -ds.xml to reference a driver within the deployment archive, allowing fully self-contained deployments to be produced?

        For the record and for anyone else reading this while troubleshooting, my module was:

        modules/org/postgresql/main:
          module.xml
          postgresql-9.1-901-1.jdbc4.jar
        

        where module.xml was:

        <module xmlns="urn:jboss:module:1.1" name="org.postgresql">
           <!-- Load with jboss-cli command:
                /subsystem=datasources/jdbc-driver=postgresql-driver:add(driver-name=postgresql-driver, driver-class-name=org.postgresql.Driver, driver-module-name=org.postgresql)
           -->
           <resources>
             <resource-root path="postgresql-9.1-901-1.jdbc4.jar"/>
           </resources>
           <dependencies>
             <module name="javax.api"/>
             <module name="javax.transaction.api"/>
           </dependencies>
         </module>
        

        (* though it made me realize how good a module repository for JBoss would be to contain pre-packaged versions of DB drivers etc, like a ports tree. Or stub modules that could fetch the jar archives from maven repos on demand w/o having to bloat the jboss downloads; same deal)

        Show
        Craig Ringer
        added a comment - - edited I've tested -ds.xml deployment with the postgresql tests without success so far. Creating the module and loading it was fuss-free*. I now have: [standalone@localhost:9999 /] /subsystem=datasources/jdbc-driver=postgresql-driver:read-resource { "outcome" => "success" , "result" => { "driver-class-name" => "org.postgresql.Driver" , "driver-module-name" => "org.postgresql" , "driver-name" => "postgresql-driver" } } If I remove the @DataSourceDefinition from JBossAS7ContainerTransactionsWith3rdPtyDataSource and instead place the following contents in src/main/webapp/WEB-INF/postgresql-ds.xml: <?xml version= "1.0" encoding= "UTF-8" ?> <datasources> <datasource jndi-name= "java:app/test-ds" enabled= " true " use-java-context= " true " pool-name= "test-ds-pool" > <connection-url>jdbc:postgresql:regress</connection-url> <driver>postgresql-driver</driver> <security> <user-name>regress</user-name> <password>regress</password> </security> </datasource> </datasources> where persistence.xml is: <?xml version= "1.0" encoding= "UTF-8" ?> <persistence version= "2.0" xmlns= "http: //java.sun.com/xml/ns/persistence" xmlns:xsi= "http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation= "http://java.sun.com/xml/ns/persistence http://java.sun.com/xml/ns/persistence/persistence_2_0.xsd" > <persistence-unit name= "test-PU" transaction-type= "JTA" > <jta-data-source>java:app/test-ds</jta-data-source> <exclude-unlisted-classes> false </exclude-unlisted-classes> <properties/> </persistence-unit> </persistence> then the deployment succeeds and so does the transaction test. So it looks like a bundled *-ds.xml file is working, though it's non-portable and requires the manual installation and enabling of a new server module. It's another approach, anyway. I'm guessing there's no way for a -ds.xml to reference a driver within the deployment archive, allowing fully self-contained deployments to be produced? For the record and for anyone else reading this while troubleshooting, my module was: modules/org/postgresql/main: module.xml postgresql-9.1-901-1.jdbc4.jar where module.xml was: <module xmlns= "urn:jboss:module:1.1" name= "org.postgresql" > <!-- Load with jboss-cli command: /subsystem=datasources/jdbc-driver=postgresql-driver:add(driver-name=postgresql-driver, driver-class-name=org.postgresql.Driver, driver-module-name=org.postgresql) --> <resources> <resource-root path= "postgresql-9.1-901-1.jdbc4.jar" /> </resources> <dependencies> <module name= "javax.api" /> <module name= "javax.transaction.api" /> </dependencies> </module> (* though it made me realize how good a module repository for JBoss would be to contain pre-packaged versions of DB drivers etc, like a ports tree. Or stub modules that could fetch the jar archives from maven repos on demand w/o having to bloat the jboss downloads; same deal)
        Hide
        Stuart Douglas
        added a comment -

        The @DataSourceDefinition annotation in the spec is extremely vague, and other than specifying that a data source must be provided does not specify that its transactional behaviour.

        What we have at the moment satisfies the spec requirements, but it behaves a lot differently to a -ds.xml data source).

        Show
        Stuart Douglas
        added a comment - The @DataSourceDefinition annotation in the spec is extremely vague, and other than specifying that a data source must be provided does not specify that its transactional behaviour. What we have at the moment satisfies the spec requirements, but it behaves a lot differently to a -ds.xml data source).
        Hide
        Craig Ringer
        added a comment -

        Stuart: Thanks for the clarification.

        Doing a bit of reading, @DataSourceDefinition appears to be specified in JSR250 section 2.13. It explicitly names the allowable interfaces the specified class must implement, and doesn't accept java.sql.Driver. Blast; that would've been a nice option to have.

        This clause of JSR250 really helps suggest a resolution to this issue:

        Vendors are not required to support properties that do not normally apply to a
        specific data source type. For example, specifying the transactional property to
        be true but supplying a value for className that implements a data source class
        other than XADataSource may not be supported.

        The above permits JBoss AS 7 to simply refuse to deploy projects using @DataSourceDefinition without transactional=false with a non-XA datasource, or at least to warn about them loudly and refuse to use them with transactional EJB methods. That might be a good idea, to make devs be explicit via transactional=false and (for EJBs) a @TransactionAttribute of NEVER, NOT_SUPPORTED, or (maybe?) SUPPORTED.

        I guess the question then has to be "what's the principle of least surprise?". What'll cause the lowest rate of unintended behavior, misunderstanding, and failure in code using the feature? Possibilities that come to mind for me include some combination of:

        (1) Loud angry warnings in the logs that @DataSourceDefinition is unsafe without an additional property specifying the JDBC driver, and that a -ds.xml should be preferred (with URL to article).

        (2) Simply failing a deployment that uses @DataSourceDefinition with a warning like the above unless transactional=false is specified. Throw if transactional EJB methods are entered with a non-transactional data source. Appears to be spec-legal and may be safest.

        (3) Reworking the JTA / pool / DS interaction to accomodate externally-provided javax.sql.DataSource implementations or XA data sources. Probably must be combined with (2).

        (4) ... ?

        IMO at minimum it should be a deployment error to specify @DataSourceDefinition(class=some.javax.sql.DataSource, transactional=true) ; transactional=true should only be legal with an XA datasource unless AS7 can cache the connections and take care of the transaction management from the regular DataSource its self.

        Show
        Craig Ringer
        added a comment - Stuart: Thanks for the clarification. Doing a bit of reading, @DataSourceDefinition appears to be specified in JSR250 section 2.13. It explicitly names the allowable interfaces the specified class must implement, and doesn't accept java.sql.Driver. Blast; that would've been a nice option to have. This clause of JSR250 really helps suggest a resolution to this issue: Vendors are not required to support properties that do not normally apply to a specific data source type. For example, specifying the transactional property to be true but supplying a value for className that implements a data source class other than XADataSource may not be supported. The above permits JBoss AS 7 to simply refuse to deploy projects using @DataSourceDefinition without transactional=false with a non-XA datasource, or at least to warn about them loudly and refuse to use them with transactional EJB methods. That might be a good idea, to make devs be explicit via transactional=false and (for EJBs) a @TransactionAttribute of NEVER, NOT_SUPPORTED, or (maybe?) SUPPORTED. I guess the question then has to be "what's the principle of least surprise?". What'll cause the lowest rate of unintended behavior, misunderstanding, and failure in code using the feature? Possibilities that come to mind for me include some combination of: (1) Loud angry warnings in the logs that @DataSourceDefinition is unsafe without an additional property specifying the JDBC driver, and that a -ds.xml should be preferred (with URL to article). (2) Simply failing a deployment that uses @DataSourceDefinition with a warning like the above unless transactional=false is specified. Throw if transactional EJB methods are entered with a non-transactional data source. Appears to be spec-legal and may be safest. (3) Reworking the JTA / pool / DS interaction to accomodate externally-provided javax.sql.DataSource implementations or XA data sources. Probably must be combined with (2). (4) ... ? IMO at minimum it should be a deployment error to specify @DataSourceDefinition(class=some.javax.sql.DataSource, transactional=true) ; transactional=true should only be legal with an XA datasource unless AS7 can cache the connections and take care of the transaction management from the regular DataSource its self.
        Hide
        henk de boer
        added a comment -

        For 7.2 we should definitely change @DataSourceDefinition to use the same mechanism as -ds.xml deployments.

        I think this by itself would be a very welcome addition. Having the behavior and performance characteristics of @DataSouceDefinition equal to -ds.xml would be great. It basically would be -ds.xml, just with standardized syntax (in so far the properties match, if the user wants more -ds.xml can always be used).

        Show
        henk de boer
        added a comment - For 7.2 we should definitely change @DataSourceDefinition to use the same mechanism as -ds.xml deployments. I think this by itself would be a very welcome addition. Having the behavior and performance characteristics of @DataSouceDefinition equal to -ds.xml would be great. It basically would be -ds.xml , just with standardized syntax (in so far the properties match, if the user wants more -ds.xml can always be used).
        Hide
        Craig Ringer
        added a comment -

        henk: It can't be, unfortunately, because -ds.xml expects a java.sql.Driver, whereas @DataSourceDefinition expects a javax.sql.DataSource, javax.sql.XADataSource or javax.sql.ConnectionPoolDataSource, and doesn't accept a java.sql.Driver .

        That means that changing it to use the -ds.xml mechanism will require one or more hacks, such as:

        • A properties entry specifying the JDBC driver class name; or
        • Discovering the JDBC driver indirectly by examining the datasource classname, package, metadata, etc or by inspecting connections returned by the datasource.

        From what I've seen in this discussion and my (poor) understanding of the code in question, the -ds.xml mechanism can't just be changed to use a javax.sql.DataSource, as it expects to do its own connection management, pooling, etc.

        As such, is changing how @DataSourceDefinition works to another method that doesn't take the same parameters going to work that well? How will you handle cases where the jdbc driver property is not specified?

        Show
        Craig Ringer
        added a comment - henk: It can't be, unfortunately, because -ds.xml expects a java.sql.Driver, whereas @DataSourceDefinition expects a javax.sql.DataSource, javax.sql.XADataSource or javax.sql.ConnectionPoolDataSource, and doesn't accept a java.sql.Driver . That means that changing it to use the -ds.xml mechanism will require one or more hacks, such as: A properties entry specifying the JDBC driver class name; or Discovering the JDBC driver indirectly by examining the datasource classname, package, metadata, etc or by inspecting connections returned by the datasource. From what I've seen in this discussion and my (poor) understanding of the code in question, the -ds.xml mechanism can't just be changed to use a javax.sql.DataSource, as it expects to do its own connection management, pooling, etc. As such, is changing how @DataSourceDefinition works to another method that doesn't take the same parameters going to work that well? How will you handle cases where the jdbc driver property is not specified?
        Hide
        henk de boer
        added a comment -

        >From what I've seen in this discussion and my (poor) understanding of the code in question, the -ds.xml mechanism can't just be changed to use a javax.sql.DataSource, as it expects to do its own connection management, pooling, etc.

        I would have to inspect the code, but I wonder it that's the case. If I'm not mistaken (and my JDBC knowledge is admittedly a bit rusty), a plain javax.sql.DataSource that's provided by the DB vendor is a direct alternative for java.sql.Driver. In fact, it's the more modern and preferable version. Such DataSource shouldn't do much if any connection management and certainly shouldn't do any pooling.

        This DataSource typically calls the DriverManager (java.sql.DriverManager), which on its turn calls the Driver.connect method. java.sql.Driver#connect can take extra properties, but apart from "user" and "password" these are DB specific. The DataSource implementation can be configured with the same kind of DB specific properties.

        So, in my understanding, the differences between java.sql.Driver and javax.sql.DataSource are rather small and they eventually end up at the same point.

        Also, for XA, -ds.xml does require a javax.sql.XADataSource, not a java.sql.Driver. Other AS implementations, like GlassFish, do all their magic with a javax.sql.DataSource (or XA variant) instead of a Driver.

        Show
        henk de boer
        added a comment - >From what I've seen in this discussion and my (poor) understanding of the code in question, the -ds.xml mechanism can't just be changed to use a javax.sql.DataSource, as it expects to do its own connection management, pooling, etc. I would have to inspect the code, but I wonder it that's the case. If I'm not mistaken (and my JDBC knowledge is admittedly a bit rusty), a plain javax.sql.DataSource that's provided by the DB vendor is a direct alternative for java.sql.Driver . In fact, it's the more modern and preferable version. Such DataSource shouldn't do much if any connection management and certainly shouldn't do any pooling. This DataSource typically calls the DriverManager ( java.sql.DriverManager ), which on its turn calls the Driver.connect method. java.sql.Driver#connect can take extra properties, but apart from "user" and "password" these are DB specific. The DataSource implementation can be configured with the same kind of DB specific properties. So, in my understanding , the differences between java.sql.Driver and javax.sql.DataSource are rather small and they eventually end up at the same point. Also, for XA, -ds.xml does require a javax.sql.XADataSource , not a java.sql.Driver . Other AS implementations, like GlassFish, do all their magic with a javax.sql.DataSource (or XA variant) instead of a Driver.
        Hide
        henk de boer
        added a comment -

        >This does have some user visible changes however, the most obvious one is that you now have to specify the driver in the 'properties' section (i.e. @DataSourceDefinition(properties=

        {"driver=h2"}

        ) )

        That would be a bit troublesome. It would mostly defeat the purpose of having a @DataSourceDefinition (or data-source in web.xml etc).

        Show
        henk de boer
        added a comment - >This does have some user visible changes however, the most obvious one is that you now have to specify the driver in the 'properties' section (i.e. @DataSourceDefinition(properties= {"driver=h2"} ) ) That would be a bit troublesome. It would mostly defeat the purpose of having a @DataSourceDefinition (or data-source in web.xml etc).
        Hide
        Stuart Douglas
        added a comment -

        I have updated that branch so it is no longer necessary to specify the Driver.

        Show
        Stuart Douglas
        added a comment - I have updated that branch so it is no longer necessary to specify the Driver.
        Hide
        Craig Ringer
        added a comment -

        Stuart: git ref?

        Also: are any docs changes necessary as a result of this change? Are there any compatibility concerns, or any differences between @DataSourceDefinition and a -ds.xml remaining? If so I'm happy to write up suitable docs, just need to know the gist of what needs to be covered.

        Show
        Craig Ringer
        added a comment - Stuart: git ref? Also: are any docs changes necessary as a result of this change? Are there any compatibility concerns, or any differences between @DataSourceDefinition and a -ds.xml remaining? If so I'm happy to write up suitable docs, just need to know the gist of what needs to be covered.
        Hide
        henk de boer
        added a comment -

        Like Craig, I'm very curious about the git ref and whether there are any differences between @DataSourceDefinition and a -ds.xml remaining.

        Show
        henk de boer
        added a comment - Like Craig, I'm very curious about the git ref and whether there are any differences between @DataSourceDefinition and a -ds.xml remaining.
        Hide
        jaikiran pai
        added a comment -

        Those of you looking for the git-ref, take a look at the "Git Pull Request" field in this JIRA. That points to the pull request containing this change. Here's it for reference https://github.com/jbossas/jboss-as/pull/2168/commits

        Show
        jaikiran pai
        added a comment - Those of you looking for the git-ref, take a look at the "Git Pull Request" field in this JIRA. That points to the pull request containing this change. Here's it for reference https://github.com/jbossas/jboss-as/pull/2168/commits
        Hide
        henk de boer
        added a comment -

        For reference, I tested the problematic application I reported at https://community.jboss.org/message/731260 on JBoss EAP 6.0.0 and it seems to work nearly perfectly now.

        A tiny small problem is that with the combination of en embedded h2 and an auto-created schema by Hibernate, the DB seems to be closed a bit too when shutting down JBoss. When Hibernate tries to drop the schema, the following exception is thrown:

        org.h2.jdbc.JdbcSQLException: Database is already closed (to disable automatic closing at VM shutdown, add ";DB_CLOSE_ON_EXIT=FALSE" to the db URL) [90121-161]
                at org.h2.message.DbException.getJdbcSQLException(DbException.java:329)
                at org.h2.message.DbException.get(DbException.java:169)
                at org.h2.message.DbException.get(DbException.java:146)
                at org.h2.message.DbException.get(DbException.java:135)
                at org.h2.jdbc.JdbcConnection.checkClosed(JdbcConnection.java:1394)
                at org.h2.jdbcx.JdbcXAConnection$PooledJdbcConnection.checkClosed(JdbcXAConnection.java:475)
                at org.h2.jdbc.JdbcConnection.checkClosed(JdbcConnection.java:1369)
                at org.h2.jdbc.JdbcConnection.createStatement(JdbcConnection.java:191)
                at org.jboss.jca.adapters.jdbc.WrappedConnection.createStatement(WrappedConnection.java:304)
                at org.hibernate.tool.hbm2ddl.DatabaseExporter.<init>(DatabaseExporter.java:54)
                at org.hibernate.tool.hbm2ddl.SchemaExport.execute(SchemaExport.java:367)
                at org.hibernate.tool.hbm2ddl.SchemaExport.drop(SchemaExport.java:318)
                at org.hibernate.tool.hbm2ddl.SchemaExport.drop(SchemaExport.java:314)
                at org.hibernate.internal.SessionFactoryImpl.close(SessionFactoryImpl.java:1446)
                at org.hibernate.ejb.EntityManagerFactoryImpl.close(EntityManagerFactoryImpl.java:194)
                at org.jboss.as.jpa.service.PersistenceUnitServiceImpl.stop(PersistenceUnitServiceImpl.java:98)
                at org.jboss.msc.service.ServiceControllerImpl$StopTask.stopService(ServiceControllerImpl.java:1911)
                at org.jboss.msc.service.ServiceControllerImpl$StopTask.run(ServiceControllerImpl.java:1874)
                at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1110)
                at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:603)
                at java.lang.Thread.run(Thread.java:732)
        

        But this is really minor. Overall, a REALLY great job done by the JBoss team!

        any differences between @DataSourceDefinition and a -ds.xml remaining? If so I'm happy to write up suitable docs, just need to know the gist of what needs to be covered.

        I'm curious about that as well. https://community.jboss.org/wiki/DataSourceConfigurationInAS7 now still mentions @DataSourceDefinition is recommended only for development and testing purposes. Is that still true?

        Show
        henk de boer
        added a comment - For reference, I tested the problematic application I reported at https://community.jboss.org/message/731260 on JBoss EAP 6.0.0 and it seems to work nearly perfectly now. A tiny small problem is that with the combination of en embedded h2 and an auto-created schema by Hibernate, the DB seems to be closed a bit too when shutting down JBoss. When Hibernate tries to drop the schema, the following exception is thrown: org.h2.jdbc.JdbcSQLException: Database is already closed (to disable automatic closing at VM shutdown, add ";DB_CLOSE_ON_EXIT=FALSE" to the db URL) [90121-161] at org.h2.message.DbException.getJdbcSQLException(DbException.java:329) at org.h2.message.DbException.get(DbException.java:169) at org.h2.message.DbException.get(DbException.java:146) at org.h2.message.DbException.get(DbException.java:135) at org.h2.jdbc.JdbcConnection.checkClosed(JdbcConnection.java:1394) at org.h2.jdbcx.JdbcXAConnection$PooledJdbcConnection.checkClosed(JdbcXAConnection.java:475) at org.h2.jdbc.JdbcConnection.checkClosed(JdbcConnection.java:1369) at org.h2.jdbc.JdbcConnection.createStatement(JdbcConnection.java:191) at org.jboss.jca.adapters.jdbc.WrappedConnection.createStatement(WrappedConnection.java:304) at org.hibernate.tool.hbm2ddl.DatabaseExporter.<init>(DatabaseExporter.java:54) at org.hibernate.tool.hbm2ddl.SchemaExport.execute(SchemaExport.java:367) at org.hibernate.tool.hbm2ddl.SchemaExport.drop(SchemaExport.java:318) at org.hibernate.tool.hbm2ddl.SchemaExport.drop(SchemaExport.java:314) at org.hibernate.internal.SessionFactoryImpl.close(SessionFactoryImpl.java:1446) at org.hibernate.ejb.EntityManagerFactoryImpl.close(EntityManagerFactoryImpl.java:194) at org.jboss.as.jpa.service.PersistenceUnitServiceImpl.stop(PersistenceUnitServiceImpl.java:98) at org.jboss.msc.service.ServiceControllerImpl$StopTask.stopService(ServiceControllerImpl.java:1911) at org.jboss.msc.service.ServiceControllerImpl$StopTask.run(ServiceControllerImpl.java:1874) at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1110) at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:603) at java.lang.Thread.run(Thread.java:732) But this is really minor. Overall, a REALLY great job done by the JBoss team! any differences between @DataSourceDefinition and a -ds.xml remaining? If so I'm happy to write up suitable docs, just need to know the gist of what needs to be covered. I'm curious about that as well. https://community.jboss.org/wiki/DataSourceConfigurationInAS7 now still mentions @DataSourceDefinition is recommended only for development and testing purposes. Is that still true?

          People

          • Assignee:
            Stuart Douglas
            Reporter:
            Craig Ringer
          • Votes:
            2 Vote for this issue
            Watchers:
            10 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: