-
Notifications
You must be signed in to change notification settings - Fork 512
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
distributor: pass write request size via metadata #6490
distributor: pass write request size via metadata #6490
Conversation
Signed-off-by: Peter Štibraný <pstibrany@gmail.com>
Signed-off-by: Peter Štibraný <pstibrany@gmail.com>
pkg/distributor/distributor_test.go
Outdated
require.NoError(t, err) | ||
|
||
// Verify that d.send added message size to metadata. | ||
require.NotEmpty(t, strconv.Itoa(req.Size()), mock.md[grpcutil.MetadataMessageSize]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure this is asserting what we want it to be asserting?
We're asserting that strconv.Itoa(size)
is not empty but even a request size of 0
will be converted to "0"
which isn't an empty string. Also I don't understand why we're passing the metadata as the msgAndArgs
argument to require.NotEmpty()
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure this is asserting what we want it to be asserting?
You're right, it should be equality test. Fixed in 5ff6b54.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes!
* Distributor: pass size of write request in grpc metadata. Signed-off-by: Peter Štibraný <pstibrany@gmail.com>
What this PR does
This PR makes distributor to pass size of WriteRequest in gRPC request metadata. That allows ingester to check for the size before reading request into memory.
Changelog will be added after implementing request size check on ingesters. (#6492)
Checklist
CHANGELOG.md
updated - the order of entries should be[CHANGE]
,[FEATURE]
,[ENHANCEMENT]
,[BUGFIX]