]> source.dussan.org Git - sonarqube.git/commitdiff
SONAR-12057 Add general locking mechanism through internal_properties
authorJanos Gyerik <janos.gyerik@sonarsource.com>
Tue, 7 May 2019 08:23:26 +0000 (10:23 +0200)
committerSonarTech <sonartech@sonarsource.com>
Wed, 15 May 2019 18:21:11 +0000 (20:21 +0200)
server/sonar-db-dao/src/main/java/org/sonar/db/property/InternalPropertiesDao.java
server/sonar-db-dao/src/main/java/org/sonar/db/property/InternalPropertiesMapper.java
server/sonar-db-dao/src/main/resources/org/sonar/db/property/InternalPropertiesMapper.xml
server/sonar-db-dao/src/test/java/org/sonar/db/property/InternalPropertiesDaoTest.java

index 3039771466b6cee15f627118e8fc24050999263d..8d2dfadac251cdb2f83b7dec6fbd81198e16a55b 100644 (file)
@@ -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<String> 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<String> 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");
   }
index c38f7c88e5b7a6fd992124c9b245a744a109dd23..0b3f41a4082d1140ae51e7f914ca9c1dd6dcea58 100644 (file)
@@ -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);
 }
index a606d9148a62597ce81dd76786f54957d671f58d..1675c5869d85f49205ad3a6d801a1268cdb6a8fc 100644 (file)
       kee=#{key,jdbcType=VARCHAR}
   </delete>
 
+  <update id="replaceValue" parameterType="Map">
+    UPDATE internal_properties
+    set
+      text_value = #{newValue,jdbcType=VARCHAR}
+    where
+      kee = #{key,jdbcType=VARCHAR}
+      and text_value = #{oldValue,jdbcType=VARCHAR}
+  </update>
 
 </mapper>
index 5c7967aa417c02b0e513d0cf70308a590ba08300..c995168d02af1314b280dc59bbdc5b8273ad9b2e 100644 (file)
@@ -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");