Core Apps: Very long TTFB for patientDashboard.page when many encounters

I see that @ssmusoke pointed you to something, but more typically I would expect these to be tested in a junit test against an in-memory database (extending BaseModuleContextSensitiveTest).

Yes, as long as the new implementation doesn’t run noticeably slower for visits with ~5 encounters. I think you also need to replace the implementation of the getUniqueDiagnoses method.

Judging from the most recent commit on that line of code, sounds exactly right: RA-624 - On the visit dashboard, the visit summary links on the left … · openmrs/openmrs-module-coreapps@d4ef60d · GitHub

At a glance, probably this one: https://github.com/openmrs/openmrs-module-emrapi/blob/master/api/src/main/java/org/openmrs/module/emrapi/visit/EmrVisitService.java

Thanks for all this @darius. I’m almost done with a renewed implementation of getPrimaryDiagnoses() (for a start). Currently working on the unit test that you mentioned.

What did you mean, that @mseaton realised that getPrimaryDiagnoses() was not returning a list of unique diagnoses? I’m pinging him here so that he can confirm. Mike the code change is here.

While reworking it, I was making getPrimaryDiagnoses() return a unique list of diagnoses, is that the wanted behaviour then?

Nevermind, I had skipped the actual commit headline:

RA-624 - On the visit dashboard, the visit summary links on the left should only show distinct diagnosis concepts

I created EA-114. @dkayiwa, I would like to claim that one, I’m almost ready with a PR for the master branch.

If all goes well, we will then also need to backport the fix to a 1.12.x branch… sigh

@mksd i have made it ready for work. Go ahead and claim. :slight_smile:

@dkayiwa, @darius,

Trying my fix in the webapp produced a NPE on EmrVisitService inside VisitDomainWrapper. I then realised that in fact that was also the case for the already existing and @Autowired DispositionService. I guess not many people must be using that one… I spent some time failing with EmrVisitService while copying its configuration from DispositionService before realising that the latter was never working in the first place. I also tried to annotate VisitDomainWrapper with @Component, to no avail.

Anyway, could anyone of you figure out why DispositionService is not injected by Spring? So that I can to the same with EmrVisitService basically…

Note that everything is injected fine within unit tests.

I guess I need to investigate what happens here:

public VisitDomainWrapper newVisitDomainWrapper() {
  VisitDomainWrapper visitDomainWrapper = new VisitDomainWrapper();
  return (VisitDomainWrapper) autowire(visitDomainWrapper);
}

In particular why it autowires fine within unit tests and not within the webapp at runtime.

But please shed some light in the meantime if you have a chance. This is quite time consuming to troubleshoot and I may not have time to look at it before anyone of you throws hints (or before Monday…)

Cc @dkayiwa, @darius, @lluismf, @wyclif

I can’t help you with Spring but I can take a look at the HQL you mention in EA-114. Have you already committed changes to your repo to take a look at it ?

At first glance it seems easy filtering diagnoses but I just noticed that diagnosis is not a table, and it’s not trivial to get them in a single query.

@dkayiwa hinted that the problem may be coming from the fact that VisitDomainWrappers are not instantiated using Spring in the patient dashboard (it again!)

Looking at visits.gsp it appears that it is down to PatientDomainWrapper.getAllVisitsUsingWrappers(): This line

VisitDomainWrapper visitWrapper = new VisitDomainWrapper(visit);

should become

VisitDomainWrapper visitWrapper = domainWrapperFactory.newVisitDomainWrapper(visit);

… in order to have all the Spring autowiring happening.

Trying out, will report soon…

Hi @lluismf,

Here is the first PR for EA-114:

This is where I currently workaround the still missing part in VisitDomainWrapper. getUniqueDiagnoses(..):

if (primaryOnly == true && confirmedOnly == true) // TODO: Re-implement that specific case with HQL
  return getUniqueDiagnosesLegacy(primaryOnly, confirmedOnly);

HQL queries are written in separate resource files and therefore the file for that 4th query is still missing, it should come here alongside the others.

Otherwise, concerning Core Apps and the very long latency on the patient dashboard, it is now gone :sweat:

@ssmusoke’s hints were really helpful when I was trying out my draft HQL queries on real data. However I am using Eclipse and not IntelliJ IDEA, nevertheless the setup is about exactly the same. I will provide details here for others since one step turned out to be tricky:

  1. Use the latest release of Eclipse, Neon.1 at the time of this writing. Version: Eclipse 4.6.1.

  2. Install the Hibernate Tools (from JBoss Tools) from the Eclipse Marketplace: Versions: JBoss Tools 4.4.1, Hibernate Tools 5.1.1.

  3. Open the Database Development perspective and create a new MySQL database connection to the ‘openmrs’ database that you want to work with. Actually the MySQL JDBC driver was not shipped with Eclipse, and strangely there is no easy way to get it from within. You must download the JAR and add a new driver definition by yourself:

    You will have to select it, ‘Edit’ it and point to your downloaded JAR.

  4. Open the Hibernate perspective and create a a new Hibernate Configuration, as an example:

    Everything should be based on the Hibernate configuration of the Core/Platform, as @ssmusoke hinted in his post. The trick is to use the actual database connection created in the previous step. At this point you can open the HQL editor and play around with queries.

There is a catch though. The HQL might not be able to expand the Database and may produce an error:

This is the detailed error:

org.hibernate.eclipse.console
Error
Mon Nov 21 18:33:30 CET 2016
org.jboss.tools.hibernate.runtime.spi.HibernateException: java.lang.NoSuchMethodError: org.slf4j.spi.LocationAwareLogger.log(Lorg/slf4j/Marker;Ljava/lang/String;ILjava/lang/String;[Ljava/lang/Object;Ljava/lang/Throwable;)V
```

After searching around I found hints on Stackoverflow that the issue came from a version mismatch and JAR collide between the SL4J artifact that Hibernate Tools intends to use (1.5.x) and the one the Core/Platform is providing (1.6.0).
The "trick" is to rebuild the Core/Platform with a 1.5.x version of SL4J:
```xml
<dependency>
  <groupId>org.slf4j</groupId>
  <artifactId>slf4j-api</artifactId>
  <version>1.5.6</version>
</dependency>
<dependency>
  <groupId>org.slf4j</groupId>
  <artifactId>jcl-over-slf4j</artifactId>
  <version>1.5.6</version>
</dependency>
```

And finally to make this work Eclipse must be restarted and the Core/Platform Maven updated and rebuilt in Eclipse.
Hopefully this will help others!

Hi Dimitri,

IMHO you don’t need 4 HQLs for all combinations of primaryOnly, confirmedOnly. I’d use a base HQL and dinamically add filters.

Base HQL: from Obs o where o.voided = 'false' and (o.encounter in (select e from Encounter e where e.visit = (select v from Visit as v where v.visitId = :visitId))

if !confirmed && !primary 
   hql += and o.concept.conceptId = :diagnosisOrderConceptId

if confirmed && !primary
   hql += and o.concept.conceptId = :diagnosisCertaintyConceptId  
          and o.valueCoded.conceptId = :confirmedCertaintyConceptId

if !confirmed && primary
  hql += and o.concept.conceptId = :diagnosisOrderConceptId
         and o.valueCoded.conceptId = :primaryOrderConceptId

if confirmed && primary
  hql += and (o.concept.conceptId = :diagnosisOrderConceptId        
              and o.valueCoded.conceptId = :primaryOrderConceptId       
              or 
              o.concept.conceptId = :diagnosisCertaintyConceptId       
              and  o.valueCoded.conceptId = :confirmedCertaintyConceptId )

Add group by (not sure if really needed) and order

You may also use a Criteria instead of HQL.

Hi @lluismf,

Sure, things can be refactored that way but at first it was easier to have the HQL queries kept separate, to play around with the Hibernate Tools in Eclipse.

Note that I had anticipated case #4 to be more complicated, this query works:

select
  o.obsGroup
from
  Obs o
where
  o.voided = 'false'
  and (o.encounter in (select e from Encounter e where e.visit = (select v from Visit as v where v.visitId = :visitId)))
  and o.concept.conceptId = :diagnosisOrderConceptId
  and o.valueCoded.conceptId = :primaryOrderConceptId
  and o.obsGroup in (
    select
      o.obsGroup
    from
      Obs o
    where
      o.voided = 'false'
      and (o.encounter in (select e from Encounter e where e.visit = (select v from Visit as v where v.visitId = :visitId)))
      and o.concept.conceptId = :diagnosisCertaintyConceptId
      and o.valueCoded.conceptId = :confirmedCertaintyConceptId
  )
group by o.encounter, o.obsGroup
order by o.encounter.encounterDatetime desc, o.obsGroup.obsDatetime desc

An inner join was in fact needed between two set of obs groups: 1) the set with a confirmed certainty inside each of its obs groups, and 2) the set with a primary order inside each of its obs groups. That returns the set of obs groups that have both the desired obs inside: confirmed certainty and primary order.

Have you tried this instead ?

and (o.encounter.visit = :visitId)

1 Like