LOGIC-93 <https://tickets.openmrs.org/browse/LOGIC-93> is about introducing a CacheableRule interface so that rule implementations opt-in to caching.
-Darius On Friday, October 14, 2011, Darius Jazayeri wrote: > The reason I introduced the cache is that the core implementation of > RuleDefinition fetches rules from the database. It's not about the memory > footprint, it's that not all rule instantiation is cheap. > > Peeking at the code, I see that we don't have a marker interface or method > to indicate which rules are cacheable. I guess we should add a CacheableRule > interface in the logic module, and have the code only cache these. > > It would really help us going forwards if there were a good unit test suite > around heavy use of logic rules. I don't have the capacity to build this > (neither time nor sufficient knowledge) but I'm happy to help coordinate. > > -Darius > > On Thursday, October 13, 2011, Anand, Vibha wrote: > > Burke: One way to implement rule cache, which I do not find useful, and > still provide a thread safe interface is by keeping reference count on the > rule token / handle. From where I come, it’s important that OpenMRS as a > framework design around multithreaded / multitasking systems seriously. In > the same vein, I strongly agree and support that it is perfectly legal to > have instance variables in a rule class (translated or otherwise) as a > design decision. I do not see much usefulness for rule cache because I > think correct rule cache management is more expensive then the little > impact it might have on the memory footprint. I hope you will consider these > design issues in fixing this and future versions.**** > > *Vibha* > > * * > > ** ** > > *From:* [email protected] [mailto:[email protected]] *On Behalf Of *Burke > Mamlin > *Sent:* Thursday, October 13, 2011 12:50 PM > *To:* [email protected] > *Subject:* Re: [OPENMRS-DEV] Major Logic Thread Safety Issue**** > > ** ** > > I'm not sure it's fair to expect rule authors to avoid any instance > variables. We either need to make new instances or – if that's a memory hog > – we could avoid needing to create a new instance every time by introducing > String > getState() and void setState(String) methods to the Rule interface.**** > > ** ** > > -Burke**** > > On Thu, Oct 13, 2011 at 6:10 PM, Ben Wolfe <[email protected]> wrote:**** > > I don't know of the implementation or reasoning here, but there is a lot > less memory used if its not instantiating new objects each time. I think we > should stick with that approach. We should either say that class level > variables are not allowed (like on controllers and servlets) or we say they > are allowed but all variables get cleared before a rule is used. > > Tammy, what was the fix in 1.5 for this? Is there a ticket? Was there a > regression test for it originally? > > Ben**** > > ** ** > > On Thu, Oct 13, 2011 at 5:46 PM, Tammy Dugan <[email protected]> wrote:*** > * > > It also corrupted 3 days worth of patient data across 4 clinics so needless > to say we were pretty unhappy that a problem we fixed in 1.5 came back in > 1.7 and corrupted patient data. > > Tammy**** > > > > On 10/13/2011 11:27 AM, McKee, Steven Jay wrote: **** > > Darius,**** > > **** > > CHICA encountered a major issue with Logic yesterday. We had information > get crossed between multiple patients, and we tracked it down to a thread > safety issue in Logic. Currently the Token Service’s “getRule(String)” > method (in OpenMRS 1.7) retrieves a rule from a rule provider and then > caches it. The next time the rule is asked for, it gets pulled directly > from the cache. Our Java rules translated by the Arden translator had class > level variables that were getting changed by concurrent threads, and we were > seeing incorrect data for patients.**** > > **** > > In OpenMRS 1.5 the logic service would cache the rule, but it would return > a new instantiation of the rule class so it was thread safe. Now in 1.7 + > the thread safety no longer exists because the same instance of the rule is > directly returned from the cache. **** > > **** > > This is a very high priority issue that needs to be resolved immediately.* > *** > > **** > > Thanks,**** > > **** > > Steve McKee**** > > Children’s Health Services Research**** > > Indiana University School of Medicine**** > > Phone: 317-278-9660**** > > Email: [email protected]**** > > **** > ------------------------------ > > Click here to unsubscribe from OpenMRS Developers' mailing list **** > > _________________________________________ To unsubscribe from OpenMRS Developers' mailing list, send an e-mail to [email protected] with "SIGNOFF openmrs-devel-l" in the body (not the subject) of your e-mail. [mailto:[email protected]?body=SIGNOFF%20openmrs-devel-l]
