Project

General

Profile

Actions

Bug #19899

closed

Stale cache wb2 in collection file list

Added by Peter Amstutz over 1 year ago. Updated about 1 year ago.

Status:
Resolved
Priority:
Normal
Assigned To:
Category:
Keep
Story points:
1.0
Release relationship:
Auto

Description

Observed with the following sequence of events:

  1. Create an empty collection
  2. Visit the empty collection with Workbench 2
  3. Use arv-put --update-collection to upload a file to the collection
  4. Hit "Refresh" on the collection page
  5. 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.

Subtasks 1 (0 open1 closed)

Task #20049: Review 19899-webdav-cache-controlResolvedLucas Di Pentima03/06/2023Actions

Related issues

Related to Arvados Workbench 2 - Bug #20142: Automatic refresh after removing files within a collection does not workDuplicateActions
Actions #1

Updated by Peter Amstutz over 1 year ago

  • Status changed from New to In Progress
Actions #2

Updated by Peter Amstutz over 1 year ago

  • Status changed from In Progress to New
Actions #3

Updated by Peter Amstutz over 1 year ago

  • Description updated (diff)
Actions #4

Updated by Peter Amstutz over 1 year ago

  • Description updated (diff)
Actions #5

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
Actions #6

Updated by Peter Amstutz over 1 year ago

  • Category set to Keep
Actions #7

Updated by Peter Amstutz over 1 year ago

  • Description updated (diff)
Actions #8

Updated by Tom Clegg over 1 year ago

  • Description updated (diff)
Actions #9

Updated by Peter Amstutz over 1 year ago

  • Target version changed from 2023-02-15 sprint to Future
Actions #10

Updated by Tom Clegg about 1 year ago

Adding the "Cache-control: must-revalidate" header seems to be the expedient solution here.
  • 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)
Actions #12

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)
Actions #13

Updated by Peter Amstutz about 1 year ago

  • Release set to 57
Actions #14

Updated by Peter Amstutz about 1 year ago

  • Target version changed from To be scheduled to 2023-02-15 sprint
Actions #15

Updated by Peter Amstutz about 1 year ago

  • Assigned To set to Stephen Smith
Actions #16

Updated by Peter Amstutz about 1 year ago

  • Target version changed from 2023-02-15 sprint to 2023-03-01 sprint
Actions #17

Updated by Brett Smith about 1 year ago

  • Related to Bug #20142: Automatic refresh after removing files within a collection does not work added
Actions #18

Updated by Peter Amstutz about 1 year ago

  • Target version changed from 2023-03-01 sprint to Development 2023-03-15 sprint
Actions #19

Updated by Stephen Smith about 1 year ago

  • Status changed from New to In Progress
Actions #20

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.

Actions #21

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.yml configures WebDAVCache.TTL to 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 abb6537 and 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?
  • 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)
Actions #22

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
Actions #23

Updated by Lucas Di Pentima about 1 year ago

Great! then it LGTM, please merge. Thanks!

Actions #24

Updated by Stephen Smith about 1 year ago

  • Status changed from In Progress to Resolved
Actions #25

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.

Actions #27

Updated by Peter Amstutz about 1 year ago

  • Status changed from Feedback to Resolved
Actions

Also available in: Atom PDF