diff --git a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/dataaccess/db/SampleDAO.java b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/dataaccess/db/SampleDAO.java index 8544f483fe979cb6fe704f888caaa53ecf932e4d..6d86d2feba237de4be611e3e4e613fe43d56d62b 100644 --- a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/dataaccess/db/SampleDAO.java +++ b/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); diff --git a/openbis/source/sql/postgresql/031/function-031.sql b/openbis/source/sql/postgresql/031/function-031.sql index 2d588758fc480517dfd4544a51daab8db1ac198b..518c4ce6e2e1a0bede43b02852f7b7166ed1e607 100644 --- a/openbis/source/sql/postgresql/031/function-031.sql +++ b/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 diff --git a/openbis/source/sql/postgresql/migration/migration-030-031.sql b/openbis/source/sql/postgresql/migration/migration-030-031.sql index 2004ec95f62c45089d9f3ebb86f702ce657cbb06..d461bf354581a40a26386fd0a148d774cf3e069c 100644 --- a/openbis/source/sql/postgresql/migration/migration-030-031.sql +++ b/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';