diff --git a/api/src/org/labkey/api/exp/property/Domain.java b/api/src/org/labkey/api/exp/property/Domain.java index 3c0d7b1c1c6..8e5460427a1 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(); void delete(@Nullable User user) throws DomainNotFoundException; default void delete(@Nullable User user, @Nullable String auditUserComment) throws DomainNotFoundException diff --git a/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java b/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java index 367d743da66..1d34569cbee 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(); } } } @@ -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..baca719ab9c 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(); DefaultValueService.get().clearDefaultValues(getContainer(), this); OntologyManager.deleteDomain(getTypeURI(), getContainer()); StorageProvisioner.get().drop(this); @@ -434,18 +434,19 @@ public Lock getDatabaseLock() } @Override - public void lockForDelete(DbSchema expSchema) + public void lockForUpdateDelete() { // 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(); + DbSchema expSchema = ExperimentService.get().getSchema(); + assert expSchema.getScope().isTransactionActive(); lock.lock(); // CONSIDER verify table exists: SELECT 1 FROM pg_tables WHERE schemaname = ? AND tablename = ? if (null != getStorageTableName() && expSchema.getSqlDialect().isPostgreSQL()) { - SQLFragment lockSQL = new SQLFragment().append("LOCK TABLE ").appendDottedIdentifiers(getDomainKind().getStorageSchemaName(), getStorageTableName()).append(" IN EXCLUSIVE MODE").appendEOS().append("\n"); + SQLFragment lockSQL = new SQLFragment().append("LOCK TABLE ").appendDottedIdentifiers(getDomainKind().getStorageSchemaName(), getStorageTableName()).append(" IN ACCESS EXCLUSIVE MODE").appendEOS().append("\n"); new SqlExecutor(expSchema).execute(lockSQL); } } diff --git a/experiment/src/org/labkey/experiment/controllers/property/PropertyController.java b/experiment/src/org/labkey/experiment/controllers/property/PropertyController.java index 76a98ccc8a9..0a894bceb14 100644 --- a/experiment/src/org/labkey/experiment/controllers/property/PropertyController.java +++ b/experiment/src/org/labkey/experiment/controllers/property/PropertyController.java @@ -47,11 +47,13 @@ import org.labkey.api.data.ContainerService; import org.labkey.api.data.DbSchema; import org.labkey.api.data.DbSchemaType; +import org.labkey.api.data.DbScope; import org.labkey.api.data.ExpDataFileConverter; import org.labkey.api.data.NameExpressionValidationResult; import org.labkey.api.data.SimpleFilter; import org.labkey.api.defaults.DefaultValueService; import org.labkey.api.exp.ChangePropertyDescriptorException; +import org.labkey.api.exp.DomainDescriptor; import org.labkey.api.exp.Identifiable; import org.labkey.api.exp.Lsid; import org.labkey.api.exp.LsidManager; @@ -60,6 +62,7 @@ import org.labkey.api.exp.TemplateInfo; import org.labkey.api.exp.api.DomainKindDesign; import org.labkey.api.exp.api.ExperimentJSONConverter; +import org.labkey.api.exp.api.ExperimentService; import org.labkey.api.exp.api.ExperimentUrls; import org.labkey.api.exp.property.Domain; import org.labkey.api.exp.property.DomainKind; @@ -618,12 +621,19 @@ public void validateForm(DomainApiForm form, Errors errors) public Object execute(DomainApiForm form, BindException errors) { GWTDomain newDomain = form.getDomainDesign(); - GWTDomain originalDomain = getDomain(form.getSchemaName(), form.getQueryName(), form.getDomainId(), getContainer(), getUser()); boolean includeWarnings = form.includeWarnings(); boolean hasErrors = false; + ValidationException updateErrors; - ValidationException updateErrors = updateDomain(originalDomain, newDomain, form.getOptions(), getContainer(), getUser(), includeWarnings, form.getAuditUserComment()); + try (DbScope.Transaction tx = ExperimentService.get().ensureTransaction()) + { + // GitHub Issue #783: Server lockup when updating data class domain design + GWTDomain originalDomain = getDomain(form.getSchemaName(), form.getQueryName(), form.getDomainId(), getContainer(), getUser(), true); + updateErrors = updateDomain(originalDomain, newDomain, form.getOptions(), getContainer(), getUser(), includeWarnings, form.getAuditUserComment()); + + tx.commit(); + } for (ValidationError ve : updateErrors.getErrors()) { @@ -1630,6 +1640,11 @@ private static void deleteDomain(String schemaName, String queryName, Container @NotNull private static GWTDomain getDomain(String schemaName, String queryName, Integer domainId, @NotNull Container container, @NotNull User user) throws NotFoundException + { + return getDomain(schemaName, queryName, domainId, container, user, false); + } + @NotNull + private static GWTDomain getDomain(String schemaName, String queryName, Integer domainId, @NotNull Container container, @NotNull User user, boolean getForUpdate) throws NotFoundException { if ((schemaName == null || queryName == null) && domainId == null) { @@ -1637,9 +1652,10 @@ private static GWTDomain getDomain(String schemaName, String queryName, Integ } GWTDomain domain; + Domain dom; if (domainId != null) { - Domain dom = PropertyService.get().getDomain(domainId); + dom = PropertyService.get().getDomain(domainId); if (dom == null) throw new NotFoundException("Could not find domain for " + domainId + "."); @@ -1651,12 +1667,23 @@ private static GWTDomain getDomain(String schemaName, String queryName, Integ else { String domainURI = PropertyService.get().getDomainURI(schemaName, queryName, container, user); - domain = DomainUtil.getDomainDescriptor(user, domainURI, container); - if (domain == null) + DomainDescriptor dd = OntologyManager.getDomainDescriptor(domainURI, container); + if (null != dd) + { + dom = PropertyService.get().getDomain(dd.getDomainId()); + if (dom == null) + throw new NotFoundException("Could not find domain for schemaName=" + schemaName + ", queryName=" + queryName + "."); + + domain = DomainUtil.getDomainDescriptor(container, user, dom, false); + } + else throw new NotFoundException("Could not find domain for schemaName=" + schemaName + ", queryName=" + queryName + "."); } + if (getForUpdate) + dom.lockForUpdateDelete(); + return domain; }