Fallback: "blocking" #1007
Fallback: "blocking" #1007
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1007 +/- ##
==========================================
+ Coverage 72.38% 72.47% +0.08%
==========================================
Files 75 75
Lines 2908 2917 +9
Branches 675 676 +1
==========================================
+ Hits 2105 2114 +9
Misses 671 671
Partials 132 132
Continue to review full report at Codecov.
|
|
@adamelmore I'd also really like to see ISR – I'm currently using fallback + a separate lambda that deletes pages from S3 periodically to get a poor man's version of ISR. I haven't been able to come up with a way to implement ISR without stepping outside lambda@edge, so I've been assuming CDK support and some core rework is needed first. |
I think the consensus was to submit to an SQS queue from the Lambda@Edge function, and have a lambda triggered from that queue. Dedup on the queue would be used to prevent multiple regen operations within the revalidate window. Noting that this would carry a bit of a latency hit; it would be faster to emit an SNS mitigation from Lambda@Edge to identically named topics in every region, with an SQS queue subscribed to each of those topics. The queue would handle dedup, and the function would still be triggered off of the queue. This would be much faster from the Lambda@Edge side (non-blocking), but also more complex. Any of that seem clear to you? |
That's more or less along the lines I was thinking. However, it would only support revalidate values larger than the 5-minute minimum deduplication window for SQS queues, right?
I don't really see the advantage. Whatever the edge lambda does, whether it's sending a message to a region-local SNS or a global SQS, it does not need to wait for confirmation, since at worst the message is lost and the next request sends a new one. The semantics of revalidation are (AFAIU) that rendering happens "at most" once in the revalidate window. |
|
Ah, sorry, I'm an idiot. The dedup could be based on metadata from S3, not just the key, so revalidate windows shorter than 5 minutes are certainly possible to support with SQS deduplication. |
You could be right here. I had thought it would be more latency to put a message on an SQS queue (living in a specific region) from any arbitrary edge location, even ignoring any ack back from SQS, but I've not tested this. |
| @@ -164,6 +179,33 @@ describe("Pages Tests", () => { | |||
| }); | |||
| }); | |||
|
|
|||
| describe("Dynamic SSG fallback blocking", () => { | |||
There was a problem hiding this comment.
I think you meant "Dynamic SSG fallback"?
There was a problem hiding this comment.
Not only that, but it duplicates (some of) the test above. It's a rebase artifact, fixed now.
There was a problem hiding this comment.
And since they duplicated the "not cached" test, they failed the e2e test run.
|
ran e2e tests again: https://github.com/serverless-nextjs/serverless-next.js/runs/2393671265?check_suite_focus=true. Looks like just one test had failed, can take a look later unless you have time to look at it |
I was able to reproduce. The request sometimes returns a cloudfront error because it has headers: The actual response code and content are correct (the test successfully checks them), but cloudfront understandably doesn't like those headers. It's probably already an issue before this pull request, but retries can get around it if the test case returns the object directly from S3 and then cloudfront has a chance to cache it. I think the bug is due to lambda-at-edge-compat rewriting the headers, but I haven't tried to understand that code yet. |
Hm, I didn't see this fail before in previous tests though...this one also failed in the previous e2e test run. So it might be some be related to your change? I know there's some existing strangeness in origin response, since we modifying whatever comes from s3 (the origin), sometimes I also saw "x-cache: Error from cloudfront" even if a proper page got returned |
I added a failing unit test and a fix for it. Looks to be already introduced in #544. |
|
Actually, looking at the headers brings up another issue. The initial SSG fallback data request (or page with blocking) had no cache control header so it took an extra request or two for cloudfront to cache it. Also shows up in the most recent e2e run as an extra retry for "serves and caches page /optional-catch-all-ssg-with-fallback/not-found" and likewise for /not-prerendered. |
Nice, thanks for a great find! I never realized that missing cache-control headers were why it was had the x-cache error at origin response, thought it was just CloudFront being flaky. Let me run the tests once again |
Implements fallback: "blocking" and tests for it.
Fixes #833
Actual change in default-handler was pretty minimal now that the rest of the SSG stuff works correctly.