Improve semaphore for DB migration executor

1 files changed, 12 insertions, 37 deletions

diff --git a/server/sonar-server/src/main/java/org/sonar/server/platform/db/migration/DatabaseMigrationImpl.java b/server/sonar-server/src/main/java/org/sonar/server/platform/db/migration/DatabaseMigrationImpl.java
index 8201832634d..571f6b4ec20 100644
--- a/server/sonar-server/src/main/java/org/sonar/server/platform/db/migration/DatabaseMigrationImpl.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/platform/db/migration/DatabaseMigrationImpl.java
@@ -20,17 +20,15 @@
 package org.sonar.server.platform.db.migration;
 
 import java.util.Date;
-import java.util.concurrent.atomic.AtomicBoolean;
-import java.util.concurrent.locks.ReentrantLock;
+import java.util.concurrent.Semaphore;
 import org.sonar.api.utils.log.Logger;
 import org.sonar.api.utils.log.Loggers;
 import org.sonar.core.util.logs.Profiler;
 import org.sonar.server.platform.Platform;
+import org.sonar.server.platform.db.migration.DatabaseMigrationState.Status;
 import org.sonar.server.platform.db.migration.engine.MigrationEngine;
 import org.sonar.server.platform.db.migration.step.MigrationStepExecutionException;
 
-import static org.sonar.server.platform.db.migration.DatabaseMigrationState.Status;
-
 /**
  * Handles concurrency to make sure only one DB migration can run at a time.
  */
@@ -46,18 +44,9 @@ public class DatabaseMigrationImpl implements DatabaseMigration {
   private final Platform platform;
   private final MutableDatabaseMigrationState migrationState;
   /**
-   * This lock implements thread safety from concurrent calls of method {@link #startIt()}
-   */
-  private final ReentrantLock lock = new ReentrantLock();
-
-  /**
-   * This property acts as a semaphore to make sure at most one db migration task is created at a time.
-   * <p>
-   * It is set to {@code true} by the first thread to execute the {@link #startIt()} method and set to {@code false}
-   * by the thread executing the db migration.
-   * </p>
+   * This semaphore implements thread safety from concurrent calls of method {@link #startIt()}
    */
-  private final AtomicBoolean running = new AtomicBoolean(false);
+  private final Semaphore semaphore = new Semaphore(1);
 
   public DatabaseMigrationImpl(DatabaseMigrationExecutorService executorService, MutableDatabaseMigrationState migrationState,
     MigrationEngine migrationEngine, Platform platform) {
@@ -69,32 +58,18 @@ public class DatabaseMigrationImpl implements DatabaseMigration {
 
   @Override
   public void startIt() {
-    if (lock.isLocked() || this.running.get()) {
-      LOGGER.trace("{}: lock is already taken or process is already running", Thread.currentThread().getName());
-      return;
-    }
-
-    if (lock.tryLock()) {
+    if (semaphore.tryAcquire()) {
       try {
-        startAsynchronousDBMigration();
-      } finally {
-        lock.unlock();
+        executorService.execute(this::doDatabaseMigration);
+      } catch (RuntimeException e) {
+        semaphore.release();
+        throw e;
       }
+    } else {
+      LOGGER.trace("{}: lock is already taken or process is already running", Thread.currentThread().getName());
     }
   }
 
-  /**
-   * This method is not thread safe and must be externally protected from concurrent executions.
-   */
-  private void startAsynchronousDBMigration() {
-    if (this.running.get()) {
-      return;
-    }
-
-    running.set(true);
-    executorService.execute(this::doDatabaseMigration);
-  }
-
   private void doDatabaseMigration() {
     migrationState.setStatus(Status.RUNNING);
     migrationState.setStartedAt(new Date());
@@ -115,7 +90,7 @@ public class DatabaseMigrationImpl implements DatabaseMigration {
       LOGGER.error("Container restart failed", t);
       saveStatus(t);
     } finally {
-      running.getAndSet(false);
+      semaphore.release();
     }
   }