Conversation
This comment has been minimized.
This comment has been minimized.
Test Results405 tests 390 ✅ 29m 58s ⏱️ Results for commit 3aaafd5. ♻️ This comment has been updated with latest results. |
This comment has been minimized.
This comment has been minimized.
|
|
📦 Pushed preview images as 🚢 See on GHCR. Use by referencing with full name as printed above, mind the registry name. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.



What this PR does / why we need it:
Fixes the flaky search tests (
SearchIT,DataRetrieverApiIT, see #12710 and #12658) and shortens the index waits in the ITs.Commands call
asyncIndexDatasetfrom inside their transaction, and the background job can run before that transaction commits. When it does, the permission doc misses the creator and the index time is never written (thedvObjectToModify is nullerrors in the server log), so the dataset stays invisible and the tests wait for nothing. This has been possible since indexing on create became async in #9558; the container based CI is fast enough to hit it.Index jobs, the index time update and role reindexing now fire a CDI event that runs after the transaction has committed (
IndexingRequest,IndexingRequestObserver). No callers change, and the 1 s sleep inIndexAsyncgoes away.On the test side
sleepForReindexpolls every 100 ms instead of sleeping a fixed 1.5 s, the dev/CI Solr soft commits every 100 ms, andsleepForDatasetIndexfails on timeout instead of warning.Which issue(s) this PR closes:
Special notes for your reviewer:
Verified against the Docker dev stack: both classes pass, 0 index-time errors in the server log (CI runs had 25 to 80),
SearchITin 137 s instead of 409 s.Suggestions on how to test this:
Run
SearchITandDataRetrieverApiITagainst the dev stack with-Ddataverse.test.solr.softcommit.millis=100, then check thatdocker logs dev_dataverse 2>&1 | grep -c 'dvObjectToModify" is null'prints 0.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?:
Yes: indexing now starts only after the transaction that changed a dataset or its permissions has committed, so a new dataset no longer risks being missing from search for its creator until the next reindex.
Additional documentation:
The testing guide documents
-Ddataverse.test.solr.softcommit.millis.