Bug #19899
closed
Stale cache wb2 in collection file list
Description
Observed with the following sequence of events:
- Create an empty collection
- Visit the empty collection with Workbench 2
- Use arv-put --update-collection to upload a file to the collection
- Hit "Refresh" on the collection page
- The file list panel remains empty
The panel remains empty across visiting different collections and coming back as well as reloading the page, which suggests this is a caching issue, either the browser caching the keep-web response, or (more likely?) keep-web caching the collection listing.
After a couple minutes, the file I uploaded with arv-put started showing up.
Discussion on 1/5
keep-web supports "Cache-Control: no-cache" and "...must-revalidate" headers, but currently workbench2 does not use them, even when the user explicitly asks for a refresh.
Another possibility is for workbench2 to use the PDH rather than UUID when requesting file listings. (However, if the user clicks through to view file content at a webdav url, they might still receive old/cached content.)
Another possibility is adding a keep-web feature allowing the client's request header to supply the expected PDH when requesting file listings/content by collection UUID. If the PDH of the keep-web cache doesn't match, this would force a reload. This would be better than the previous approach in that when the user subsequently clicks through to the file content, the cache will also be up to date.
Plan¶
- Write a cypress test that demonstrates the bug (loads the collection page, then modifies the collection on the API server, then reloads the page and checks for the change)
- Add "Cache-control: must-revalidate" to the WebDAV requests
- Confirm that the cypress test passes with the addition of the Cache-control header.
Related issues
Updated by Peter Amstutz over 1 year ago
- Subject changed from Stale cache in collection file list to Stale cache wb2 in collection file list
Updated by Peter Amstutz over 1 year ago
- Target version changed from 2023-02-15 sprint to Future
Updated by Tom Clegg over 1 year ago
- No special feature needed in keep-web, just standard cache control that is already implemented (unlike the "pass pdh hint to keep-web" option)
- When the user clicks through to a keep-web link, the keep-web cache will be at least as current as the listing they were seeing in wb2 (unlike the "request listing by pdh, but link to keep-web by uuid" option)
Updated by Peter Amstutz about 1 year ago
- Story points set to 1.0
- Target version changed from Future to To be scheduled
- Description updated (diff)
Updated by Peter Amstutz about 1 year ago
- Target version changed from To be scheduled to 2023-02-15 sprint
Updated by Peter Amstutz about 1 year ago
- Target version changed from 2023-02-15 sprint to 2023-03-01 sprint
Updated by Brett Smith about 1 year ago
- Related to Bug #20142: Automatic refresh after removing files within a collection does not work added
Updated by Peter Amstutz about 1 year ago
- Target version changed from 2023-03-01 sprint to Development 2023-03-15 sprint
Updated by Stephen Smith about 1 year ago
- Status changed from New to In Progress
Updated by Stephen Smith about 1 year ago
Changes at arvados|aede6055f332c1680f6fe955e03e3e6364f8eca8 19899-webdav-cache-control
and arvados-workbench2|f3a16bca3ff6839b25baf86d0a0e96f63139efe9 19899-webdav-cache-control
Tests developer-tests-workbench2: #1114
- Added Cache-Control must-revalidate to webdav service by default as suggested above
- Changed webdav config field to private and added getters/setters to avoid consumers overwriting headers field
I wasn't able to figure out why the collection tests pass without the fix - it's easily verified to solve the problem outside of cypress when modifying a collection manifest through a PATCH request to the collections api, but the same operations inside cypress don't reproduce the issue. The absence of OPTIONS requests in the electron cypress runner also seems to be a red herring, as the firefox runner sends OPTIONS requests but still doesn't exhibit the issue. I suspect that the integration tests setup might do something that prevents the caching issue from presenting.
Updated by Lucas Di Pentima about 1 year ago
Some comments and findings:
- Previously existing cypress test didn't fail because the file
arvados-workbench2/tools/arvados_config.ymlconfiguresWebDAVCache.TTLto be zero seconds.- This is because the test was initially aimed to exercise the auto refresh mechanism when wb2 receives websockets events. I've just tried running the test with cache TTL set to 300s using commit
abb6537and it failed correctly. - I suggest you could just leave it at a non-zero value like 300s or just remove the entry entirely so that the defaults get applied.
- Note that when I tried to run the test with the cache conf value set to 300s on commit
f3a16bc, the test fails in what I think is an incorrect way. Perhaps that's a sign that the webdav code changes introduced some other issue?
- This is because the test was initially aimed to exercise the auto refresh mechanism when wb2 receives websockets events. I've just tried running the test with cache TTL set to 300s using commit
- I think it would also be useful to make sure the webdav code always sends the "must revalidate" header via a test. I think a unit test should be enough (and easier to write than a cypress test)
Updated by Stephen Smith about 1 year ago
Changes at arvados-workbench2|66bb2fcb669234cc9ab42eb19e253cbc3078c6cb
Tests developer-tests-workbench2: #1118
- Removed the webdav cache ttl - it seems the failing test was just a flaky test and it passes all tests fine after the cache-control header is added
- Added to webdav unit tests to check for cache-control header
Updated by Lucas Di Pentima about 1 year ago
Great! then it LGTM, please merge. Thanks!
Updated by Stephen Smith about 1 year ago
- Status changed from In Progress to Resolved
Updated by Peter Amstutz about 1 year ago
- Target version changed from Development 2023-03-15 sprint to Development 2023-03-29 Sprint
- Status changed from Resolved to Feedback
On tordo, running v2.6.0~dev20230315204706
I am testing the "banner" feature which uses CollectionService.getFileContents() which is implemented using WebDAV.get().
It is sending the header "Cache-Control: must-revalidate"
However, according to https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Cache-Control "must-revalidate" is only valid for responses, not requests.
I think we want Workbench to be using "no-cache" to ensure the browser re-fetches it every time.
Updated by Peter Amstutz about 1 year ago
19899-cache-control-fix @ e16f2125588512201e3bd22ae9a19e10cb69d367
Updated by Peter Amstutz about 1 year ago
- Status changed from Feedback to Resolved