-
Notifications
You must be signed in to change notification settings - Fork 5.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
split metrics based on UDPPayload size #2795
Conversation
My only concern is that we will also be doing the splitting with the http client as well. I'll try to move the conversion from |
That's actually where we found the problem, the HTTP client was not parsing
data properly. The code to split at the db entry point is in the UDP
listener, not the HTTP.
In our case we send UDP from all telegraf collectors to a relay that
batches and forwards data to InfluxDB via HTTP. If the data is not parsed
BEFORE leaving telegraf, it doesn't get parsed properly in HTTP.
On Fri, May 12, 2017 at 2:28 PM Daniel Nelson ***@***.***> wrote:
My only concern is that we will also be doing the splitting with the http
client as well. I'll try to move the conversion from telegraf.Metric to
[]bytes into the Client interface.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#2795 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ADA1nh58UKlXieNNS0hBnOLbgIYLyZM3ks5r5LL2gaJpZM4NZmn4>
.
--
--
Sebastian Borza
PGP: EDC2 BF61 4B91 14F2 AAB4 06C9 3744 7F3F E411 0D3E
|
Changing the Client interface is going to be too big of a change for 1.3, so I think we should add a field to the InfluxDB struct called |
That sounds completely fine by me, we can patch our version in the meantime.
Thanks a lot for taking a look though!
On Fri, May 12, 2017 at 4:18 PM Daniel Nelson ***@***.***> wrote:
Changing the Client interface is going to be too big of a change for 1.3,
so I think we should add a field to the InfluxDB struct called
splitPayload that is true if there is at least one udp clients. Then we
split the metrics if that is true.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#2795 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ADA1nlpZOBtpCpmlCIZJKGRaCT-5_f9-ks5r5MzCgaJpZM4NZmn4>
.
--
--
Sebastian Borza
PGP: EDC2 BF61 4B91 14F2 AAB4 06C9 3744 7F3F E411 0D3E
|
This fix should go in 1.3.0, so I'll merge this now, add the |
This reverts commit b16eb6e.
Required for all PRs:
Issue Description
Without this fix, metric streams would send data blindly up to the max size of the
UDPPayload
buffer without processing newline and/or timestamps. This would result in malformed packets being processed at the DB entry point causing either partial writes or complete failed points.The patch here splits the measurements up to a max size of the
UDPPayload
value with enough room to add both the newline and timestamp information. This results in more packets being sent, but ensures the payload will be properly formatted for receipt at the DB entry point.