diff --git a/api/src/org/labkey/api/exp/property/Domain.java b/api/src/org/labkey/api/exp/property/Domain.java index 3c0d7b1c1c6..34e2640a650 100644 --- a/api/src/org/labkey/api/exp/property/Domain.java +++ b/api/src/org/labkey/api/exp/property/Domain.java @@ -88,7 +88,7 @@ public interface Domain extends IPropertyType * This pattern effectively forces all callers who are trying to manipulate this domain to queue up. */ Lock getDatabaseLock(); - void lockForDelete(DbSchema expSchema); + void lockForUpdateDelete(DbSchema lockSchema); void delete(@Nullable User user) throws DomainNotFoundException; default void delete(@Nullable User user, @Nullable String auditUserComment) throws DomainNotFoundException diff --git a/api/src/org/labkey/api/exp/property/DomainUtil.java b/api/src/org/labkey/api/exp/property/DomainUtil.java index af2309f42c8..4310bbd2a64 100644 --- a/api/src/org/labkey/api/exp/property/DomainUtil.java +++ b/api/src/org/labkey/api/exp/property/DomainUtil.java @@ -54,6 +54,7 @@ import org.labkey.api.exp.PropertyDescriptor; import org.labkey.api.exp.PropertyType; import org.labkey.api.exp.TemplateInfo; +import org.labkey.api.exp.api.ExperimentService; import org.labkey.api.exp.api.SampleTypeDomainKind; import org.labkey.api.exp.api.StorageProvisioner; import org.labkey.api.gwt.client.AuditBehaviorType; @@ -785,6 +786,10 @@ public static ValidationException updateDomainDescriptor(GWTDomain kind = d.getDomainKind(); ValidationException validationException = validateProperties(d, update, kind, orig, user); diff --git a/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java b/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java index 367d743da66..6a5dc219038 100644 --- a/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java +++ b/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java @@ -1263,11 +1263,14 @@ public void indexDataClass(ExpDataClassImpl dataClass, SearchService.TaskIndexin if (table == null) return; - // Index the data class if it has never been indexed OR it has changed since it was last indexed + indexDataClassData(dataClass, q); + + // GitHub Issue 783: Server lockup when updating data class domain design + // Index DataClass after data indexing, to avoid holding locks on exp.DataClass table for too long SQLFragment sql = new SQLFragment("SELECT * FROM ") .append(getTinfoDataClass(), "dc") .append(" WHERE dc.LSID = ?").add(dataClass.getLSID()) - .append(" AND (dc.lastIndexed IS NULL OR dc.lastIndexed < ?)") + .append(" AND (dc.lastIndexed IS NULL OR dc.lastIndexed < ?)") // Index the data class if it has never been indexed OR it has changed since it was last indexed .add(dataClass.getModified()); DataClass dClass = new SqlSelector(getExpSchema().getScope(), sql).getObject(DataClass.class); @@ -1276,8 +1279,6 @@ public void indexDataClass(ExpDataClassImpl dataClass, SearchService.TaskIndexin ExpDataClassImpl impl = new ExpDataClassImpl(dClass); impl.index(q, table); } - - indexDataClassData(dataClass, q); }); } @@ -4627,7 +4628,7 @@ private static void _lockDomainsAndProvisionedTables(AssayService assayService, { for (var domain : provider.getDomains(expProtocol)) { - domain.lockForDelete(expSchema); + domain.lockForUpdateDelete(expSchema); } } } @@ -8046,7 +8047,7 @@ public ValidationException updateDataClass(@NotNull Container c, @NotNull User u if (!errors.hasErrors()) { transaction.addCommitTask(() -> clearDataClassCache(c), DbScope.CommitTaskOption.IMMEDIATE, POSTCOMMIT, POSTROLLBACK); - transaction.addCommitTask(() -> indexDataClass(getDataClass(c, dataClass.getName()), SearchService.get().defaultTask().getQueue(c, SearchService.PRIORITY.modified)), POSTCOMMIT); + transaction.addCommitTask(() -> indexDataClass(dataClass, SearchService.get().defaultTask().getQueue(c, SearchService.PRIORITY.modified)), POSTCOMMIT); transaction.commit(); } } diff --git a/experiment/src/org/labkey/experiment/api/SampleTypeServiceImpl.java b/experiment/src/org/labkey/experiment/api/SampleTypeServiceImpl.java index af04d68fa32..94b1feb99a9 100644 --- a/experiment/src/org/labkey/experiment/api/SampleTypeServiceImpl.java +++ b/experiment/src/org/labkey/experiment/api/SampleTypeServiceImpl.java @@ -331,6 +331,10 @@ public void indexSampleType(ExpSampleType sampleType, SearchService.TaskIndexing return; queue.addRunnable((q) -> { + indexSampleTypeMaterials(sampleType, q); + + // GitHub Issue 783: Server lockup when updating data class domain design + // Index MaterialSource after materials indexing, to avoid holding locks on exp.MaterialSource table for too long // Index all ExpMaterial that have never been indexed OR where either the ExpSampleType definition or ExpMaterial itself has changed since last indexed SQLFragment sql = new SQLFragment("SELECT * FROM ") .append(getTinfoMaterialSource(), "ms") @@ -345,8 +349,6 @@ public void indexSampleType(ExpSampleType sampleType, SearchService.TaskIndexing ExpSampleTypeImpl impl = new ExpSampleTypeImpl(materialSource); impl.index(q, null); } - - indexSampleTypeMaterials(sampleType, q); }); } diff --git a/experiment/src/org/labkey/experiment/api/property/DomainImpl.java b/experiment/src/org/labkey/experiment/api/property/DomainImpl.java index 62390100c31..2f7186ce200 100644 --- a/experiment/src/org/labkey/experiment/api/property/DomainImpl.java +++ b/experiment/src/org/labkey/experiment/api/property/DomainImpl.java @@ -405,7 +405,7 @@ public void delete(@Nullable User user, @Nullable String auditUserComment) throw ExperimentService exp = ExperimentService.get(); try (DbScope.Transaction transaction = exp.getSchema().getScope().ensureTransaction()) { - lockForDelete(exp.getSchema()); + lockForUpdateDelete(exp.getSchema()); DefaultValueService.get().clearDefaultValues(getContainer(), this); OntologyManager.deleteDomain(getTypeURI(), getContainer()); StorageProvisioner.get().drop(this); @@ -434,19 +434,19 @@ public Lock getDatabaseLock() } @Override - public void lockForDelete(DbSchema expSchema) + public void lockForUpdateDelete(DbSchema lockSchema) { // NOTE code relies on the lock returned from Domain.getLock() does not require unlock(). var lock = getDatabaseLock(); assert lock instanceof DbScope.ServerLock; - assert ExperimentService.get().getSchema().getScope().isTransactionActive(); + assert lockSchema.getScope().isTransactionActive(); lock.lock(); // CONSIDER verify table exists: SELECT 1 FROM pg_tables WHERE schemaname = ? AND tablename = ? - if (null != getStorageTableName() && expSchema.getSqlDialect().isPostgreSQL()) + if (null != getStorageTableName() && lockSchema.getSqlDialect().isPostgreSQL()) { - SQLFragment lockSQL = new SQLFragment().append("LOCK TABLE ").appendDottedIdentifiers(getDomainKind().getStorageSchemaName(), getStorageTableName()).append(" IN EXCLUSIVE MODE").appendEOS().append("\n"); - new SqlExecutor(expSchema).execute(lockSQL); + SQLFragment lockSQL = new SQLFragment().append("LOCK TABLE ").appendDottedIdentifiers(getDomainKind().getStorageSchemaName(), getStorageTableName()).append(" IN ACCESS EXCLUSIVE MODE").appendEOS().append("\n"); + new SqlExecutor(lockSchema).execute(lockSQL); } }