referenceapplication 2.5 is not compatible with iSantePlus module we developped with referenceapplication 2.3

guerschon · February 16, 2017, 3:42pm

I have discovered that 2.5 version of the reference application is not compatible with the iSantePlus module for the following reason: · The 2.5 version of the reference application is compatible with the 3.3.0 htmlformentry module and the htmlformentryui-1.6.1, while the iSantePlus module uses the htmlformentry-2.6 module and the htmlformentryui-1.4. (please look at the attached files)

I tried to downgrade the htmlformentry and the htmlformentryui to the versions that support the iSantePlus module, but when I do so I ran into a compatibility issue with the 2.5 reference application.

I then tried to make the isantePlus module to work with the recommended versions of htmlformentry and htmlformentryui. I discovered that the isantePlus module uses a set of methods that causes issues with the recommended versions.(see screenshot 1 pulled from line 24 of isantePlusHtmlForm.java)

@craigappl @k.joseph

craigappl · February 16, 2017, 5:00pm

Hi Everyone,

Here’s the full stack trace in a pastebin

Craig

mseaton · February 16, 2017, 5:15pm

@craigappl / @guerschon - at the bottom of that stack trace you can see the underlying cause:

When making a call to save your form, you get this validation error:

Caused by: org.openmrs.api.ValidationException: '' failed to validate with reason: name: This value exceeds the maximum length of {0} permitted for this field.

Are you setting a name for your form?

Mike

k.joseph · February 16, 2017, 6:26pm

check and make sure all the form names you have for all the forms at: https://github.com/IsantePlus/openmrs-module-isanteplus/tree/master/omod/src/main/webapp/resources/htmlforms don’t exceed 50 characters.

see;

github.com

openmrs/openmrs-core/blob/master/api/src/main/java/org/openmrs/api/db/hibernate/HibernateAdministrationDAO.java#L184


		// (solution for junit tests that usually use hsql
		// hsql does not like the backtick.  Replace the backtick with the hsql
		// escape character: the double quote (or nothing).
		if (HibernateUtil.isHSQLDialect(sessionFactory)) {
			sql = sql.replace("`", "");
		}
		return DatabaseUtil.executeSQL(sessionFactory.getCurrentSession(), sql, selectOnly);
	}
	
	@Override
	public int getMaximumPropertyLength(Class<? extends OpenmrsObject> aClass, String fieldName) {
		if (configuration == null) {
			HibernateSessionFactoryBean sessionFactoryBean = (HibernateSessionFactoryBean) applicationContext
			        .getBean("&sessionFactory");
			configuration = sessionFactoryBean.getConfiguration();
		}
		
		PersistentClass persistentClass = configuration.getClassMapping(aClass.getName().split("_")[0]);
		if (persistentClass == null) {
			throw new APIException("Couldn't find a class in the hibernate configuration named: " + aClass.getName());
		} else {

and

github.com

openmrs/openmrs-core/blob/master/api/src/main/resources/org/openmrs/api/db/hibernate/Form.hbm.xml#L30


		<id name="formId" type="int" column="form_id" unsaved-value="0">
			<generator class="native">
				<param name="sequence">form_form_id_seq</param>
			</generator>
		</id>


		<property name="uuid" type="java.lang.String"
			column="uuid" length="38" unique="true" />


		<property name="name" type="java.lang.String" 
			column="name" not-null="true" length="255" />


		<property name="version" type="java.lang.String" 
			column="version" not-null="true" length="50" />
		
		<property name="build" type="java.lang.Integer" column="build" 
			not-null="false" />
		
		<property name="published" type="boolean" column="published"
			length="1" not-null="false" />

@darius, @burke, @dkayiwa, @wyclif, @raff: while checking through this, i noticed the data model form#name supports upto 255 whereas the form.hbm.xml or mapping allows upto 50 characters, must not those match even for the rest of objects and columns!!

guerschon · February 21, 2017, 3:23pm

I have modified the isantePlus module to be compatible with version 2.5. I essentially modified the isantePlusServiceImpl.java files in which I replaced the old method getObservation with the new method and IsantePlusFormsFragmentController.java in which I shortened the names of the forms. I also changed the names of the xml files for the forms. As of now I can access the patient dashboard and display forms. The problem that we have now is that it takes a long time to access the dashboard and open forms. I wonder if this is caused by the new method getObservationsByPersonAndConcept (…).I have created a new branch on Github with the iSantePlus module code changes. https://github.com/IsantePlus/openmrs-module-isanteplus/tree/UpdatedIsantePlusTo2.5

mseaton · February 21, 2017, 5:13pm

@k.joseph, not sure why but I never saw your last post. Yes, this sounds like a bug that we should ticket and fix.

@guerschon, to clarify - are you saying that the same general patient dashboard code that was running on refapp 2.3 now runs much slower on refapp 2.5? Or has it always been slow and you want to figure out how to speed it up generally?

Thanks, Mike

guerschon · February 21, 2017, 5:49pm

The patient dashboard is slow when i install the isantePlus module

craigappl · February 21, 2017, 8:26pm

@guerschon

I’m working through the issues here are a few things:

Right now, it takes 4 seconds to load the clinician facing dashboard on my virtual machine after I click the patient’s name in the patient search.

The “Context.getObsService().getObservationsByPersonAndConcept(patient, viralLoadConcept));” requires that we pass in a person instead of a patient.Can you add “patient.getPerson()” every time we call getObservationsByPersonAndConcept across the module?

The end results should look like this:

Context.getObsService().getObservationsByPersonAndConcept(patient.getPerson(), viralLoadConcept));

The form file names were changed in the latest commit, but they aren’t properly referenced in the fragment, causing links that aren’t showing up in the forms history fragment. These are loaded in by the module activator based on the iSantePlusConstants value. Can you update the names in this line?
When I click on an HTMLform to enter it, it takes 119 seconds and MySQL jumps to taking 97+% of the system resources. This also happens when I open the HTML form in the old UI. I ran “show processlist\G” in mySQL and here’s the query that seems to be taking the longest:

*************************** 4. row *************************** Id: 137 User: openmrs_user Host: localhost:33157 db: openmrs Command: Query Time: 0 State: Copying to tmp table Info: select this_.concept_id as y0_ from concept_reference_map this_ inner join concept concept3_ on this

I’m testing against MySQL 5.6. @mseaton, @dkayiwa, @k.joseph Did anything change in how HTML forms are rendered? Do you know how I could triage what’s taking so long in loading HTML forms? Is there anything in this particular form that stands out at you?

Craig

mseaton · February 21, 2017, 9:32pm

@craigappl - just based on what you write above, and the query that you said is taking the longest, and a quick search of the form, it seems almost certain that the culprit is the concept mapping lookups on the form.

Doing a search in my browser of “CIEL” on the form you linked to, I get 1,194 hits! This means that you are looking up nearly 1200 concepts by mapping on this form. That’s a lot

Chances are, there is a lot of room for improvement in htmlformentry in terms of looking up concepts. This could involve caching of mappings to concept ids to improve future loading times (i.e. the first load is slow, but subsequent loads are faster), to building in-memory lookup tables on module startup to make such loading nearly instantaneous. This seems like a useful enhancement to add.

Is this something your team could help contribute back into htmlformentry? Looks like a straightforward improvement to HtmlFormEntryUtil.getConcept(string). Can you create a ticket for this in the htmlformentry module and mention me and @mogoodrich in the ticket so that it is on our radar?

Let me know if this makes sense…

Mike

darius · February 21, 2017, 10:18pm

Ideally we would solve the issue of speeding up lookup of concepts by mapping in openmrs-core, and not just within HFE.

In this talk post @raff mentions a general approach for this. However it requires the (not yet release) Platform 2.1, which supports both indexing concepts via Lucene, and support for fetching cached items.

As it happens, it’s on my Bahmni to-do list to release platform 2.1 in the next <2 months. So if @craigappl’s timeline and tolerance for running a just-release platform permit it, maybe we can just ensure we have the right Lucene index for find-concept-by-mapping in openmrs-core. Thoughts?

craigappl · February 21, 2017, 10:28pm

Thanks @mseaton

Here’s our process:

We load in the latest CIEL dictionary as part of our deployment process. mysql -u OPENMRS_USER -pOPENMRS_USER_PASS openmrs < openmrs_concepts_1.11.2_20161009.sql
Our forms are loaded when the module is loaded using the htmlformentry module’s HtmlFormEntryService.class in our module activator.

Notes:

Our HTMLForms use conceptId=“CIEL:1271” which makes it a lookup map
The form loading time on OpenMRS v2.3 (Tomcat7, Java 7, MySQL 5.5, HTMLFormEntry v2.6 and HTMLFormEntryExt1.9 v1.6.1) only takes 3.5 seconds.

Questions:

We chose to use maps in the conceptId assuming it was the best practice. Are others doing differently?
Given the process, above, should we be doing this a different way?

mseaton · February 21, 2017, 10:33pm

@craigappl, as @darius mentions, this is likely an issue from the core platform version - between refapp 2.3 and 2.5, core was upgraded from 1.11.4 to 2.0.1. So I’d look at what changed there first, in terms of looking up concepts by mapping.

@darius, I agree with fixing it there, but for all of the implementations that are not going to be on platform 2.1 in the near future, making a (relatively minor) improvement to htmlformentry still probably makes sense. No reason not to do both…

Mike

mogoodrich · February 22, 2017, 4:10am

Just read this over quickly and haven’t looked at the code yet, but are we saying that form loading time jumped from around 3.5 second to 120 seconds after an upgrade from 1.11.4 to 2.0.1? Regardless of long-term fixes, wouldn’t that suggest that a gross inefficiency in getConceptByMapping was introduced at some point and should be fixed (and backported)? (Or are we saying that assumedly the slowdown was caused by the conceptual change to using Lucerne, which we won’t want to revert, but will only be able improve going forward in 2.1?)

Take care, Mark

darius · February 22, 2017, 4:22am

I agree that if the same code runs 40x slower when going from platform 1.11 to 2.0 then we should treat that as a bug and address it.

So we have steps to reproduce this? (Sorry I’m also just skimming this thread.)

-Darius (by phone)

mogoodrich · February 22, 2017, 2:46pm

@craigappl can you confirm that what I stated above is accurate?

mogoodrich · February 22, 2017, 2:56pm

this seems like a likely culprit, I suspect it negates any benefits that indexing may be having:

mogoodrich · February 22, 2017, 3:09pm

Oh, and, yes, to answer your earlier question @craigappl, using a source/mapping pair in the concept ID (over straight concept ID or uuid) would be considered best practice. (Another reason we should fix the bug that is making this so slow).

Mark

craigappl · February 22, 2017, 5:37pm

Hi,

I’m going to setup two identical vms on my machine today with platform 1.11.x and 2.0.4. What’s the best way to replicate this environment so others can evaluate it? Are there any dev/test instances available online with OpenMRS 1.11.x?

Thanks, Craig

mogoodrich · February 22, 2017, 5:51pm

I’ve created a new ticket for what I think is the issue:

@craigappl do you have any dev capacity available? If so, if you could build a custom openmrs.war with the change made in TRUNK-4452 reverted and see if it fixes the performance problem you were seeing. If it does, you’ll have a pretty strong case for flagging TRUNK-5071 as a priority ticket to fix and backport.

Mark

craigappl · February 22, 2017, 5:55pm

Thanks Mark!

Yes, I’ll build the war and test it out. I’ll likely have to get to this tomorrow. I’ll make sure to document the steps in the ticket.

Thanks, Craig