From c044bc52f76cd83318261e00683176b3b852baf8 Mon Sep 17 00:00:00 2001 From: rob bygrave Date: Thu, 22 Oct 2020 16:37:18 +1300 Subject: [PATCH] #2089 - Postgres - Use NO KEY with FOR UPDATE clauses with Postgres --- .../dbplatform/postgres/PostgresPlatform.java | 6 +- .../org/tests/basic/TestQueryForUpdate.java | 13 ++- .../basic/TestQueryForUpdatePostgresLock.java | 86 +++++++++++++++++++ 3 files changed, 100 insertions(+), 5 deletions(-) create mode 100644 ebean-core/src/test/java/org/tests/basic/TestQueryForUpdatePostgresLock.java diff --git a/ebean-api/src/main/java/io/ebean/config/dbplatform/postgres/PostgresPlatform.java b/ebean-api/src/main/java/io/ebean/config/dbplatform/postgres/PostgresPlatform.java index 19584b789..abe233a71 100644 --- a/ebean-api/src/main/java/io/ebean/config/dbplatform/postgres/PostgresPlatform.java +++ b/ebean-api/src/main/java/io/ebean/config/dbplatform/postgres/PostgresPlatform.java @@ -116,11 +116,11 @@ public class PostgresPlatform extends DatabasePlatform { protected String withForUpdate(String sql, Query.ForUpdate forUpdateMode) { switch (forUpdateMode) { case SKIPLOCKED: - return sql + " for update skip locked"; + return sql + " for no key update skip locked"; case NOWAIT: - return sql + " for update nowait"; + return sql + " for no key update nowait"; default: - return sql + " for update"; + return sql + " for no key update"; } } diff --git a/ebean-core/src/test/java/org/tests/basic/TestQueryForUpdate.java b/ebean-core/src/test/java/org/tests/basic/TestQueryForUpdate.java index 45c5d5b9f..cbb808f4b 100644 --- a/ebean-core/src/test/java/org/tests/basic/TestQueryForUpdate.java +++ b/ebean-core/src/test/java/org/tests/basic/TestQueryForUpdate.java @@ -21,7 +21,6 @@ import static org.junit.Assert.assertTrue; public class TestQueryForUpdate extends BaseTestCase { - @Test @ForPlatform({Platform.H2, Platform.ORACLE, Platform.POSTGRES, Platform.SQLSERVER, Platform.MYSQL, Platform.MARIADB}) public void testForUpdate() { @@ -35,6 +34,8 @@ public class TestQueryForUpdate extends BaseTestCase { query.findList(); if (isSqlServer()) { assertThat(sqlOf(query)).contains("with (updlock)"); + } else if (isPostgres()) { + assertThat(sqlOf(query)).contains("for no key update"); } else { assertThat(sqlOf(query)).contains("for update"); } @@ -67,7 +68,11 @@ public class TestQueryForUpdate extends BaseTestCase { if (isH2() || isPostgres()) { assertSql(sql.get(0)).contains("from e_basic t0 where t0.id ="); assertSql(sql.get(1)).contains("from e_basic t0 where t0.id ="); - assertSql(sql.get(1)).contains("for update"); + if (isPostgres()) { + assertSql(sql.get(1)).contains("for no key update"); + } else { + assertSql(sql.get(1)).contains("for update"); + } } transaction.end(); @@ -91,6 +96,8 @@ public class TestQueryForUpdate extends BaseTestCase { assertThat(sqlOf(query)).contains("for update"); } else if (isSqlServer()) { assertThat(sqlOf(query)).contains("with (updlock,nowait)"); + } else if (isPostgres()) { + assertThat(sqlOf(query)).contains("for no key update nowait"); } else { assertThat(sqlOf(query)).contains("for update nowait"); } @@ -116,6 +123,8 @@ public class TestQueryForUpdate extends BaseTestCase { assertThat(sqlOf(query)).contains("with (updlock,nowait)"); } else if (isH2()) { assertThat(sqlOf(query)).contains("for update"); + } else if (isPostgres()) { + assertThat(sqlOf(query)).contains("for no key update nowait"); } else { assertThat(sqlOf(query)).contains("for update nowait"); } diff --git a/ebean-core/src/test/java/org/tests/basic/TestQueryForUpdatePostgresLock.java b/ebean-core/src/test/java/org/tests/basic/TestQueryForUpdatePostgresLock.java new file mode 100644 index 000000000..1f20b2254 --- /dev/null +++ b/ebean-core/src/test/java/org/tests/basic/TestQueryForUpdatePostgresLock.java @@ -0,0 +1,86 @@ +package org.tests.basic; + +import io.ebean.BaseTestCase; +import io.ebean.DB; +import io.ebean.annotation.ForPlatform; +import io.ebean.annotation.Platform; +import io.ebean.annotation.Transactional; +import org.junit.Test; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.tests.model.basic.Article; +import org.tests.model.basic.Section; + +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.TimeUnit; + +import static org.assertj.core.api.Assertions.assertThat; + +public class TestQueryForUpdatePostgresLock extends BaseTestCase { + + private static final Logger log = LoggerFactory.getLogger(TestQueryForUpdatePostgresLock.class); + + private long timePreInsert; + private long timePostInsert; + private long timePreLock; + private long timePostLock; + + @Test + @ForPlatform(Platform.POSTGRES) + public void testForUpdatePostgresLock() throws InterruptedException { + + Article article = new Article("lockTest", "auth"); + DB.save(article); + final Integer id = article.getId(); + + ExecutorService exec = Executors.newFixedThreadPool(2); + exec.submit(() -> lockArticle(id)); + exec.submit(() -> insertSection(id)); + exec.awaitTermination(2, TimeUnit.SECONDS); + exec.shutdown(); + + // assert the lock was obtained before the insert was attempted + assertThat(timePreLock).isLessThan(timePreInsert); + // assert that the insert wasn't waiting on the lock to complete + assertThat(timePostInsert).isLessThan(timePostLock); + } + + /** + * This holds row lock on article for 1 second. + * With FOR NO KEY UPDATE this does not block the insert. + */ + @Transactional + private void lockArticle(Integer id) { + timePreLock = System.currentTimeMillis(); + log.info("lock start"); + DB.find(Article.class).setId(id).forUpdate().findOne(); + sleep(1000); + timePostLock = System.currentTimeMillis(); + log.info("lock done"); + } + + /** + * This inserts with FK to the article that is locked. + * With FOR NO KEY UPDATE this insert does not wait on the lock. + */ + @Transactional + private void insertSection(Integer id) { + sleep(100); + log.info("insert start"); + timePreInsert = System.currentTimeMillis(); + Section section = new Section(); + section.setArticle(DB.getReference(Article.class, id)); + DB.save(section); + timePostInsert = System.currentTimeMillis(); + log.info("inserted"); + } + + private void sleep(int millis) { + try { + Thread.sleep(millis); + } catch (InterruptedException e) { + throw new RuntimeException(e); + } + } +}