-
Notifications
You must be signed in to change notification settings - Fork 6
feat: report remaining egress quotas #583
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
base: main
Are you sure you want to change the base?
Changes from 3 commits
cccc2e7
7a6e01c
c30b39c
e2515ff
2f28238
fc9a4b5
11e36f7
7193801
de090e0
200c7f7
91a488e
a0ebbe4
760fca9
3bbe611
d288566
7ac7e62
5e16c99
7ff29ee
e9ea5b6
73a1452
9c6307c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -202,6 +202,27 @@ export default { | |
| 'Cache-Control', | ||
| `public, max-age=${env.CLIENT_CACHE_TTL}`, | ||
| ) | ||
| try { | ||
| const contentLengthHeader = | ||
| retrievalResult.response.headers.get('content-length') | ||
| const estimatedEgress = contentLengthHeader | ||
| ? Number.parseInt(contentLengthHeader, 10) || 0 | ||
| : 0 | ||
| const remainingCdn = | ||
| retrievalCandidate.cdnEgressQuota - BigInt(estimatedEgress) | ||
| const remainingCacheMiss = | ||
| retrievalCandidate.cacheMissEgressQuota - BigInt(estimatedEgress) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have mixed feelings about relying on content-length. It's not guaranteed that the response will have this header. I am concerned that the remaining egress reported by FilBeam will be unpredictable. Also, there may be other requests in progress that will further reduce the remaining egress quotas - I think it's impossible to give the client a precise value. I propose simplifying the report and reporting the remaining egress quota as it was at the start of the response, at the time sent back the headers. @pyropy @juliangruber thoughts?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
@bajtos You once mentioned that content length is encoded in CID V2, would it be possible to use that instead of the
That's true, we should document that this may report an imprecise amount back to the user.
That might be a good start. I think these headers might be used to raise alerts and trigger top-ups (either manual or automated) and not for precise accounting so that could work.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Yes, we can extract the piece size from the CID, but this will not work for range requests (e.g. when downloading a 1MB chunk of a video file, as browsers typically do).
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @bajtos +1 to simplifying the report, first get it simple and correct, we can improve it later |
||
| response.headers.set( | ||
| 'X-Cdn-Egress-Remaining', | ||
|
akronim26 marked this conversation as resolved.
Outdated
|
||
| String(remainingCdn < 0n ? 0n : remainingCdn), | ||
| ) | ||
| response.headers.set( | ||
| 'X-Cache-Miss-Egress-Remaining', | ||
| String(remainingCacheMiss < 0n ? 0n : remainingCacheMiss), | ||
| ) | ||
|
akronim26 marked this conversation as resolved.
Outdated
|
||
| } catch (e) { | ||
| console.warn('Failed to compute egress remaining headers', e) | ||
| } | ||
| return response | ||
| } | ||
|
|
||
|
|
@@ -288,6 +309,27 @@ export default { | |
| 'Cache-Control', | ||
| `public, max-age=${env.CLIENT_CACHE_TTL}`, | ||
| ) | ||
| try { | ||
|
akronim26 marked this conversation as resolved.
Outdated
|
||
| const contentLengthHeader = | ||
| retrievalResult.response.headers.get('content-length') | ||
| const estimatedEgress = contentLengthHeader | ||
| ? Number.parseInt(contentLengthHeader, 10) || 0 | ||
| : 0 | ||
| const remainingCdn = | ||
| retrievalCandidate.cdnEgressQuota - BigInt(estimatedEgress) | ||
| const remainingCacheMiss = | ||
| retrievalCandidate.cacheMissEgressQuota - BigInt(estimatedEgress) | ||
| response.headers.set( | ||
| 'X-Cdn-Egress-Remaining', | ||
| String(remainingCdn < 0n ? 0n : remainingCdn), | ||
| ) | ||
| response.headers.set( | ||
| 'X-Cache-Miss-Egress-Remaining', | ||
| String(remainingCacheMiss < 0n ? 0n : remainingCacheMiss), | ||
| ) | ||
| } catch (e) { | ||
| console.warn('Failed to compute egress remaining headers', e) | ||
| } | ||
| return response | ||
| } catch (error) { | ||
| const { status } = getErrorHttpStatusMessage(error) | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.