Skip to content

Conversation

arianvp
Copy link
Member

@arianvp arianvp commented Jun 10, 2025

NAR files are immutable. There is no harm in caching them for longer than the default 24 hours. It might save us on bandwidth cost to S3 and lower costs?

NAR files are immutable. There is no harm in caching them for longer than the default 24 hours.    It might save us on bandwidth cost to S3 and lower costs?
@arianvp arianvp requested a review from edef1c June 10, 2025 12:30
@mweinelt
Copy link
Member

Does this only apply for successful (HTTP 200) responses?

@arianvp
Copy link
Member Author

arianvp commented Jun 10, 2025

Because the is-404 condition has priority = 0 I would expect it to take precedence. But to be honest I'm not very familiar with Fastly and VCL

We could perhaps add an explicit

cache_setting {
  name  = "cache-404"
  cache_condition = "is-404"
  ttl  = 86400 # 1 day
}

to be 100% sure?

@mweinelt
Copy link
Member

What I'm worried about is that someone could abuse the negative cache and block new build results, so limiting the duration on 404 responses sounds good to me.

@arianvp
Copy link
Member Author

arianvp commented Jun 10, 2025

What I'm worried about is that someone could abuse the negative cache and block new build results, so limiting the duration on 404 responses sounds good to me.

So I think this is already the behaviour due to this rule

https://github.com/NixOS/infra/blob/main/terraform/cache.tf#L233-L240

But we can add the explicit cache_setting to be sure.

@arianvp arianvp marked this pull request as ready for review June 10, 2025 20:23
@arianvp arianvp requested a review from a team as a code owner June 10, 2025 20:23
arianvp added 2 commits June 13, 2025 00:05
This allows us to test them in staging environment (I think?)
@arianvp
Copy link
Member Author

arianvp commented Jun 12, 2025

Okay this should now deploy the changes to staging environment as opposed to applying immediately.

Would love to apply this pairing together with someone

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add comment(s) to tge relevant part of this file explaining what we've learned about the is-404 codepath?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes will do

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/2025-s3-sponsorship-more-resources-for-a-sustainable-nix/67019/1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants