Cache initial results of the Samples page #180
Conversation
- Move the existing Oncotree cache object "one level deeper" to the value of the key "oncotree" inside the main node-cache object. This accomplishes two things: (1) organize/add more data into the cache e.g. samples query results, and (2) call the method that refreshes the Oncotree cache only once when the Oncotree cache expires. Previously, we had a bug where this method was called for every single Oncotree cache item, leading to numerous calls to our database and the Oncotree API - Rename some instances of "Oncotree data" to "Oncotree API data" to make clearer Oncotree data from Neo4j vs. those from the API - Remove the includeCancerTypeFieldsInSearch function inside oncotree.ts that is no longer used and needed
| @@ -5,6 +5,7 @@ import { configureApp } from "./middlewares/configureApp"; | |||
| import { configureSession } from "./middlewares/configureSession"; | |||
| import { configureLogging } from "./middlewares/configureLogging"; | |||
| import { configureRoutes } from "./routes"; | |||
| require("log-timestamp"); // adds a timestamp to every log statement | |||
There was a problem hiding this comment.
To help us better diagnose console errors of the Express app in the future.
There was a problem hiding this comment.
Many parts in this file, especially around setting up of the Oncotree cache, comes from the old oncotree.ts. I renamed that file to this cache.ts to include all cache-related code.
See the current oncotree.ts for reference.
|
@ao508 This PR has been deployed to the dashboard in dev for your review. |
There was a problem hiding this comment.
I'm not sure what the right approach is for addressing discrepancies because it is a little awkward.
My flow for updating sample 08039_JQ_6
- I edited a sample in the samples view (
/samples)
As noted already in discussions, this update isn't going to show up on the /samples page until the cache refreshes
But it will show up if you search for the sample in the /samples view
And then if you were to immediately clear your search the table will go back to showing the outdated information
The options I can think of now are
- Put a pin in this issue for now and just communicate with the PMs that this discrepancy is going to exist for the time being
- Find a way to inject the update(s) to the sample(s) in the existing cache so as to make it seem as if the cache really is up to date
- Trigger a refresh when a sample update happens
- ?????
Quan's notes:
We discussed and aligned on option 2 as it seems like the most performant option.
@ao508 This has been fixed using option 2, deployed to dev, and tested with multiple sample updates. |
There was a problem hiding this comment.
I think it's very close to fixing that awkward behavior we noticed yesterday but still not quite there.
I updated the sample type for this sample to Normal and while the sample type field shows that update, the cmo sample name does not (I confirmed that the db entry did get an updated cmo sample name as a result of the update I made)
The issue is that we know what update the user wants to make on Submit ... but we technically don't know what the label is going to. be updated to (if it's updated).
Maybe this is a separate mechanism that we need to incorporate? Something that communicates that label update separately?
Actually while we're thinking about that.. what if the status and validation report become "failed"? I think we'd see a similar issue there..
We probably need to brainstorm this a bit more. One thing I can sorta imagine bridging this weird gap in data updates is to schedule a delayed query to neo4j with by the sample primary ids that we are expecting an update for?
Edit: To add - I think these are the only two pieces of info (cmoSampleName and status) that we're going to have to handle this way. Maybe we can even build a small query that just returns these two?
| DashboardSample, | ||
| | "primaryId" | ||
| | "cmoSampleName" | ||
| | "historicalCmoSampleNames" |
There was a problem hiding this comment.
good catch! I forgot about having to include this field in the query for latest updates.
This PR made the following changes:
This PR reduces the % memory usage from 44.5 to 43.8 (seen in dev).