Set larger maxInboundMetadataSize to accommodate unbounded Server error messages#1397
Merged
Spikhalskiy merged 1 commit intotemporalio:masterfrom Aug 31, 2022
Merged
Conversation
bergundy
reviewed
Aug 30, 2022
| .maxInboundMessageSize(MAX_INBOUND_MESSAGE_SIZE) | ||
| // to accommodate unbounded Server status messages that may be over the default limit of | ||
| // 8Kb | ||
| .maxInboundMetadataSize(MAX_INBOUND_MESSAGE_SIZE / 8); |
Member
There was a problem hiding this comment.
Why not use a separate constant for this? Not seeing how this is tied to message size.
Contributor
Author
There was a problem hiding this comment.
Well, this is the problem. What should be the number? And if we don’t know, how is a separate constant really better than tangling these two values together?
The benefit of connecting them is that it’s actually easier to say how many MAX_INBOUND_MESSAGE_SIZEs do we allocate on the buffers (like 1.12x right now).
But I don’t care much, we can chose some random really large size that “should be enough”. Like 1Mb.
bergundy
approved these changes
Aug 30, 2022
2e6d5d9 to
cca6767
Compare
cca6767 to
a6830da
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Sets larger
maxInboundMetadataSize = MAX_INBOUND_MESSAGE_SIZE / 8to accommodate unbounded Server error messagesCloses #1396