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

Increase max-content-length to 50 MB #4059

Merged
merged 2 commits into from
Oct 12, 2018

Conversation

chetanmeh
Copy link
Member

@chetanmeh chetanmeh commented Oct 8, 2018

With current setup reading code attachment more than 8 MB results in error (#4028). Increase max-content-length to 50 MB allows reading code attachments upto maximum allowed 48 MB

Description

Akka uses max-content-length to ensure limits on incoming request entities. By default it is 8 MB while OpenWhisk allows max code upto 48 MB. To enable such large action code as attachment increase the max-content-length to 50 MB

Basic Test

This PR also adds a system basic test to ensure that such large actions can be created. Due to constrained resource on travis test only create action code upto 10 MB. However it can be increased via config by setting system property whisk.action.size.max (in MB)

This test would also help to validate if the proxy and other settings are properly configured. See #3712 for details on which all settings need to be configured

Related issue and scope

My changes affect the following components

  • API
  • Controller
  • Message Bus (e.g., Kafka)
  • Loadbalancer
  • Invoker
  • Intrinsic actions (e.g., sequences, conductors)
  • Data stores (e.g., CouchDB)
  • Tests
  • Deployment
  • CLI
  • General tooling
  • Documentation

Types of changes

  • Bug fix (generally a non-breaking change which closes an issue).
  • Enhancement or new feature (adds new functionality).
  • Breaking change (a bug fix or enhancement which changes existing behavior).

Checklist:

  • I signed an Apache CLA.
  • I reviewed the style guides and followed the recommendations (Travis CI will check :).
  • I added tests to cover my changes.
  • My changes require further changes to the documentation.
  • I updated the documentation where necessary.

@codecov-io
Copy link

Codecov Report

Merging #4059 into master will decrease coverage by 4.9%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4059      +/-   ##
==========================================
- Coverage   86.07%   81.16%   -4.91%     
==========================================
  Files         147      147              
  Lines        7173     7173              
  Branches      449      449              
==========================================
- Hits         6174     5822     -352     
- Misses        999     1351     +352
Impacted Files Coverage Δ
...core/database/cosmosdb/RxObservableImplicits.scala 0% <0%> (-100%) ⬇️
...core/database/cosmosdb/CosmosDBArtifactStore.scala 0% <0%> (-95.52%) ⬇️
...sk/core/database/cosmosdb/CosmosDBViewMapper.scala 0% <0%> (-92.6%) ⬇️
...whisk/core/database/cosmosdb/CosmosDBSupport.scala 0% <0%> (-83.34%) ⬇️
...abase/cosmosdb/CosmosDBArtifactStoreProvider.scala 0% <0%> (-58.83%) ⬇️
...la/whisk/core/database/cosmosdb/CosmosDBUtil.scala 92% <0%> (-4%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5da62ed...2504c7a. Read the comment docs.

@@ -349,6 +349,10 @@ public static boolean onLinux() {
return osname.equalsIgnoreCase("linux");
}

public static int getMaxActionSizeMB(){
return Integer.parseInt(getProperty("whisk.action.size.max", "10"));
Copy link
Member

Choose a reason for hiding this comment

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

@chetanmeh, where is whisk.action.size.max set?

Copy link
Member Author

Choose a reason for hiding this comment

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

Currently its not set and hence defaults to 10 MB. I wish to configure this (via system property) when we run system basic test against out setup with the max value that the cluster supports.

@chetanmeh
Copy link
Member Author

@dubee @markusthoemmes Please review

Copy link
Member

@rabbah rabbah left a comment

Choose a reason for hiding this comment

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

LGTM.

@rabbah rabbah merged commit 4783c99 into apache:master Oct 12, 2018
@chetanmeh chetanmeh deleted the support-big-action branch November 6, 2018 12:02
BillZong pushed a commit to BillZong/openwhisk that referenced this pull request Nov 18, 2019
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