Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Caching issue when updating Web UI #3011

Closed
mofojed opened this issue Oct 21, 2022 · 2 comments · Fixed by #3038 or #3045
Closed

Caching issue when updating Web UI #3011

mofojed opened this issue Oct 21, 2022 · 2 comments · Fixed by #3038 or #3045
Assignees
Labels
bug Something isn't working

Comments

@mofojed
Copy link
Member

mofojed commented Oct 21, 2022

Description

There seems to still be an issue with caching the Web UI index.html. Even though I've updated the web version, it is returning the same ETag, loading the old version from memory. The Web UI then fails to load because the JS files no longer exist.

Steps to reproduce

  1. Run ./gradlew server-jetty-app:run
  2. Open http://localhost:10000/ide/ and let the UI load
  3. Stop the server
  4. Update WEB_VERSION in web/client-ui/Dockerfile to a new version
  5. Run ./gradlew server-jetty-app:run
  6. Open http://localhost:10000/ide/ and let the UI load

Expected results
6. New UI should load

Actual results
6. UI fails to load because of a caching issue. See screenshots.

Additional details and attachments

Before updating Web packages. ETag for /ide/ is W/"dw/nfoEwv/cdw/nNJbAiXY":
First load:
image
After refresh:
image

After updating Web Packages (on my branch web-v0.20.0):
First load, ETag still has the same value:
image
This causes the js pages not to load, as the hash for the js files is incorrect:
image

After a hard refresh, it loads properly, but still the same ETag W/"dw/nfoEwv/cdw/nNJbAiXY":
image

That ETag is different than the ETags for other files.

Versions

  • Deephaven: SHA 41e4b2f5790dabd0dceaa1663a043013ab4fac48
  • OS: Ubuntu
  • Browser: Chrome
@mofojed mofojed added bug Something isn't working triage labels Oct 21, 2022
@mofojed mofojed self-assigned this Oct 21, 2022
@mofojed mofojed removed the triage label Oct 21, 2022
@mofojed
Copy link
Member Author

mofojed commented Oct 21, 2022

@niloc132
Copy link
Member

Jetty only sends weak etags, which are a hash of the file's path, length, and last modified date. Additionally, Gradle sets a consistent last modified date for every file every build, so that unchanged files result in unchanged jars, but this means that a change to a file served by HTTP that doesn't change its length will not correctly update the etag.

Current proposal is that we disable etags again, and just make the client rely on dates, with the cache filter marking any hash-in-the-name files as cached forever (here defined as "for one year"). See #3016.

niloc132 added a commit that referenced this issue Oct 28, 2022
While in theory a weak etag should be sufficient, Jetty's implementation
is based only on name, file length, and last modified date. Gradle
defautls to giving all files the same last modified date to avoid two
builds of the same contents having different jar outputs, so for a given
file name, only its length can change, and if that length is the same
with different bytes, browsers will not get updates. While asking gradle
to not remove last modified dates would work, it also would require the
build to be slower by rebuilding downstream artifacts even though
contents did not change.

Fixes #3011
@niloc132 niloc132 reopened this Oct 31, 2022
niloc132 added a commit to niloc132/deephaven-core that referenced this issue Oct 31, 2022
Jetty always returns the last-modified date of the file, but Gradle
unsets the "true" last modified date in favor of ensuring that build
files cache only based on their contents. Disabling this feature is
possible, but still may present as buggy in Jetty, such as if a 1.0
release occurred, then 2.0, then a 1.1 hotfix - if an installation of
1.1 were later replaced with 2.0, the 1.1 files would be "newer" so
would be considered up to date.

This patch removes the last-modified value by only returning -1 when
requested. We could go further and return an etag that actually maps to
the release version or the contents of the file, but that is left for
future work.

Fixes deephaven#3011
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
2 participants