[BI-1294] Implement Caching Layer for Experiments#264
Conversation
mlm483
left a comment
There was a problem hiding this comment.
Looks really good. I learned more about our caching layer.
Please consider my suggestions, but I don't consider them critical, this looks OK to merge.
| } | ||
| } | ||
|
|
||
| private Map<String, BrAPITrial> fetchProgramExperiment(UUID programId) throws ApiException { |
There was a problem hiding this comment.
Would fetchProgramExperiments be more accurate, as one Program can have many Experiments?
There was a problem hiding this comment.
Yep, good catch! updated.
| return experimentById(processExperimentsForDisplay(programExperiments, program.getKey())); | ||
| } | ||
|
|
||
| private Map<String, BrAPITrial> experimentById(List<BrAPITrial> trials) { |
There was a problem hiding this comment.
Would experimentsById be a more accurate method name?
There was a problem hiding this comment.
I'm leaving the name as is because it's describing the hashmap returned that stores a single experiment for a given id.
| public List<BrAPITrial> getTrialsByName(List<String> trialNames, Program program) throws ApiException { | ||
| Map<String, BrAPITrial> cache = programExperimentCache.get(program.getId()); | ||
| List<BrAPITrial> trials = new ArrayList<>(); |
There was a problem hiding this comment.
Tim mentioned that we may eventually use something like RediSearch to optimize these methods, might be worth putting a TODO comment anywhere that will need to be updated. Unless you think it will be obvious.
There was a problem hiding this comment.
Ok, I added a comment.
| public Optional<BrAPITrial> getTrialByDbId(String trialDbId, Program program) throws ApiException { | ||
| List<BrAPITrial> trials = getTrialsByDbIds(List.of(trialDbId), program); | ||
| return Utilities.getSingleOptional(trials); | ||
| } |
There was a problem hiding this comment.
Does it make sense to either
- remove this unused method, or
- update it to utilize the cache?
There was a problem hiding this comment.
It already uses the cache viagetTrialsByDbIds. As for the unused methods, it was decided in discussions with Tim to leave these in place for now.
73a732b to
b5c2e5d
Compare
cc6ba1c to
3945b2c
Compare
Description
Story: BI-1294
An instance of
ProgramCachewas added as a field inBrAPITrialDAOfor caching experiments on the redis server. Methods in the DAO were updated to create,update, and read experiemnts from the cache. The program key was stripped from the trial name before storing in the cache.Dependencies
none
Testing
Verify that experiment importing, exporting, and displaying of experiments in both group table and individual table still works as expected with lower latency.
Checklist: