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

Updating Varnish default `CACHE_SIZE` to 500M #1585

Merged
merged 2 commits into from Feb 4, 2020
Merged

Conversation

@dasrecht
Copy link
Member

dasrecht commented Jan 24, 2020

The Default memory allowance can lead to content delivery issues due to a full cache. We're bumping the cache size of varnish to 500M

Checklist

  • Affected Issues have been mentioned in the Closing issues section
  • Documentation has been written/updated
  • PR title is ready for changelog and subsystem label(s) applied
@dasrecht dasrecht changed the title Updating Varnish cache size to 500m Updating Varnish default cache_size to 500m Jan 24, 2020
@dasrecht dasrecht changed the title Updating Varnish default cache_size to 500m Updating Varnish default `CACHE_SIZE` to 500M Jan 24, 2020
Copy link
Contributor

seanhamlin left a comment

I agree this is a sensible step in the right direction. One thing I have found that also works wonders is disabling the caching of 'large' binary files (e.g. .docx, .zip etc). Having an upper limit of say 10MB to which if you are over forces a cache miss would be far hugely beneficial for some sites.

  if ( beresp.http.Content-Length ~ "[0-9]{8,}" ) {
    set beresp.do_stream = true;
    return (pass);
  }

Unsure if you want to scope creep this PR, or whether this should be a follow up PR.

@dasrecht

This comment has been minimized.

Copy link
Member Author

dasrecht commented Jan 28, 2020

Oh that's a great idea! Totally fine with having this in this PR as long as we make it distinguishable in the changelog

@seanhamlin

This comment has been minimized.

Copy link
Contributor

seanhamlin commented Jan 28, 2020

Ah good point. Perhaps it should be a follow up PR then, then it will get its own changelog entry.

Raised #1613 to ensure we don't lose track of this.

@tobybellwood tobybellwood added this to the v1.3.0 milestone Feb 4, 2020
@Schnitzel Schnitzel merged commit a24ccb4 into master Feb 4, 2020
1 check failed
1 check failed
continuous-integration/jenkins/pr-merge This commit cannot be built
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
You can’t perform that action at this time.