[LMS-814] added obtaining explicit lock before insert to samples table both on DAO and DB level

SVN: 10604

openbis/source/java/ch/systemsx/cisd/openbis/generic/server/dataaccess/db/SampleDAO.java

@@ -37,6 +37,7 @@ import ch.systemsx.cisd.openbis.generic.shared.dto.GroupPE;
 import ch.systemsx.cisd.openbis.generic.shared.dto.HierarchyType;
 import ch.systemsx.cisd.openbis.generic.shared.dto.SamplePE;
 import ch.systemsx.cisd.openbis.generic.shared.dto.SampleTypePE;
+import ch.systemsx.cisd.openbis.generic.shared.dto.TableNames;
 
 /**
  * Implementation of {@link ISampleDAO} for databases.
@@ -51,12 +52,14 @@ public class SampleDAO extends AbstractDAO implements ISampleDAO
 
     /**
      * This logger does not output any SQL statement. If you want to do so, you had better set an
-     * appropriate debugging level for class {@link JdbcAccessor}.
-     * </p>
+     * appropriate debugging level for class {@link JdbcAccessor}. </p>
      */
     private static final Logger operationLog =
             LogFactory.getLogger(LogCategory.OPERATION, SampleDAO.class);
 
+    private static final String LOCK_TABLE_SQL =
+            "LOCK TABLE " + TableNames.SAMPLES_TABLE + " IN EXCLUSIVE MODE";
+
     SampleDAO(final SessionFactory sessionFactory, final DatabaseInstancePE databaseInstance)
     {
         super(sessionFactory, databaseInstance);
@@ -87,11 +90,29 @@ public class SampleDAO extends AbstractDAO implements ISampleDAO
         }
     }
 
+    /**
+     * Obtains an explicit exclusive lock on 'samples' table. This function should always be
+     * executed before saving a sample because we have a complex unique code check in a trigger and
+     * we don't want any race condition or deadlock (if lock is gathered in the trigger). See
+     * [LMS-814] for details.
+     */
+    private final void lockTable()
+    {
+        executeUpdate(LOCK_TABLE_SQL);
+    }
+
+    /**
+     * <b>IMPORTANT</b> - every method which executes this method should first obtain lock on table
+     * using {@link SampleDAO#lockTable()}. The obtained lock is reentrant so this method could as
+     * well obtain it itself with a small additional cost if there are many saves in one
+     * transaction.
+     */
     private final void internalCreateSample(final SamplePE sample,
             final HibernateTemplate hibernateTemplate)
     {
         validatePE(sample);
         sample.setCode(CodeConverter.tryToDatabase(sample.getCode()));
+
         hibernateTemplate.saveOrUpdate(sample);
         if (operationLog.isInfoEnabled())
         {
@@ -108,6 +129,7 @@ public class SampleDAO extends AbstractDAO implements ISampleDAO
         assert sample != null : "Unspecified sample";
 
         final HibernateTemplate hibernateTemplate = getHibernateTemplate();
+        lockTable();
         internalCreateSample(sample, hibernateTemplate);
         hibernateTemplate.flush();
     }
@@ -236,6 +258,7 @@ public class SampleDAO extends AbstractDAO implements ISampleDAO
         assert samples != null && samples.size() > 0 : "Unspecified or empty samples.";
 
         final HibernateTemplate hibernateTemplate = getHibernateTemplate();
+        lockTable();
         for (final SamplePE samplePE : samples)
         {
             internalCreateSample(samplePE, hibernateTemplate);

openbis/source/sql/postgresql/031/function-031.sql

@@ -78,6 +78,7 @@ CREATE OR REPLACE FUNCTION SAMPLE_CODE_UNIQUENESS_CHECK() RETURNS trigger AS $$
 DECLARE
    counter  INTEGER;
 BEGIN
+	LOCK TABLE samples IN EXCLUSIVE MODE;
 	IF (NEW.samp_id_part_of is NULL) THEN
 		IF (NEW.dbin_id is not NULL) THEN
 			SELECT count(*) into counter FROM samples 

openbis/source/sql/postgresql/migration/migration-030-031.sql

@@ -100,3 +100,45 @@ insert into data_types
  ,'MULTILINE_VARCHAR'
  ,'Long text'
 );
+
+-- -------
+-- Added explicit lock Samples table in exclusive mode to prevent race condition
+-- -------
+
+CREATE OR REPLACE FUNCTION SAMPLE_CODE_UNIQUENESS_CHECK() RETURNS trigger AS $$
+DECLARE
+   counter  INTEGER;
+BEGIN
+	LOCK TABLE samples IN EXCLUSIVE MODE;
+	IF (NEW.samp_id_part_of is NULL) THEN
+		IF (NEW.dbin_id is not NULL) THEN
+			SELECT count(*) into counter FROM samples 
+				where id != NEW.id and code = NEW.code and samp_id_part_of is NULL and dbin_id = NEW.dbin_id;
+			IF (counter > 0) THEN
+				RAISE EXCEPTION 'Insert/Update of Sample (Code: %) failed because database instance sample with the same code already exists.', NEW.code;
+			END IF;
+		ELSIF (NEW.grou_id is not NULL) THEN
+			SELECT count(*) into counter FROM samples 
+				where id != NEW.id and code = NEW.code and samp_id_part_of is NULL and grou_id = NEW.grou_id;
+			IF (counter > 0) THEN
+				RAISE EXCEPTION 'Insert/Update of Sample (Code: %) failed because group sample with the same code already exists.', NEW.code;
+			END IF;
+		END IF;
+        ELSE
+		IF (NEW.dbin_id is not NULL) THEN
+			SELECT count(*) into counter FROM samples 
+				where id != NEW.id and code = NEW.code and samp_id_part_of = NEW.samp_id_part_of and dbin_id = NEW.dbin_id;
+			IF (counter > 0) THEN
+				RAISE EXCEPTION 'Insert/Update of Sample (Code: %) failed because database instance sample with the same code and being the part of the same parent already exists.', NEW.code;
+			END IF;
+		ELSIF (NEW.grou_id is not NULL) THEN
+			SELECT count(*) into counter FROM samples 
+				where id != NEW.id and code = NEW.code and samp_id_part_of = NEW.samp_id_part_of and grou_id = NEW.grou_id;
+			IF (counter > 0) THEN
+				RAISE EXCEPTION 'Insert/Update of Sample (Code: %) failed because group sample with the same code and being the part of the same parent already exists.', NEW.code;
+			END IF;
+		END IF;
+        END IF;   
+   RETURN NEW;
+END;
+$$ LANGUAGE 'plpgsql';