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

No Cache-Control headers on IA CDN requests #8927

Closed
RayBB opened this issue Mar 20, 2024 · 6 comments · Fixed by #8944
Closed

No Cache-Control headers on IA CDN requests #8927

RayBB opened this issue Mar 20, 2024 · 6 comments · Fixed by #8944
Assignees
Labels
Lead: @RayBB Issues overseen by Ray (Onboarding & Documentation Lead) [manages] Priority: 2 Important, as time permits. [managed] Theme: Performance Issues related to UI or Server performance. [managed] Type: Bug Something isn't working. [managed]

Comments

@RayBB
Copy link
Collaborator

RayBB commented Mar 20, 2024

Problem

These two JS load on every page: https://openlibrary.org/cdn/archive.org/donate.js and. https://openlibrary.org/cdn/archive.org/analytics.js

However, they are not cached like the rest of the JS.
This causes a lot (2 seconds) delay when on a very slow connection (only a few 100ms on a decent connection).

Their headers:

Content-Encoding: gzip
Content-Type: text/javascript
Date: Wed, 20 Mar 2024 11:48:52 GMT
Referrer-Policy: no-referrer-when-downgrade
Server: nginx/1.18.0 (Ubuntu)
Vary: Accept-Encoding
X-Ol-Stats: ""

Headers for all other requests:

Cache-Control: max-age=315360000
Content-Encoding: gzip
Content-Type: application/javascript
Date: Wed, 20 Mar 2024 11:03:17 GMT
Etag: W/"65e78c3f-69215"
Expires: Thu, 31 Dec 2037 23:55:55 GMT
Last-Modified: Tue, 05 Mar 2024 21:18:55 GMT
Referrer-Policy: no-referrer-when-downgrade
Server: nginx/1.18.0 (Ubuntu)

Evidence / Screenshot

image

Relevant URL(s)

Reproducing the bug

No response

Context

No response

Notes from this Issue's Lead

Proposal & constraints

Add cache control headers.

Probably should also add defer or async to their script tags so they don't block loading when they're not cached but this could be a different issue.
https://javascript.info/script-async-defer

Related files

class ia_js_cdn(delegate.page):
path = r'/cdn/archive.org/(donate\.js|analytics\.js)'
def GET(self, filename):
web.header('Content-Type', 'text/javascript')
return web.ok(fetch_ia_js(filename))

<script src="/cdn/archive.org/analytics.js" type="text/javascript"></script>
<script src="$static_url('build/all.js')" type="text/javascript"></script>

<div id="donato"></div>
<script src="%s" data-platform="ol"></script>

Stakeholders

PS: I don't have bandwidth to take this right now.

@RayBB RayBB added Type: Bug Something isn't working. [managed] Needs: Triage This issue needs triage. The team needs to decide who should own it, what to do, by when. [managed] Needs: Lead labels Mar 20, 2024
@QuantuM410
Copy link
Contributor

Hey! I would like to work on this issue :)

@RayBB
Copy link
Collaborator Author

RayBB commented Mar 20, 2024

@QuantuM410 go for it!

@mekarpeles mekarpeles added Theme: Performance Issues related to UI or Server performance. [managed] Priority: 2 Important, as time permits. [managed] Lead: @RayBB Issues overseen by Ray (Onboarding & Documentation Lead) [manages] and removed Needs: Triage This issue needs triage. The team needs to decide who should own it, what to do, by when. [managed] Needs: Lead labels Mar 25, 2024
@mekarpeles mekarpeles added this to the Sprint 2024-03 milestone Mar 25, 2024
@cdrini
Copy link
Collaborator

cdrini commented Mar 29, 2024

Since the deploy, the number of transactions to this endpoint are ~1/4 of what they were before!
image

@RayBB
Copy link
Collaborator Author

RayBB commented Mar 29, 2024

@cdrini that's great thanks for the update! Is TPM thousands per minute?

Good job @QuantuM410 ! Very nice impact :)

@cdrini
Copy link
Collaborator

cdrini commented Mar 29, 2024

Transactions per minute!

@cdrini
Copy link
Collaborator

cdrini commented Mar 29, 2024

This was quite useful; would you mind creating a new issue to add the headings to partials.json ? That's another one which I think would work well with some browser-side caching! CC @jimchamp

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Lead: @RayBB Issues overseen by Ray (Onboarding & Documentation Lead) [manages] Priority: 2 Important, as time permits. [managed] Theme: Performance Issues related to UI or Server performance. [managed] Type: Bug Something isn't working. [managed]
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants