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

rpc_util: revise limit reader N to maxReceiveMessageSize #5593

Closed
wants to merge 1 commit into from

Conversation

lichunzhu
Copy link

@lichunzhu lichunzhu commented Aug 18, 2022

Currently, limitReader's stop bytes in decompress is set to maxReceiveMessageSize+1.

However, it seems that received message's length won't exceed maxReceiveMessageSize (we have a check before). It maxReceiveMessageSize is set to maxInt server can't read any data from decompress function because maxReceiveMessageSize+1 will overflow MAX_INT64, as the problem in pingcap/tidb-binlog#1152 (comment).

So I guess we can change this limit to maxReceiveMessageSize.

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Aug 18, 2022

CLA Signed

The committers listed above are authorized under a signed CLA.

  • ✅ login: lichunzhu / name: Chunzhu Li (ee47fc3)

@easwars
Copy link
Contributor

easwars commented Aug 24, 2022

However, it seems that received message's length won't exceed maxReceiveMessageSize (we have a check before).

This is true only when the anonymous interface with DecompressedSize() method is implemented, and this is currently true only for gzip.

And could you please clarify your motivation behind this change? Unless this is breaking something in a very fundamental way, we are inclined to not change the way things are currently.

Also, if you have a strong reason to go ahead and make this change, we would also like to see some tests added. Thanks.

@github-actions
Copy link

This PR is labeled as requiring an update from the reporter, and no update has been received after 6 days. If no update is provided in the next 7 days, this issue will be automatically closed.

@github-actions github-actions bot added the stale label Aug 30, 2022
@github-actions github-actions bot closed this Sep 6, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 6, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants