Skip to content

GitHub Issue 1708: persist sample dashboard and insights stats - #8231

Merged
XingY merged 18 commits into
developfrom
fb_sampleCount
Oct 30, 2026
Merged

XingY merged 18 commits into
developfrom
fb_sampleCount

Conversation

@XingY

@XingY XingY commented Oct 26, 2026 •

Copy link
Copy Markdown
Contributor

Rationale

Lets a module maintain sample counts ahead of time and serve them wherever the server used to count samples live. The insights panel and the home dashboard charts recount every sample for every user on each page load, which on a large installation is among the most expensive queries a regular user can trigger, and the sample type list's count column has the same problem. Platform owns the sample write paths and the sample type table, so it has to tell a module when samples change and be able to ask it for a count, without itself knowing anything about how those counts are stored. GitHub Issue 1708: https://github.com/LabKey/internal-issues/issues/1708

Related Pull Requests

Changes

  • Sample change notifications now carry which sample types changed in which folder, and let a module record that inside the transaction making the change and hold the client notification back until its counts are current.
  • Deletes and cross-folder moves notify for every folder whose samples actually changed, rather than only the folder the operation was invoked in.
  • The sample type list's sample count reads counts a module maintains where one is installed, falling back to counting samples directly wherever those counts aren't current.

Comment on lines 124 to 128
SQLFragment liveCount = new SQLFragment("(SELECT COUNT(*) FROM ").append(ExperimentServiceImpl.get().getTinfoMaterial(), "m")
.unsafeAppend(" WHERE m.CpasType = ").append(ExprColumn.STR_TABLE_ALIAS + ".LSID")
.unsafeAppend(" AND ")
.append(sampleCountContainerFilter.getSQLFragment(getSchema(), new SQLFragment("m.container")))
.unsafeAppend(")");

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Move this into an else under the if statement below?

}
catch (Exception e)
{
LOG.error("Sample counts listener failed to record a change in " + container.getPath()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

SQLExceptions will abort the whole transaction. We probably can't continue and need to bail out.

* what lets these run after the container itself is gone.
*/

private static class DeindexAssayBatchesTask extends AccumulatingCommitTask<String, ExpExperiment>

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@labkey-jeckels I included some unrelated AccumulatingCommitTask switch in this branch to address the warnings logged while running tests.

@XingY
XingY requested a review from labkey-jeckels October 28, 2026 15:49
@XingY
XingY merged commit 9146f29 into develop Oct 30, 2026
14 of 15 checks passed
@XingY
XingY deleted the fb_sampleCount branch October 30, 2026 20:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants