You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by GitBox <gi...@apache.org> on 2019/04/17 21:36:50 UTC

[GitHub] [incubator-superset] betodealmeida opened a new pull request #7319: Cache invalidation

betodealmeida opened a new pull request #7319: Cache invalidation
URL: https://github.com/apache/incubator-superset/pull/7319
 
 
   ### CATEGORY
   
   Choose one
   
   - [ ] Bug Fix
   - [X] Enhancement (new features, refinement)
   - [ ] Refactor
   - [ ] Add tests
   - [ ] Build / Development Environment
   - [ ] Documentation
   
   ### SUMMARY
   <!--- Describe the change below, including rationale and design decisions -->
   
   This PR implements cache invalidation for charts (after https://github.com/apache/incubator-superset/pull/7032), in two places:
   
   (1) In the frontend, every time a `POST` is done by `SupersetClient` to get chart data, we invalidate the cache. This happens when the user clicks "force refresh" in a dashboard, or clicks "run query" in the explore view. When this happens, we extract the chart id (`slice_id`) and **delete all stored responses that reference that chart** — this would include, eg, a cached response of the chart with extra filters from a dashboard.
   
   (2) In the backend, we use a similar strategy. Unfortunately, we can't iterate over all the cached keys like in the frontend, to find all related cached responses. So we compute the key that would be generated **if a `GET` request was made**, and invalidate it.
   
   <!-- ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF -->
   <!--- Skip this if not applicable -->
   
   ### TEST PLAN
   <!--- What steps should be taken to verify the changes -->
   
   I've tested manually, by modifying charts and ensuring that dashboards refresh correctly:
   
   1. Create chart in explore view, browser does a `POST` request.
   2. Reload chart, browser does `GET` and caches the response.
   3. Reload chart, browser reuses cached response.
   4. Reload chart after cache expiration, browser does conditional response.
   5. Click "Run Query". Browser does a `POST`, cache is invalidated for that chart. Server-side cache is also invalidated.
   
   Similarly for dashboards. Before, if you clicked force refresh on a dashboard it would performa a `POST` and show the new data. But reloading the dashboard would show the old charts, since they were still cached in the browser.
   
   I tried adding Cypress integration tests, but for some reason I get an error when writing to the Cache API (it works fine in Chrome) from an integration test:
   
   ```javascript
   TypeError: Failed to execute 'put' on 'Cache': parameter 2 is not of type 'Response'.
       at eval (webpack-internal:///./node_modules/@superset-ui/connection/esm/callApi/callApi.js:17)
   ```
   
   ### ADDITIONAL INFORMATION
   <!--- Check any relevant boxes with "x" -->
   <!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
   - [ ] Has associated issue:
   - [ ] Changes UI
   - [ ] Requires DB Migration.
   - [ ] Confirm DB Migration upgrade and downgrade tested.
   - [X] Introduces new feature or API
   - [ ] Removes existing feature or API
   
   ### REVIEWERS
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org