Skip to content
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.

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

[PERFORMANCE] Opening the samples leads to not acceptable loading times #8610

Closed
Tracked by #7734
markusmann-vg opened this issue Mar 29, 2022 · 5 comments · Fixed by #9127
Closed
Tracked by #7734

[PERFORMANCE] Opening the samples leads to not acceptable loading times #8610

markusmann-vg opened this issue Mar 29, 2022 · 5 comments · Fixed by #9127
Assignees
Labels
backend Affects the web backend change A change of an existing feature (ticket type) laboratory Samples, pathogen tests and lab messages performance Issues that are meant to increase the performance of the application qa-verified Issue has been tested and verified by QA

Comments

@markusmann-vg
Copy link

markusmann-vg commented Mar 29, 2022

Feature Description

As a user with the role clinician or case office the loading times of the samples should be reasonable.

Problem Description

In some German health departments the loading times for opening the samples directorytakes way too long. The health departments are reporting loading times up to 2,5 minutes.
The Long Query Logs can be found here

Additional info

It seems that for samples the problem occurs with Surveillance Officer, Clinician, contact supervisor and the importing user

Possible Alternatives

Additional Information

@markusmann-vg markusmann-vg added important change A change of an existing feature (ticket type) performance Issues that are meant to increase the performance of the application labels Mar 29, 2022
@StefanKock StefanKock changed the title [PERFORMANCE] Opening the samples and tasks directory lead to not acceptable loading times [PERFORMANCE] Opening the samples and lead to not acceptable loading times Mar 31, 2022
@StefanKock StefanKock added laboratory Samples, pathogen tests and lab messages backend Affects the web backend labels Mar 31, 2022
@MartinWahnschaffe
Copy link
Contributor

See #8637

@StefanKock
Copy link
Contributor

StefanKock commented Apr 7, 2022

I checked the SQL of SampleFacade.getIndexList and it shows known patterns of not reused joins for jurisdiction checks and user filters.

...
LEFT OUTER JOIN Contact contact5_ ON sample0_.associatedContact_id=contact5_.id
LEFT OUTER JOIN District district6_ ON contact5_.district_id=district6_.id
LEFT OUTER JOIN cases case7_ ON contact5_.caze_id=case7_.id
LEFT OUTER JOIN District district8_ ON case7_.district_id=district8_.id
LEFT OUTER JOIN Person person9_ ON contact5_.person_id=person9_.id
LEFT OUTER JOIN cases case10_ ON contact5_.caze_id=case10_.id  -- <- duplicates case7_ (jurisdiction)
LEFT OUTER JOIN cases case11_ ON contact5_.caze_id=case11_.id  -- <- duplicates case7_ (jurisdiction)
LEFT OUTER JOIN cases case12_ ON contact5_.caze_id=case12_.id  -- <- duplicates case7_ (jurisdiction)
LEFT OUTER JOIN cases case13_ ON contact5_.caze_id=case13_.id  -- <- duplicates case7_ (user filter)
LEFT OUTER JOIN Contact contacts14_ ON case13_.id=contacts14_.caze_id
LEFT OUTER JOIN EventParticipant eventparti15_ ON sample0_.associatedEventParticipant_id=eventparti15_.id
LEFT OUTER JOIN EVENTS event16_ ON eventparti15_.event_id=event16_.id
LEFT OUTER JOIN LOCATION location17_ ON event16_.eventLocation_id=location17_.id
LEFT OUTER JOIN District district18_ ON location17_.district_id=district18_.id
LEFT OUTER JOIN Person person19_ ON eventparti15_.person_id=person19_.id
LEFT OUTER JOIN EVENTS event20_ ON eventparti15_.event_id=event20_.id  -- <- duplicates event16_ (jurisdiction)
LEFT OUTER JOIN LOCATION location21_ ON event20_.eventLocation_id=location21_.id
LEFT OUTER JOIN EVENTS event22_ ON eventparti15_.event_id=event22_.id  -- <- duplicates event16_ (jurisdiction)
LEFT OUTER JOIN LOCATION location23_ ON event22_.eventLocation_id=location23_.id
LEFT OUTER JOIN EVENTS event24_ ON eventparti15_.event_id=event24_.id  -- <- duplicates event16_ (user filter)
LEFT OUTER JOIN LOCATION location25_ ON event24_.eventLocation_id=location25_.id
LEFT OUTER JOIN users user26_ ON event24_.reportingUser_id=user26_.id  -- <- not needed join, fixed with #8637
LEFT OUTER JOIN users user27_ ON event24_.responsibleUser_id=user27_.id  -- <- not needed join, fixed with #8637
LEFT OUTER JOIN cases case28_ ON eventparti15_.resultingCase_id=case28_.id
LEFT OUTER JOIN Contact contacts29_ ON case28_.id=contacts29_.caze_id
LEFT OUTER JOIN Facility facility30_ ON sample0_.lab_id=facility30_.id
LEFT OUTER JOIN samples sample31_ ON sample0_.referredTo_id=sample31_.id
LEFT OUTER JOIN users user32_ ON sample0_.reportingUser_id=user32_.id  -- <- not needed join, could be changed in SampleJurisdictionPredicateValidator

SampleFacade.getIndexList BEFORE.sql.txt

@StefanKock StefanKock changed the title [PERFORMANCE] Opening the samples and lead to not acceptable loading times Opening the samples and lead to not acceptable loading times Apr 7, 2022
@StefanKock
Copy link
Contributor

When #8688 and #8747 are done, I currently only see a minor change to get rid of a user join LEFT OUTER JOIN users user32_ ON sample0_.reportingUser_id=user32_.id

@MartinWahnschaffe MartinWahnschaffe changed the title Opening the samples and lead to not acceptable loading times Opening the samples leads to not acceptable loading times Apr 11, 2022
@vidi42
Copy link
Contributor

vidi42 commented Apr 11, 2022

Waiting for #8688 and #8747

@markusmann-vg markusmann-vg changed the title Opening the samples leads to not acceptable loading times [PERFORMANCE] Opening the samples leads to not acceptable loading times Apr 28, 2022
@MartinWahnschaffe MartinWahnschaffe changed the title [PERFORMANCE] Opening the samples leads to not acceptable loading times Opening the samples leads to not acceptable loading times Apr 28, 2022
@markusmann-vg markusmann-vg changed the title Opening the samples leads to not acceptable loading times [PERFORMANCE] Opening the samples leads to not acceptable loading times May 2, 2022
@StefanKock
Copy link
Contributor

StefanKock commented May 7, 2022

With previous tickets and small changes on SampleJurisdictionPredicateValidator the joins are now significantly reduced.

...
LEFT OUTER JOIN contact contact5_ ON sample0_.associatedContact_id=contact5_.id
LEFT OUTER JOIN District district6_ ON contact5_.district_id=district6_.id
LEFT OUTER JOIN cases case7_ ON contact5_.caze_id=case7_.id
LEFT OUTER JOIN District district8_ ON case7_.district_id=district8_.id
LEFT OUTER JOIN contact contacts9_ ON case7_.id=contacts9_.caze_id
LEFT OUTER JOIN Person person10_ ON contact5_.person_id=person10_.id
LEFT OUTER JOIN EventParticipant eventparti11_ ON sample0_.associatedEventParticipant_id=eventparti11_.id
LEFT OUTER JOIN EVENTS event12_ ON eventparti11_.event_id=event12_.id
LEFT OUTER JOIN LOCATION location13_ ON event12_.eventLocation_id=location13_.id
LEFT OUTER JOIN District district14_ ON location13_.district_id=district14_.id
LEFT OUTER JOIN EventParticipant eventperso15_ ON event12_.id=eventperso15_.event_id
LEFT OUTER JOIN cases case16_ ON eventperso15_.resultingCase_id=case16_.id
LEFT OUTER JOIN contact contacts17_ ON case16_.id=contacts17_.caze_id
LEFT OUTER JOIN Person person18_ ON eventparti11_.person_id=person18_.id
LEFT OUTER JOIN Facility facility19_ ON sample0_.lab_id=facility19_.id
LEFT OUTER JOIN samples sample20_ ON sample0_.referredTo_id=sample20_.id
...

SampleFacade.getIndexList AFTER.sql.txt

But I found another flaw in the logic: An afterwards fetch for data of last pathogen test results (SampleService.getIndexList ~ line 329) uses an IN clause with sample uuids that is not batched. It might be inperformant and will hit the 32k parameter limit of PostgreSQL. Update on finding: Since the initial query is already batched (usually with 100 entries), the parameter limit is not a concern. I also checked the resulting query with explain analyze and it fetched first the relevant samples by uuid, then joined the corresponding pathogen tests by Sample.id - I see nothing concerning here.

Pathogen test attributes query:

SELECT pathogente0_.testType AS col_0_0_,
       pathogente0_.cqValue AS col_1_0_,
       sample1_.uuid AS col_2_0_
FROM PathogenTest pathogente0_
CROSS JOIN samples sample1_
WHERE pathogente0_.sample_id=sample1_.id
  AND pathogente0_.deleted=FALSE
  AND (sample1_.uuid in ('WU3KU2-2T636F-RARIE4-XLRA2O3U'))
ORDER BY pathogente0_.changeDate DESC

@StefanKock StefanKock added this to the Sprint 115 - 1.72.0 milestone May 11, 2022
BarnaBartha added a commit that referenced this issue May 11, 2022
…ple_jurisdiction_id_joins

#8610 avoid sample jurisdiction id joins
@AndyBakcsy-she AndyBakcsy-she self-assigned this May 16, 2022
@AndyBakcsy-she AndyBakcsy-she added the qa-verified Issue has been tested and verified by QA label May 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend Affects the web backend change A change of an existing feature (ticket type) laboratory Samples, pathogen tests and lab messages performance Issues that are meant to increase the performance of the application qa-verified Issue has been tested and verified by QA
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants