Faster file and dataset pages for datasets with many files

[ErykKul]

What this PR does / why we need it:

Loading and reading the files of a dataset version ran database queries per file, and for tabular files per variable. For a dataset with 64,278 files, one file page took about 17 seconds.

• Eager relations, loaded with every file (weaving is off, so LAZY is ignored): FileMetadata.dataFile, DataFile.ingestRequest, DataFile.thumbnailForDataset, DvObject.storageQuota.
• Lazy collections read per file by the exports and the file listing JSON: FileMetadata.fileCategories, FileMetadata.varGroups, DataFile.dataTables, DataFile.dataFileTags.
• Per tabular file and variable: DataTable.dataVariables, DataVariable.invalidRanges, summaryStatistics, categories, variableMetadatas.

These get EclipseLink @BatchFetch(BatchFetchType.IN), so each is loaded for up to 500 items per query. IN rather than the default JOIN, so that paged file queries still load only their page.

The dataset page had two more costs that grow with the number of files:

• DatasetVersionServiceBean.findDeep loaded the version with all files and ten of their relations in one joined query, which EclipseLink turns into objects slowly. It now loads the version alone; the files come with the batch fetching above. The named query DatasetVersion.findById no longer joins the files either: EclipseLink refuses to batch fetch for objects built from a join (error 6169). DataFile.embargo and DataFile.retention get @BatchFetch too.
• Without a search term, the page asked Solr for all files of the version, with all stored fields, only to read the facet counts. It now asks for no rows then, and for the file ids only when it needs them.

Dataset.findById and DatasetServiceBean.findDeep, used by GET /api/datasets/{id} and by indexing, joined all files of the dataset with 18 of their relations in the same way. With the batch fetching above, this failed with EclipseLink 6169 (found in review). They now load the dataset alone. The API response is unchanged: compared byte for byte with develop for datasets with 10,000 and 64,000 files, by id and by persistent id, as admin and anonymous. For 64,000 files the response starts after 1.7 s instead of 3.5 s.

Which issue(s) this PR closes:

• Closes File page loads every file of the dataset version, very slow for datasets with many files #12739

Special notes for your reviewer:

DatasetVersionFileMetadatasPerformanceIT runs five operations on a smaller and a larger dataset and fails if the select queries grow with the number of files or variables: checking for restricted files, export file details (regular and tabular), the file listing JSON, and the schema.org JSON-LD. All five fail on develop (for example 409 vs 809 queries for the export of 50 vs 100 files) and pass here (17 vs 17). TabularDataExportIT goes from 8,037 select queries to 34, HugeDatasetExportPerformanceIT from 8,009 to 25. A sixth check loads a version like the dataset page and reads the relations findDeep used to join (16 queries for both sizes); it fails with EclipseLink 6169 if the files are joined in the query again. A seventh does the same for a dataset loaded like DatasetServiceBean.findDeep (10 queries for both sizes). The test fixtures now set the dataset as the owner of its files, as in a real installation; without that, the dataset had no files to load.

VariableMetadata.categoriesMetadata is also read per variable, but the test fixtures cannot create variable metadata yet, so it is left out.

Suggestions on how to test this:

mvn test-compile failsafe:integration-test -DskipUnitTests=true -Dit.test='DatasetVersionFileMetadatasPerformanceIT,TabularDataExportIT,HugeDatasetExportPerformanceIT' (needs Docker). On an installation with a large dataset, open a file page and the dataset page before and after. Locally, for a published dataset with 10,000 files, the file page took 0.73 to 1.06 s on develop (36d1f0f) and 0.25 to 0.34 s with this PR. For a published dataset with 64,000 files, the dataset page took 4.2 s (median, time to first byte) with only the file page changes, 3.0 s with the Solr change and 1.8 s with both. On our production installation (6.7.1 with these commits), the dataset page of a dataset with 64,278 files went from 9.5 s with only the file page changes to about 3 s.

Does this PR introduce a user interface change? If mockups are available, please link/include them here:

No.

Is there a release notes update needed for this change?:

Included.

Additional documentation:

None.

[coveralls]

coverage: 25.501% (+0.09%) from 25.411% — 12739-file-page-many-files into develop

[github-actions]

Test Results

406 tests  ±0   391 ✅ ±0   34m 7s ⏱️ +7s
 55 suites ±0    15 💤 ±0 
 55 files   ±0     0 ❌ ±0 

Results for commit 6282364. ± Comparison against base commit 2553300.

♻️ This comment has been updated with latest results.

[pdurbin]

@ErykKul I'm just putting this here but please investigate this failing test: at edu.harvard.iq.dataverse.api.DatasetsIT.testSummaryDatasetVersionsDifferencesAPI(DatasetsIT.java:6765)

[ErykKul]

@pdurbin It looks like the reason is a race condition making the tests flaky. It did help me to find something I did not yet address in #12712 , I will commit the change there. Here I am just re-running the tests.

[qqmyers]

Looks good w.r.t. code review, so I'll approve so we can move to QA. I made a couple comments but the only change I suggest is to simplify the release note.

The one concern from AI was that this change means findDeep calls no longer assure everything is loaded so if we use the object outside the initial context, some relations may not be loaded. I don't think we do that in general, but it's possible that we do that in workflows or some onSuccess and/or async calls. I'm going to do some testing/QA in those areas at QDR. (It doesn't sound to hard to fix that - either you iterate though to sure things load, or otherwise adapt to not rely on everything already being loaded in those, hopefully rare, cases.)

[qqmyers]

Just a note that AI says BatchFetch isn't optimal for OneToOne fields, but this is better than no annotation given that we don't have weaving on to allow lazy fetch, when most values are null, etc. So we might want to revisit these if/when we get weaving working (I tried it once pre-AI and didn't manage to fix the issues that arose.)

[ErykKul]

Agreed.

[qqmyers]

Another AI note about batchfetch - it suggests using it as hints on specific queries instead of as an annotation. I don't think that's worth worrying about right now though.

[ErykKul]

Agreed.

[qqmyers]

Why aren't fields like this made batchfetch? Is it just scope of the PR or are these fields somehow different? (I note this one was dropped from the findDeep hints in previous PRs).

[ErykKul]

Deliberate. No code reads DataFile.getGuestbookResponses(); responses are queried through the service bean, and the collection is only loaded by the cascade when a file is deleted. It is one row per download, so batching it would only add queries and memory. Same for auxiliary files, ingest reports and access requests. The annotated relations are the ones the page, API, export and indexing read per file.

[qqmyers]

FWIW: I'd call this an improvement rather than bug fix, but my main comment would be a suggestion to simplify the note, e.g.: The number of queries required to load datasets and files has been greatly reduced for large datasets. An additional change improves the loading time for solr facets on the dataset page. The result is significantly faster dataset and file page and api response times. See #12739.

(The suggested phrasing above makes it sound like the file API is also faster like the dataset one - is that the case? I was guessing it might be but didn't check. If not - faster dataset and file page loading and dataset api response times.)

[ErykKul]

Done, with your wording. The file listing API benefits too: 7 queries per file before, none now (new IT case).

[ErykKul]

@qqmyers
Thanks! On the concern: in EclipseLink a relation that is not loaded yet is not an error state. The first access runs the query, also on a detached object after the transaction has ended, because the value holder keeps its own session. The code relies on that already: the dataset page reads the files after the service bean's transaction has ended, and the async export after publishing reads the tabular variables, which findDeep never join fetched. Workflows re-find or merge the dataset before using it. To be sure the batches also work on detached objects, I added a test case that loads a version, closes the entity manager and then reads the file relations: they load, in the same number of queries as inside the transaction. So I don't think we need code to force loading. The release note is simplified as you suggested, and QA at QDR is welcome, of course.

[sonarqubecloud]

Quality Gate failed

Failed conditions
0.0% Coverage on New Code (required ≥ 80%)

See analysis details on SonarQube Cloud

[github-actions]

📦 Pushed preview images as

ghcr.io/gdcc/dataverse:12739-file-page-many-files

ghcr.io/gdcc/configbaker:12739-file-page-many-files

🚢 See on GHCR. Use by referencing with full name as printed above, mind the registry name.

[landreev]

I see that this appears to work as intended.
But, dumb question:
What is the difference between find() and findDeep() at this point, and do we really need the latter at all? - I may be missing something that it adds...

[landreev]

This prod. dataset with 1,577 files has been used as the benchmark for the total query count behind rendering the dataset page:
https://dataverse.harvard.edu/dataset.xhtml?persistentId=doi:10.7910/DVN/29236

The result stayed virtually unchanged, at ~2,100+ queries since at least 6.7.1; was last measured 10 days ago for 6.12 (2,136 queries).

Testing with this PR reduces the number to 591.

A decent bulk of the savings comes from eliminating (N = number of files) queries on the 1:1 relationship to StorageQuota. This is great of course; but I've been thinking lately that there was likely never a good reason for StorageQuota to be a standalone table, rather than just an added column in dataverse/dataset. I implemented StorageUse as a standalone table, so that it could be incremented/adjusted in real time (especially on large/busy collections with many child objects), without having to modify the main table. ... And I must have replicated that setup for quotas without thinking much - but that was likely a mistake.

[landreev]

Locust test.
Comparing the results for the 2 runs, before vs after this PR was added:

• Obvious, measurable improvement on dataset.xhtml for datasets with many files;
• No significant difference on dataset pages with few files (for ex., doi:10.7910/DVN/29669) - also expected;
• Also, apparent measurable improvement (2X) on /dataverse/harvard and /api/search?q=*. Less intuitive/expected, but must be a function of a reduced overall load on the application/database.

This is our standard, heavier-load test - 250 pseudousers bombarding the site, with 5-20 sec. sleeps between calls.