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

[Issue 240] Add check for max message size #263

Merged
merged 3 commits into from
May 28, 2020
Merged

Conversation

izackwu
Copy link
Contributor

@izackwu izackwu commented May 26, 2020

Fixes #240

Motivation

Issue #240: Add client-side check for max message size

Modifications

  1. When creating a connection, try to get maxMessageSize from handshake
    response command. If it's not set, then use the default maxMessageSize
    value defined in the client side.

  2. When sending a message, check whether the size of payload exceeds
    maxMessageSize. If so, return error immediately without adding this
    meesage into sending queue.

  3. To implement these, I made some tiny modifications in Connection
    interface and added a field in its implementation struct.

1. When creating a connection, try to get maxMessageSize from handshake
response command. If it's not set, then use the default maxMessageSize
value defined in the client side.
2. When sending a message, check whether the size of payload exceeds
maxMessageSize. If so, return error immediately without adding this
meesage into sending queue.
3. To implement these, I made some tiny modifications in Connection
interface and added a field in its implementation struct.
Copy link
Member

@wolfstudy wolfstudy left a comment

Choose a reason for hiding this comment

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

The changes LGTM, @keithnull can you test case for this?

@wolfstudy wolfstudy added this to the 0.1.1 milestone May 26, 2020
@izackwu
Copy link
Contributor Author

izackwu commented May 26, 2020

@wolfstudy okay, I'll try to add some test cases for this change later

@izackwu izackwu requested a review from wolfstudy May 27, 2020 09:44
Copy link
Member

@wolfstudy wolfstudy left a comment

Choose a reason for hiding this comment

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

The change LGTM +1

pulsar/producer_partition.go Outdated Show resolved Hide resolved
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.

Check max message size in go client
3 participants