-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
feat(http): add configurable limit to points batch size on write endpoint #16469
Conversation
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.
Approving. I need to make a ticket to add handling for http.StatusRequestEntityTooLarge to the client.
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.
This is an awesome improvement, @GeorgeMac 🥇
@@ -72,7 +93,7 @@ const ( | |||
) | |||
|
|||
// NewWriteHandler creates a new handler at /api/v2/write to receive line protocol. | |||
func NewWriteHandler(log *zap.Logger, b *WriteBackend) *WriteHandler { | |||
func NewWriteHandler(log *zap.Logger, b *WriteBackend, opts ...WriteHandlerOption) *WriteHandler { |
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.
❤️ the use of functional options
handleError = func(err error, code, message string) { | ||
h.HandleHTTPError(ctx, &influxdb.Error{ | ||
Code: code, | ||
Op: "http/handleWrite", | ||
Msg: message, | ||
Err: err, | ||
}, w) | ||
} |
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.
💯 for refactoring this to a function
|
||
code := influxdb.EInternal | ||
if errors.Is(err, ErrMaxBatchSizeExceeded) { | ||
code = influxdb.ETooLarge |
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.
Great to see the use of proper HTTP status codes 🎉
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.
You can tell what wikipedia page I spent some time on for this PR 😂
0686180
to
35d13eb
Compare
Closes #13100
This enables support for a limit on the points batch size in bytes (after compression if compression is enabled).
To configure this the functional option
WithMaxBatchSizeByte(50 * 1024 * 1024)
should be used when constructing ahttp.WriteHandler
.