random choose host from provided hosts-list#31
Conversation
| config :resend_on_failure, :validate => :boolean, :default => false | ||
|
|
||
| # Number of attempts to be made resending metrics before abandoning | ||
| config :resend_attempts, :validate => :number, :default => 3 |
There was a problem hiding this comment.
This resend-attempts feature seems out of scope for the proposed change (add multiple hosts, choose at random). Can you move it to a separate PR?
| @logger.warn("Connection refused to graphite server, sleeping...", :host => @host, :port => @port) | ||
| sleep(@reconnect_interval) | ||
| retry | ||
| address, _, port = host.rpartition(":") |
There was a problem hiding this comment.
This won't work for ipv6 addresses that omit a port, such as ::1 which we may assume means address ::1 default port 2003. In this case, the address would become "::" and port "1"
| retry | ||
| address, _, port = host.rpartition(":") | ||
| @logger.debug? && @logger.debug("Trying to send metrics to", :address => address, :port => port) | ||
| socket(address, port).puts(message) |
There was a problem hiding this comment.
Am I misreading the code, maybe? It looks like this creates a new connection for every event which is a regression from the previous implementation which reused existing connection.
| @logger.debug? && @logger.debug("Trying to send metrics to", :address => address, :port => port) | ||
| socket(address, port).puts(message) | ||
| rescue Exception => e | ||
| @logger.debug? && @logger.debug("Suffering from", :e => e.message) |
There was a problem hiding this comment.
Suffering is probably not the right word. Also, catching Exception isn't the right thing, you probably want StandardError, if the case is that you want a general catch-all. Exception will catch things like SyntaxError or NoMemoryError that you probably dont' want to be catching here.
|
Also, lots of tests are failing for this PR: https://travis-ci.org/logstash-plugins/logstash-output-graphite/builds/240121828?utm_source=github_status&utm_medium=notification |
|
thank you for feedback @jordansissel! |
|
@jordansissel ping |
2 participants
supercedes #21