Skip to content
This repository has been archived by the owner on Jun 19, 2022. It is now read-only.

Requests with very large payload (>1Gb) need to be gracefully handled #1754

Closed
Ectelion opened this issue Sep 22, 2020 · 3 comments · Fixed by #1772
Closed

Requests with very large payload (>1Gb) need to be gracefully handled #1754

Ectelion opened this issue Sep 22, 2020 · 3 comments · Fixed by #1772
Assignees
Labels
area/broker kind/bug Something isn't working priority/1 Blocks current release defined by release/* label or blocks current milestone release/2
Milestone

Comments

@Ectelion
Copy link
Contributor

Describe the bug
Currently, there seems to be no validation that gracefully handles the case with very large payload (>1Gb are possible). Regardless of payload size, the Broker attempts to load it in-memory, which in some cases makes it prone to out-of-memory issues.

Expected behavior
There should be a cap on payload. The Broker shouldn't attempt to process requests with payload exceeding the cap, but raise a client error (i.e. a response code like 4xx). It probably makes sense to make this cap configurable.

To Reproduce
Both of the following tests resulted in a repro, although this must also be reproducible under a lower RPS.
Test 1: Generate the load of ~100 RPS with 3.5Gb request payload.
Test 2: Generate the load of ~1000 RPS with 256Mb request payload.

Exit critera
A unit test validating this behavior.

@Ectelion Ectelion added kind/bug Something isn't working area/broker labels Sep 22, 2020
@Ectelion
Copy link
Contributor Author

I think the max cap should be lower than both of (1) default Pub/Sub throughput quota and (2) OustandingBytesLimit on ReceiverSettings (otherwise, the message may get stuck on PubSub).

@Ectelion Ectelion added the duplicate This issue or pull request already exists label Sep 22, 2020
@Ectelion
Copy link
Contributor Author

Actually, same issue was just reported in #1753, so we may consider this as a duplicate.

@grantr grantr reopened this Sep 22, 2020
@grantr grantr added priority/1 Blocks current release defined by release/* label or blocks current milestone release/2 and removed duplicate This issue or pull request already exists labels Sep 22, 2020
@grantr grantr added this to the Backlog milestone Sep 22, 2020
@Ectelion Ectelion modified the milestones: Backlog, v0.18.0-M3 Sep 25, 2020
@Ectelion Ectelion self-assigned this Sep 25, 2020
@Ectelion
Copy link
Contributor Author

There is actually a bit wider issue: down the page with PubSub quotas, there appears to be an additional table with non-extendable quotas. It specifies "10Mb" as the max message size. Just in case, I've double checked and requests with payload above that are indeed not passing at the publishing stage.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area/broker kind/bug Something isn't working priority/1 Blocks current release defined by release/* label or blocks current milestone release/2
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants