Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[#654] H2 Database locking issue #166

Closed
wants to merge 1 commit into from
Closed

[#654] H2 Database locking issue #166

wants to merge 1 commit into from

Conversation

alexanderjarvis
Copy link
Contributor

No description provided.

@orefalo
Copy link
Contributor

orefalo commented Apr 3, 2011

Maybe a little more details would be helpful, what do you think ?

Here is what the H2 doc says:

Transaction isolation is provided for all data manipulation language (DML) statements. Most data definition language (DDL) statements commit the current transaction. See the Grammar for details.

This database supports the following transaction isolation levels:

Read Committed
This is the default level. Read locks are released immediately after executing the statement, but write locks are kept until the transaction commits. Higher concurrency is possible when using this level.
To enable, execute the SQL statement SET LOCK_MODE 3
or append ;LOCK_MODE=3 to the database URL: jdbc:h2:/test;LOCK_MODE=3
Serializable
Both read locks and write locks are kept until the transaction commits. To enable, execute the SQL statement SET LOCK_MODE 1
or append ;LOCK_MODE=1 to the database URL: jdbc:h2:
/test;LOCK_MODE=1
Read Uncommitted
This level means that transaction isolation is disabled.
To enable, execute the SQL statement SET LOCK_MODE 0
or append ;LOCK_MODE=0 to the database URL: jdbc:h2:~/test;LOCK_MODE=0

In other words, appending LOCK_MODE=0 turns off transaction isolation.

Why would you want this pulled as part of the core ?

@alexanderjarvis
Copy link
Contributor Author

Thanks for your comment.

The problem that I was having was related to the FunctionalTests and loading Fixtures.

If you look at the lighthouse bug I have included a sample project (with tests) to show the original exception that was occurring:
http://play.lighthouseapp.com/projects/57987/tickets/654-severe-error-with-functionaltest-and-h2-database-engine#ticket-654-3

I am not 100% sure if this is the best solution, but it is the only one I could get to work for all cases. The problem (as I see it) is that the Fixtures data is not being committed properly and the transaction is not closed and so when the Controller code tries to access the data model, the table is locked.

Please also note that H2 provides table (not row) based locking by default and that this can be changed by adding MVCC=TRUE to the db url. I tried this, but then instead of the locking exception, the data did not exist for my second test (reloading the database for each test) and so I tried adding flush(), merge(), and even creating a new entity manager inside the @before method of the JUnit test in the hope that the data would be committed.

Perhaps another solution that I have not tried yet would be to wrap the Fixtures.loadModels() method in a custom transaction using the JPAPlugin.startTx() method.

@orefalo
Copy link
Contributor

orefalo commented Apr 3, 2011

IMHO, turning off transaction isolation is the worst thing to do.

Ok, I can reproduce your issue - looks like a transaction issue in Fixtures. My guess... Fixtures.load doesn't commit the jpa transaction.

@Before
public void setUp() {
    Fixtures.deleteAllModels();
    Fixtures.load("data.yml");
    Cache.clear();

}

@Test
public void testUsers() {
    response = GET(BASE_CONTROLLER_PATH + "?client_id=bob@gmail.com");
    assertIsOk(response);
}

PS: when you submit a pull request, I would recommend to include the lighthouse ticket url and vice versa

@guillaumebort
Copy link
Contributor

Well, I can agree with Olivier than disabling transaction isolation is not the best idea, but it's just for the development and test database.

It looks like the previous in memory database (hsqldb) used 'read uncommitted' as isolation level. So we should probably disable transaction isolation for h2 as well for compatibility reasons.

So I will apply this fix. Anyway you can still starts h2 using a different isolation level by providing your own URL.

@orefalo
Copy link
Contributor

orefalo commented Apr 4, 2011

You're the boss Guillaume

@alexanderjarvis
Copy link
Contributor Author

Thanks Guillaume.

I tried providing the db URL as a configuration property, but it was overridden by the code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants