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

fix: set cache control for only _next folder #851

Merged
merged 1 commit into from
Nov 1, 2024
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 6 additions & 4 deletions tooling/build/scripts/publisher.sh
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,12 @@ ls -al
# Publish to S3
echo "Publishing to S3..."
start_time=$(date +%s)
aws s3 sync . s3://$S3_BUCKET_NAME/$SITE_NAME/$CODEBUILD_BUILD_NUMBER/latest --delete --no-progress

# Set all files to have 10 minutes of cache, except for those in the _next folder
aws s3 sync . s3://$S3_BUCKET_NAME/$SITE_NAME/$CODEBUILD_BUILD_NUMBER/latest --delete --no-progress --cache-control "max-age=600" --exclude "_next/*"

# Set all files in the _next folder to have 1 day of cache
aws s3 sync _next s3://$S3_BUCKET_NAME/$SITE_NAME/$CODEBUILD_BUILD_NUMBER/latest/_next --delete --no-progress --cache-control "max-age=86400"

# Update CloudFront origin path
echo "Updating CloudFront origin path..."
Expand All @@ -119,6 +124,3 @@ echo "ETag: $ETag"
jq '.Distribution.DistributionConfig' distribution.json > distribution-new.json
jq ".Origins.Items[0].OriginPath = \"/$SITE_NAME/$CODEBUILD_BUILD_NUMBER/latest\"" distribution-new.json > distribution-config.json
aws cloudfront update-distribution --id $CLOUDFRONT_DISTRIBUTION_ID --distribution-config file://distribution-config.json --if-match $ETag

# Invalidate CloudFront cache
Copy link
Contributor

Choose a reason for hiding this comment

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

if you don't invalidate, the max time for a change to be live is 30 mins of build + 10 mins of previous cache = 40 mins right

I think we should still invalidate the cache, but having the TTL longer for _next helps to solve for those weird edge cases where the old versions of the HTMLs are being loaded and is unable to find the right _next chunk

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm no, we can't invalidate because it is the CloudFront invalidation process that is opaque to us and caused us some issues. If we perform the invalidation, then the cached file in CloudFront gets removed, which then causes the 404 errors for the chunks that we were seeing earlier. We still want the old cached file in CloudFront to be retained so that in case users are still seeing the old index.html file which references these old chunks, they don't receive a 404.

Copy link
Contributor

Choose a reason for hiding this comment

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

okay so what is the maximum time for a change to be live to MOPs now? is 10 mins + build time right?

If builds are still around 20-30 mins + 10mins, I think we still need to optimise this to be faster, but can be a future improvement

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah in the worst case would be 10 minutes + build times but I think we should accept this because otherwise we lose the benefits of having CloudFront provide the caching for us, which can affect our time to first byte.

aws cloudfront create-invalidation --distribution-id $CLOUDFRONT_DISTRIBUTION_ID --paths "/*"
Loading