Add HTTP(s) Subscriptions #6655
Conversation
|
By analyzing the blame information on this pull request, we identified @gunnaraasen and @e-dard to be potential reviewers |
| Database: p.Database, | ||
| RetentionPolicy: p.RetentionPolicy, | ||
| }) | ||
| for _, p := range p.Points { |
There was a problem hiding this comment.
I'm not so fond of re-using an outer variable name for a different type... maybe pt for the internal or r for the WritePointsRequest?
|
Made a few comments here and there. My biggest concern is the serialization effect of waiting for the round-trip for HTTP subscriptions. Even for a single subscription target, each |
|
@joelegasse I have updated the service to sends writes over a channel to each PointsWriter, this way writes can happen concurrently. Have a look and let me know what you think. |
|
The concurrency aspect is nice, but once a single subscription's buffered channel becomes full, it blocks everything... I'm not sure how these write requests correspond to incoming write requests to the server, but if the server is handling a concurrent write load, and each of those requests result in one I'm not sure I'm being much help here, since I'm not familiar enough with subscriptions to provide more knowledgeable recommendations, but... How would you feel about just dropping new |
|
LGTM 👍 |
Adds support for the
httpandhttpsprotocols for subscriptions.To use it specify a destination like so:
If a single HTTP based backed slows down for what ever reason all subscriptions will be effected but normal writes to InfluxDB will continue without issues. Should we isolate each of the destinations?
A config option has been added to the
[subscription]section calledhttp-timeoutwhich specifies the client timeout when writing data to any http destination. Default is 30s