Performance improvement of Requisitions for Approval/Convert searches

20 views
Skip to first unread message

jkon...@soldevelo.com

unread,
Jun 30, 2017, 10:46:11 AM6/30/17
to OpenLMIS Dev
Hi all,

While measuring our recent performance improvements, I discovered two additional endpoints which work very slowly:
/api/requisitionsForApproval
/api/requisitionsForConvert
They use a full requisition dto and getting each of these takes about 40 seconds for EM program in Malawi. Opening "Approve" page with 17 requisitions took over 12 minutes on our dev server. We can easily improve this by returning BasicRequisitionDto like we did in requisition search, but we are missing some fields there: date submitted (which is currently taken from the list of requisition statuses) and geographic zone of the facility.

It seems to me that we have two options here:
- Add a list of requisition statuses and full facility representation to the BasicRequisitionDto
- Create dateSubmitted and geographicZone fields in that dto and populate them in dto builder

Does making that change sound reasonable? Which do you think would be better solution?

Best Regards,
Jakub

Paweł Gesek

unread,
Jun 30, 2017, 11:14:05 AM6/30/17
to Jakub Kondrat, OpenLMIS Dev
Extending the dto sound more reasonable. Is date submitted part of the requisition or are we getting it from the status list? Holding date submitted redundantly in the requisition object might be a good way to improve performance, since we would avoid joining with the status change table. We are already doing it with created date.

I also don't think we would want the full facility representation in the basic dto - I think we should keep it fairly minimal.


SolDevelo
Sp. z o.o. [LLC] / www.soldevelo.com
Al. Zwycięstwa 96/98, 81-451, Gdynia, Poland
Phone: +48 58 782 45 40 / Fax: +48 58 782 45 41

--
You received this message because you are subscribed to the Google Groups "OpenLMIS Dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to openlmis-dev+unsubscribe@googlegroups.com.
To post to this group, send email to openlm...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/openlmis-dev/bfb5f5d2-3c68-4580-81c6-eaccfe8e1989%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.



--

Paweł Gesek
Technical Project Manager
pge...@soldevelo.com / +48 690 020 875



SolDevelo
Sp. z o.o. [LLC] / www.soldevelo.com
Al. Zwycięstwa 96/98, 81-451, Gdynia, Poland
Phone: +48 58 782 45 40 / Fax: +48 58 782 45 41

jkondrat....@gmail.com

unread,
Jul 3, 2017, 6:09:54 AM7/3/17
to OpenLMIS Dev, jkon...@soldevelo.com
Yes, we are getting date submitted from status list, but it turned out that we already have that list in BasicRequisitionDto. In that case, adding geogprahic zone to BasicFacilityDto would suffice, and it seems that we need the whole hierarchy as UI is looking up for the one with correct level number.


On Friday, 30 June 2017 17:14:05 UTC+2, Paweł Gesek wrote:
Extending the dto sound more reasonable. Is date submitted part of the requisition or are we getting it from the status list? Holding date submitted redundantly in the requisition object might be a good way to improve performance, since we would avoid joining with the status change table. We are already doing it with created date.

I also don't think we would want the full facility representation in the basic dto - I think we should keep it fairly minimal.
On Fri, Jun 30, 2017 at 4:46 PM, <jkon...@soldevelo.com> wrote:
Hi all,

While measuring our recent performance improvements, I discovered two additional endpoints which work very slowly:
/api/requisitionsForApproval
/api/requisitionsForConvert
They use a full requisition dto and getting each of these takes about 40 seconds for EM program in Malawi. Opening "Approve" page with 17 requisitions took over 12 minutes on our dev server. We can easily improve this by returning BasicRequisitionDto like we did in requisition search, but we are missing some fields there: date submitted (which is currently taken from the list of requisition statuses) and geographic zone of the facility.

It seems to me that we have two options here:
- Add a list of requisition statuses and full facility representation to the BasicRequisitionDto
- Create dateSubmitted and geographicZone fields in that dto and populate them in dto builder

Does making that change sound reasonable? Which do you think would be better solution?

Best Regards,
Jakub


SolDevelo
Sp. z o.o. [LLC] / www.soldevelo.com
Al. Zwycięstwa 96/98, 81-451, Gdynia, Poland
Phone: +48 58 782 45 40 / Fax: +48 58 782 45 41

--
You received this message because you are subscribed to the Google Groups "OpenLMIS Dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to openlmis-dev...@googlegroups.com.

To post to this group, send email to openlm...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/openlmis-dev/bfb5f5d2-3c68-4580-81c6-eaccfe8e1989%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.



--

Paweł Gesek
Technical Project Manager
pge...@soldevelo.com / +48 690 020 875

Brandon Bowersox-Johnson

unread,
Jul 3, 2017, 2:35:06 PM7/3/17
to OpenLMIS Dev, jkondrat....@gmail.com

Hi Jakub,

 

It looks like a pull request was merged today for this change:

https://github.com/OpenLMIS/openlmis-requisition/pull/36

 

Please let us know what the final outcome was for the design/decision that was discussed on this thread.

 

-Brandon

jkon...@soldevelo.com

unread,
Jul 4, 2017, 8:26:11 AM7/4/17
to OpenLMIS Dev, jkondrat....@gmail.com
Hi Brandon, 

Finally, these endpoints were changed to return BasicRequisitionDto, which only contains basic information about a requisition - no line items or non full-supply products. Additionally, we had to add one field to BasicFacilityDto - geographic zone, which is used in the list of requisitions for approval. It also turned out that there was no RAML for /requisitionsForConvert so I had to add one for the existing java class (RequisitionWithSupplyingDepotsDto) that was returned there.

Best,
Jakub

To post to this group, send email to openl...@googlegroups.com.

Reply all
Reply to author
Forward
0 new messages