[BI-2579] Refresh cache after import completes#448
Conversation
mlm483
left a comment
There was a problem hiding this comment.
This change makes sense to me, I suggested a change to preserver current behavior.
I'll test this along with the BrAPI server changes as I review the BrAPI server changes.
src/main/java/org/breedinginsight/brapi/v2/dao/BrAPIGermplasmDAO.java
Outdated
Show resolved
Hide resolved
mlm483
left a comment
There was a problem hiding this comment.
Whenever we make changes to application.properties.template in the BrAPI server, we make corresponding changes to src/main/resources/brapi/properties/application.properties in bi-api.
I think it makes sense to do that in this PR. If you'd like me to make that change and push to this branch, let me know.
I will add the properties that were added for each ticket. The BI-2578 ones I'll add to that bi-api MR, the 2579 ones I'll add here. |
I think I'll add the newly added params for both MRs separately if that makes sense. |
nickpalladino
left a comment
There was a problem hiding this comment.
I thought maybe there was some case where data was expected to be in the cache but wasn't which is why it was done the previous way but testing seems to be working with the changes.
Description
Story: BI-2579
To optimize the importing experience for germs, I've changed the code so that the cache only refreshes one time during the import process, at the end.
This idea of posting and refresing the cache from the response of a batch chunk being imported confuses me. Why would we not just refresh at the end of the process, or, since the cache refreshes everytime you view germplasm, is it done at all?
Maybe someone here can tell me a little bit more, but, when disabling this refreshing process, I found the performance of the import drastically improves. This is another problem that would also not exist if the cache were removed entirely and replaced with paginated requests for the data that needs to be viewed, but this can work as a stop-gap as the cache still exists.
Dependencies
This should be merged once the corresponding brapi prod server branch is merged.
Testing
Simply run a Germplasm import on a large dataset, and note the performance difference when enabled vs. disabled. Germs are still able to load when you view the Germplasm tab since it asks for a refresh anyways.
Checklist: