WebAPI v2.11 fails to start with many sources configured #2031
Comments
|
@anthonysena , i'm not sure it's the number of CDMs that is the problem but rather some value is longer than 100 chars (although not sure what that could have been). I'm basing it on this error: I wish it would output what the value was that was longer than 100 chars, that would have helped. |
|
Linking this to #2007 as they are rooted in the same problem. |
|
@anthonysena I would address other questions to Sergei Suvorov @ssuvorov-fls - he is the author of the original code changes in #1993 (I only created the PR). |
|
Thanks @anton-abushkevich for the update here and I'll tag @ssuvorov-fls on the PR as I have a few questions that are likely better for that thread. |
|
Hello @anthonysena Can we disable the Achilles caching when we do not require it? My presumption was that setting <cdm.cache.cron.warming.enable>false</cdm.cache.cron.warming.enable> would do that while also allowing for the previous "results" cache controlled by <cdm.result.cache.warming.enable>true</cdm.result.cache.warming.enable> to work as before?
Why is warmCaches() invoked in 2 places? Place 1 and Place 2
Can you clarify what buckets represents in this loop? It appears like you are trying to bucket the caching operations into a single job? I'm not sure if I'm interpreting this correctly.
It appears we lost the ability to control the cached based on daimon priority. Now it appears that any source with a vocabulary and results daimon will automatically qualify to have the Achilles results cached? If I have this correct, we'll need to restore this functionality.
|
|
Thanks @ssuvorov-fls - I also posted some questions on #2032 (comment) so I'm hoping we can keep the conversation going on that pull request. A couple of follow up points to your notes here:
If
Here is the code: WebAPI/src/main/java/org/ohdsi/webapi/service/CDMResultsService.java Lines 437 to 445 in 0985211 The original functionality skipped warming the record count cache if a source had a results daimon with priority == 0. The change in the code above now prioritizes the daimon to have its cache as one of the first steps in the List<> instead. The changes I started to make on #2032 aims to address this. If you could review, I'd appreciate it. |
* fails to start with many sources configured #2031 * Starting to add back the ability to control caching based on daimon priority Co-authored-by: Anthony Sena <[email protected]>
* fails to start with many sources configured #2031 * Starting to add back the ability to control caching based on daimon priority Co-authored-by: Anthony Sena <[email protected]> (cherry picked from commit fad3dab)
Expected behavior
WebAPI starts without error after installing v2.11
Actual behavior
WebAPI fails to start after installing v2.11
Steps to reproduce behavior
Installing this in our environment had the following error when starting up WebAPI:
A bit more about our environment: we have a large number (> 10) of CDMs configured with
vocabularyandresultsdaimons. When reviewing the stack trace above, my first impression was that perhaps something about thesource_keywas problematic and causing a job name > 100 characters. This was not the case. I was then looking through the changes here: https://github.com/OHDSI/WebAPI/pull/1993/files#diff-45561d1ce808215d1229fd2a7d080664d05d70b47e13bfa830d9f400db258194L109-R147 and see that we're callingwarmCaches()in multiple places in this service.To try and debug, I set up my local WebAPI DB with 2 sources to see if I could reproduce the problem. When I did this, I found that WebAPI is starting multiple warming jobs, even when I manually set
<cdm.cache.cron.warming.enable>false</cdm.cache.cron.warming.enable>in the pom.xml. See below:Upon re-review of #1993, I'm unclear on a few items:
<cdm.cache.cron.warming.enable>false</cdm.cache.cron.warming.enable>would do that while also allowing for the previous "results" cache controlled by<cdm.result.cache.warming.enable>true</cdm.result.cache.warming.enable>to work as before?warmCaches()invoked in 2 places? Place 1 and Place 2WebAPI/src/main/java/org/ohdsi/webapi/service/CDMResultsService.java
Lines 426 to 458 in 0985211
sourceIds). The loop that starts on 429 adds each sourceId to a collection and then this collection is used on line 448 to callcreateJobbut this collection is not purged likesourceKeysandjobStepswhich I believe is where we're creating the duplication and potentially violating the length of the job name.bucketsrepresents in this loop? It appears like you are trying to bucket the caching operations into a single job? I'm not sure if I'm interpreting this correctly.I'm sorry I missed so much in the review of #1993 but we also need to better communicate desired functionality ahead of implementation moving forward. The brief description in #1978 is insufficient to support building new features without describing how they will work, how we plan to preserve backwards compatibility and so on.
The text was updated successfully, but these errors were encountered: