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

grpc: Fix encoded message size reported in error message #8033

Merged
merged 1 commit into from
Jan 24, 2025

Conversation

arjan-bal
Copy link
Contributor

@arjan-bal arjan-bal commented Jan 23, 2025

This PR fixes two bugs in the encoded message size error message:

  1. Using len(b) returns the number of buffers in a BufferSlice and not the number of bytes in the BufferSlice.
  2. Finding the size after calling b.Free() would return 0 if there are no remaining references to the buffers within the slice.

RELEASE NOTES:

  • grpc: Fix the number of bytes reported in the error message when encoded messages are larger than 4GB.

@arjan-bal arjan-bal added the Area: RPC Features Includes Compression, Encoding, Attributes/Metadata, Interceptors. label Jan 23, 2025
@arjan-bal arjan-bal added this to the 1.71 Release milestone Jan 23, 2025
@arjan-bal arjan-bal requested a review from easwars January 23, 2025 19:10
Copy link
Member

@dfawley dfawley left a comment

Choose a reason for hiding this comment

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

Is there an open issue that this fixes or did you just happen to notice this?

Copy link

codecov bot commented Jan 23, 2025

Codecov Report

Attention: Patch coverage is 0% with 2 lines in your changes missing coverage. Please review.

Project coverage is 82.27%. Comparing base (67bee55) to head (ca0af15).
Report is 4 commits behind head on master.

Files with missing lines Patch % Lines
rpc_util.go 0.00% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #8033      +/-   ##
==========================================
+ Coverage   82.22%   82.27%   +0.04%     
==========================================
  Files         383      382       -1     
  Lines       38688    38711      +23     
==========================================
+ Hits        31813    31848      +35     
+ Misses       5555     5547       -8     
+ Partials     1320     1316       -4     
Files with missing lines Coverage Δ
rpc_util.go 80.67% <0.00%> (-0.22%) ⬇️

... and 30 files with indirect coverage changes

@arjan-bal
Copy link
Contributor Author

Is there an open issue that this fixes or did you just happen to notice this?

For debugging #8023, I changed the type of BufferSlice from a slice to a struct. I was storing stack traces along with the BufSlice objects. This broke the build and I spotted this issue when fixing all the places that relied on BufSlice being a slice.

@arjan-bal arjan-bal assigned arjan-bal and unassigned easwars Jan 24, 2025
@arjan-bal arjan-bal merged commit 35cec50 into grpc:master Jan 24, 2025
14 of 15 checks passed
@arjan-bal arjan-bal deleted the fix-buf-slice-len branch January 24, 2025 04:53
purnesh42H pushed a commit to purnesh42H/grpc-go that referenced this pull request Jan 30, 2025
janardhanvissa pushed a commit to janardhanvissa/grpc-go that referenced this pull request Feb 13, 2025
janardhanvissa pushed a commit to janardhanvissa/grpc-go that referenced this pull request Feb 13, 2025
janardhanvissa pushed a commit to janardhanvissa/grpc-go that referenced this pull request Feb 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: RPC Features Includes Compression, Encoding, Attributes/Metadata, Interceptors. Type: Bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants