From 925daa3fdb2b5936553482338a8ab869ccfda639 Mon Sep 17 00:00:00 2001 From: Janos Gyerik Date: Tue, 7 May 2019 10:23:26 +0200 Subject: [PATCH] SONAR-12057 Add general locking mechanism through internal_properties --- .../db/property/InternalPropertiesDao.java | 48 +++++++++ .../db/property/InternalPropertiesMapper.java | 6 ++ .../db/property/InternalPropertiesMapper.xml | 8 ++ .../property/InternalPropertiesDaoTest.java | 101 ++++++++++++++++++ 4 files changed, 163 insertions(+) diff --git a/server/sonar-db-dao/src/main/java/org/sonar/db/property/InternalPropertiesDao.java b/server/sonar-db-dao/src/main/java/org/sonar/db/property/InternalPropertiesDao.java index 3039771466b..8d2dfadac25 100644 --- a/server/sonar-db-dao/src/main/java/org/sonar/db/property/InternalPropertiesDao.java +++ b/server/sonar-db-dao/src/main/java/org/sonar/db/property/InternalPropertiesDao.java @@ -43,6 +43,12 @@ import static java.util.Collections.singletonList; public class InternalPropertiesDao implements Dao { + /** + * A common prefix used by locks. {@see InternalPropertiesDao#tryLock} + */ + public static final String LOCK_PREFIX = "lock."; + + static final int KEY_MAX_LENGTH = 20; private static final int TEXT_VALUE_MAX_LENGTH = 4000; private static final Optional OPTIONAL_OF_EMPTY_STRING = Optional.of(""); @@ -174,6 +180,48 @@ public class InternalPropertiesDao implements Dao { return rows.iterator().next(); } + /** + * Try to acquire a lock with the specified name, for specified duration. + * + * Returns false if the lock exists with a timestamp > now - duration, + * or if the atomic replacement of the timestamp fails (another process replaced first). + * + * Returns true if the lock does not exist, or if exists with a timestamp <= now - duration, + * and the atomic replacement of the timestamp succeeds. + * + * The lock is considered released when the specified duration has elapsed. + */ + public boolean tryLock(DbSession dbSession, String name, int maxAgeInSeconds) { + String key = LOCK_PREFIX + '.' + name; + if (key.length() > KEY_MAX_LENGTH) { + throw new IllegalArgumentException("lock name is too long"); + } + + long now = system2.now(); + + Optional timestampAsStringOpt = selectByKey(dbSession, key); + if (!timestampAsStringOpt.isPresent()) { + return tryCreateLock(dbSession, key, String.valueOf(now)); + } + + String oldTimestampString = timestampAsStringOpt.get(); + long oldTimestamp = Long.parseLong(oldTimestampString); + if (oldTimestamp > now - maxAgeInSeconds * 1000) { + return false; + } + + return getMapper(dbSession).replaceValue(key, oldTimestampString, String.valueOf(now)) == 1; + } + + private boolean tryCreateLock(DbSession dbSession, String name, String value) { + try { + getMapper(dbSession).insertAsText(name, value, system2.now()); + return true; + } catch (Exception ignored) { + return false; + } + } + private static void checkKey(@Nullable String key) { checkArgument(key != null && !key.isEmpty(), "key can't be null nor empty"); } diff --git a/server/sonar-db-dao/src/main/java/org/sonar/db/property/InternalPropertiesMapper.java b/server/sonar-db-dao/src/main/java/org/sonar/db/property/InternalPropertiesMapper.java index c38f7c88e5b..0b3f41a4082 100644 --- a/server/sonar-db-dao/src/main/java/org/sonar/db/property/InternalPropertiesMapper.java +++ b/server/sonar-db-dao/src/main/java/org/sonar/db/property/InternalPropertiesMapper.java @@ -34,4 +34,10 @@ public interface InternalPropertiesMapper { void insertAsClob(@Param("key") String key, @Param("value") String value, @Param("createdAt") long createdAt); void deleteByKey(@Param("key") String key); + + /** + * Replace the value of the specified key, only if the existing value matches the expected old value. + * Returns 1 if the replacement succeeded, or 0 if failed (old value different, or record does not exist). + */ + int replaceValue(@Param("key") String key, @Param("oldValue") String oldValue, @Param("newValue") String newValue); } diff --git a/server/sonar-db-dao/src/main/resources/org/sonar/db/property/InternalPropertiesMapper.xml b/server/sonar-db-dao/src/main/resources/org/sonar/db/property/InternalPropertiesMapper.xml index a606d9148a6..1675c5869d8 100644 --- a/server/sonar-db-dao/src/main/resources/org/sonar/db/property/InternalPropertiesMapper.xml +++ b/server/sonar-db-dao/src/main/resources/org/sonar/db/property/InternalPropertiesMapper.xml @@ -75,5 +75,13 @@ kee=#{key,jdbcType=VARCHAR} + + UPDATE internal_properties + set + text_value = #{newValue,jdbcType=VARCHAR} + where + kee = #{key,jdbcType=VARCHAR} + and text_value = #{oldValue,jdbcType=VARCHAR} + diff --git a/server/sonar-db-dao/src/test/java/org/sonar/db/property/InternalPropertiesDaoTest.java b/server/sonar-db-dao/src/test/java/org/sonar/db/property/InternalPropertiesDaoTest.java index 5c7967aa417..c995168d02a 100644 --- a/server/sonar-db-dao/src/test/java/org/sonar/db/property/InternalPropertiesDaoTest.java +++ b/server/sonar-db-dao/src/test/java/org/sonar/db/property/InternalPropertiesDaoTest.java @@ -43,12 +43,19 @@ import org.sonar.db.DbTester; import static java.lang.Boolean.FALSE; import static java.lang.Boolean.TRUE; +import static org.apache.commons.lang.RandomStringUtils.randomAlphabetic; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.entry; +import static org.mockito.ArgumentMatchers.anyLong; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyNoMoreInteractions; import static org.mockito.Mockito.when; +import static org.sonar.db.property.InternalPropertiesDao.KEY_MAX_LENGTH; +import static org.sonar.db.property.InternalPropertiesDao.LOCK_PREFIX; public class InternalPropertiesDaoTest { @@ -396,6 +403,100 @@ public class InternalPropertiesDaoTest { verifyNoMoreInteractions(mapperMock); } + @Test + public void tryLock_succeeds_if_lock_did_not_exist() { + long now = new Random().nextInt(); + when(system2.now()).thenReturn(now); + assertThat(underTest.tryLock(dbSession, A_KEY, 60)).isTrue(); + + assertThat(underTest.selectByKey(dbSession, key(A_KEY))).contains(String.valueOf(now)); + } + + @Test + public void tryLock_succeeds_if_lock_acquired_before_lease_duration() { + int lockDurationSeconds = 60; + + long before = new Random().nextInt(); + when(system2.now()).thenReturn(before); + assertThat(underTest.tryLock(dbSession, A_KEY, lockDurationSeconds)).isTrue(); + + long now = before + lockDurationSeconds * 1000; + when(system2.now()).thenReturn(now); + assertThat(underTest.tryLock(dbSession, A_KEY, lockDurationSeconds)).isTrue(); + + assertThat(underTest.selectByKey(dbSession, key(A_KEY))).contains(String.valueOf(now)); + } + + @Test + public void tryLock_fails_if_lock_acquired_within_lease_duration() { + long now = new Random().nextInt(); + when(system2.now()).thenReturn(now); + assertThat(underTest.tryLock(dbSession, A_KEY, 60)).isTrue(); + assertThat(underTest.tryLock(dbSession, A_KEY, 60)).isFalse(); + + assertThat(underTest.selectByKey(dbSession, key(A_KEY))).contains(String.valueOf(now)); + } + + @Test + public void tryLock_fails_if_it_would_insert_concurrently() { + String name = randomAlphabetic(5); + String key = key(name); + + long now = new Random().nextInt(); + when(system2.now()).thenReturn(now); + assertThat(underTest.tryLock(dbSession, name, 60)).isTrue(); + + InternalPropertiesMapper mapperMock = mock(InternalPropertiesMapper.class); + DbSession dbSessionMock = mock(DbSession.class); + when(dbSessionMock.getMapper(InternalPropertiesMapper.class)).thenReturn(mapperMock); + when(mapperMock.selectAsText(ImmutableList.of(key))) + .thenReturn(ImmutableList.of()); + doThrow(RuntimeException.class).when(mapperMock).insertAsText(eq(key), anyString(), anyLong()); + + assertThat(underTest.tryLock(dbSessionMock, name, 60)).isFalse(); + + assertThat(underTest.selectByKey(dbSession, key)).contains(String.valueOf(now)); + } + + @Test + public void tryLock_fails_if_concurrent_caller_succeeded_first() { + int lockDurationSeconds = 60; + String name = randomAlphabetic(5); + String key = key(name); + + long now = 123456;//new Random().nextInt(); + long oldTimestamp = now - lockDurationSeconds * 1000; + when(system2.now()).thenReturn(oldTimestamp); + assertThat(underTest.tryLock(dbSession, name, lockDurationSeconds)).isTrue(); + when(system2.now()).thenReturn(now); + + InternalPropertiesMapper mapperMock = mock(InternalPropertiesMapper.class); + DbSession dbSessionMock = mock(DbSession.class); + when(dbSessionMock.getMapper(InternalPropertiesMapper.class)).thenReturn(mapperMock); + InternalPropertyDto dto = new InternalPropertyDto(); + dto.setKey(key); + dto.setValue(String.valueOf(oldTimestamp - 1)); + when(mapperMock.selectAsText(ImmutableList.of(key))) + .thenReturn(ImmutableList.of(dto)); + + assertThat(underTest.tryLock(dbSessionMock, name, lockDurationSeconds)).isFalse(); + + assertThat(underTest.selectByKey(dbSession, key)).contains(String.valueOf(oldTimestamp)); + } + + @Test + public void tryLock_throws_if_lock_name_would_produce_too_long_key() { + String tooLongName = randomAlphabetic(KEY_MAX_LENGTH - LOCK_PREFIX.length()); + + expectedException.expect(IllegalArgumentException.class); + expectedException.expectMessage("lock name is too long"); + underTest.tryLock(dbSession, tooLongName, 60); + } + + private String key(String name) { + return LOCK_PREFIX + '.' + name; + } + private void expectKeyNullOrEmptyIAE() { expectedException.expect(IllegalArgumentException.class); expectedException.expectMessage("key can't be null nor empty"); -- 2.39.5