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

Stream lambda function compiler responses #1340

Merged
merged 6 commits into from
Apr 16, 2024

Conversation

manuelwedler
Copy link
Collaborator

@manuelwedler manuelwedler commented Apr 10, 2024

See #1326

I didn't implement any chunking myself. Instead I used the pipeline function because the docs recommend to do so. AWS's responseStream will just take care of chunking it.

Note that that streamed responses also have a limit of 20 MB (usual payload response was 6 MB). Maybe it could make sense to fallback to local compilation if an error is received from the lambda function?

TODO:

  • Add a dashboard to Grafana when fallback is used

@marcocastignoli
Copy link
Member

Note that that streamed responses also have a limit of 20 MB (usual payload response was 6 MB). Maybe it could make sense to fallback to local compilation if an error is received from the lambda function?

I like this idea!

Copy link
Member

@marcocastignoli marcocastignoli left a comment

Choose a reason for hiding this comment

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

Just a small doubt regarding handling a JSON.parse potential error

forceEmscripten
);
} catch (e) {
logger.error(
Copy link
Member

@marcocastignoli marcocastignoli Apr 10, 2024

Choose a reason for hiding this comment

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

I think that here we need to run local compilation only on the specific "too big output" error not all the errors

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

(I pushed the new commit before I saw your comment update)

The problem with this is that I cannot really find out which error code is given by AWS in the case the response is too big. I was checking the docs and they were not very helpful (https://docs.aws.amazon.com/lambda/latest/api/API_InvokeWithResponseStreamCompleteEvent.html).

We could also try to find a contract whose compiler output is bigger than the response stream limit of 20 MB. I'm just not sure how to go about that.

Copy link
Member

@marcocastignoli marcocastignoli Apr 11, 2024

Choose a reason for hiding this comment

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

I'm thinking... Maybe on the lambda side you can find the output size and if it's more than 20mb send a specific response. Could this work?

Continues here: #1340 (comment)

};
console.debug("Compilation output: ", output);

const outputBlob = new Blob([JSON.stringify(output)]);
Copy link
Member

Choose a reason for hiding this comment

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

Continuing this comment: #1340 (comment)

like this:

if (weight(outputBlob) > 20MB ) {
  output = { error: "specific error for this case" }
  return or throw
}

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ah nice idea! That should work out! Thanks for the hint

@manuelwedler manuelwedler force-pushed the fix-lambda-response-limit branch from 8348703 to 08f8d8f Compare April 11, 2024 09:03
@manuelwedler
Copy link
Collaborator Author

manuelwedler commented Apr 15, 2024

I added the version identifiers to the environment configurations. This change will break the running staging and production deployments if the lambda function is not updated at the same time as these changes are merged. In the future, the versioning will prevent this, meaning that we can deploy a new lambda function and later just increment the version in the configuration.

In summary, we need to do the following:

  • At the same time as merging this PR, deploy the new lambda function (compile) and create a new version.
  • At the same time as making the next release, deploy the new lambda function (compile-production) and create a new version.

@manuelwedler
Copy link
Collaborator Author

I created a dashboard for visualizing the number of times the fallback to local compilation is invoked (the dashboard is empty since the changes are not released yet). I expect that this fallback will be barely used, but let's see in Grafana.

Copy link
Member

@marcocastignoli marcocastignoli left a comment

Choose a reason for hiding this comment

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

👍👍

@manuelwedler manuelwedler merged commit d464fb3 into staging Apr 16, 2024
3 of 4 checks passed
@manuelwedler manuelwedler deleted the fix-lambda-response-limit branch April 16, 2024 09:32
kuzdogan pushed a commit that referenced this pull request Apr 23, 2024
* Stream lambda function compiler responses

* Log parsing errors from Lambda response

* Fallback to local compilation if compilation with lambda function fails

* Only fallback to local compilation when lambda compilation failed due to the stream response limit

* SolcLambdaWithLocalFallback: log a warning instead of an error

* Add version identifiers to lambda function names
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: COMPLETED
Development

Successfully merging this pull request may close these issues.

3 participants