Skip to content

GitHub Issue #783: Server lockup when updating data class domain design #7332

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
XingY wants to merge 10 commits into
base: develop
Choose a base branch
from
+14 −9
Open

GitHub Issue #783: Server lockup when updating data class domain design #7332

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
4586f8f
GitHub Issue #783: Server lockup when updating data class domain design
XingY Jan 15, 2026
4b6a6d2
lock provisioned table on domain update
XingY Jan 19, 2026
9e42f38
Merge branch 'develop' into fb_dataclassLock
XingY Jan 19, 2026
d1da5c7
lock
XingY Jan 19, 2026
2e3dab8
transaction
XingY Jan 19, 2026
10e4700
revert transaction
XingY Jan 19, 2026
5e8f69e
revert transaction
XingY Jan 19, 2026
c5017f3
Merge branch 'develop' into fb_dataclassLock
XingY Jan 20, 2026
075770b
Merge branch 'develop' into fb_dataclassLock
XingY Jan 22, 2026
bbf426a
revert indexing order
XingY Jan 22, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion api/src/org/labkey/api/exp/property/Domain.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Copy link
Contributor

@labkey-jeckels labkey-jeckels Jan 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know this isn't from your change, but I don't understand why we're passing this as an argument. It looks like all callers are using the exp schema.


void delete(@Nullable User user) throws DomainNotFoundException;
default void delete(@Nullable User user, @Nullable String auditUserComment) throws DomainNotFoundException
Expand Down
5 changes: 5 additions & 0 deletions api/src/org/labkey/api/exp/property/DomainUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,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;
Expand Down Expand Up @@ -799,6 +800,10 @@ public static ValidationException updateDomainDescriptor(GWTDomain<? extends GWT
return validationException;
}

var lockSchema = ExperimentService.get().getSchema();
if (lockSchema.getScope().isTransactionActive())
d.lockForUpdateDelete(lockSchema);
Copy link
Contributor

@labkey-matthewb labkey-matthewb Jan 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This lock is very heavy-handed. Is it possible to only acquire this if/when we know we have actual changes to the provisioned table?

Copy link
Contributor Author

@XingY XingY Jan 21, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That might be too late though. In this case the other transaction acquired shared lock before the current transaction perform Add column, resulting in failure to Add column, because the other transaction is also waiting for the current transaction to release write lock on exp.dataclass.

What we really need I think is READ "nolock", which postgres doesn't seem to support.


DomainKind<?> kind = d.getDomainKind();
ValidationException validationException = validateProperties(d, update, kind, orig, user);

Expand Down
4 changes: 2 additions & 2 deletions experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4627,7 +4627,7 @@ private static void _lockDomainsAndProvisionedTables(AssayService assayService,
{
for (var domain : provider.getDomains(expProtocol))
{
domain.lockForDelete(expSchema);
domain.lockForUpdateDelete(expSchema);
}
}
}
Expand Down Expand Up @@ -8046,7 +8046,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();
}
}
Expand Down
12 changes: 6 additions & 6 deletions experiment/src/org/labkey/experiment/api/property/DomainImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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);
}
}

Expand Down