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

Slow SOAP queries with INCLUDEs #117

Open
kevinphippsstfc opened this issue Mar 11, 2021 · 2 comments
Open

Slow SOAP queries with INCLUDEs #117

kevinphippsstfc opened this issue Mar 11, 2021 · 2 comments
Labels
performance Issues related to poor performance of the program

Comments

@kevinphippsstfc
Copy link
Contributor

Whilst investigating the long running queries issue (#115) I found that there are two queries in the resolveDatasetIds method of DataSelection using the SOAP API, both of which request a Dataset including the Investigation and Facility, that are very slow. These are on the following lines:

https://github.com/icatproject/ids.server/blob/v1.11.0/src/main/java/org/icatproject/ids/DataSelection.java#L110
https://github.com/icatproject/ids.server/blob/v1.11.0/src/main/java/org/icatproject/ids/DataSelection.java#L130

and when run against the Diamond preprod ICAT they take 2 seconds to complete. This is very repeatable - not randomly slow every so often.

I found that when I request only the required fields using the ICAT REST client using the query:

SELECT ds.id, ds.name, ds.location, inv.id, inv.name, inv.visitId, fac.id, fac.name FROM Dataset ds JOIN ds.investigation inv JOIN inv.facility fac WHERE ds.id=?

the query only takes about 80ms.

Whilst 2 seconds per query might not sound a lot, as with #115 there is a timeout of 30 minutes to generate a preparedID, so without allowing time for anything else, this already limits the number of Datasets that can be requested to 900 (30x30). On the Diamond ICAT we do have users requesting hundreds of Datasets (partly because we don't allow them to select whole Investigations) so this has the potential to be a problem.

At the same time as converting these two queries to use the REST client, that would only leave 2 remaining uses of the SOAP client in this class (on lines 118 and 125) so it would be worth converting these at the same time. That would mean that the horrible double section of exception handling on line 152 could be reduced to just one section for the REST client.

A batch of import statements for the SOAP client classes could also be removed.

Once this has been done, there would be no need to pass the SOAP client into the constructor for DataSelection, thus making that a bit simpler and tidier as well.

@RKrahl
Copy link
Member

RKrahl commented Mar 15, 2021

Many thanks for pointing this out! What you describe here is a small portion of what I called "awkward code" in #109.

@RKrahl RKrahl added the performance Issues related to poor performance of the program label Mar 15, 2021
@RKrahl RKrahl added this to the 1.12.0 milestone Apr 1, 2021
@RKrahl
Copy link
Member

RKrahl commented Apr 9, 2021

As discussed with @kevinphippsstfc, we will postpone that one in favour of proceeding with the release of 1.12.

@RKrahl RKrahl removed this from the 1.12.0 milestone Apr 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance Issues related to poor performance of the program
Projects
None yet
Development

No branches or pull requests

2 participants