]> granicus.if.org Git - icinga2/commitdiff
Fix TLS Race Connecting to InfluxDB
authorSimon Murray <spjmurray@yahoo.co.uk>
Mon, 14 Aug 2017 15:20:45 +0000 (16:20 +0100)
committerMichael Friedrich <michael.friedrich@icinga.com>
Thu, 17 Aug 2017 16:27:31 +0000 (18:27 +0200)
Rather than leaving stale connections about we tried to poll for data coming in
from InfluxDB and timeout if it didn't repond in a timely manner.  This introduced
a race where the timeout triggers, a context switch occurs where data is actually
available and the TlsStream spins trying to asynchronously notify that data is
available, but which never gets read.  Not only does this use up 100% of a core,
but it also slowly starves the system of handler threads at which point metrics
stop being delivered.

This basically removes the poll and timeout, any TLS socket erros should be
detected by TCP keep-alives.

Fixes #5460 #5469

refs #5504

doc/09-object-types.md
lib/perfdata/influxdbwriter.cpp
lib/perfdata/influxdbwriter.hpp
lib/perfdata/influxdbwriter.ti

index 78b59192ce77e193c9fa9cf30e932176bbbef2d8..cceed0bb799670405ba5d39d2189d0b78e0feb61 100644 (file)
@@ -926,7 +926,6 @@ Configuration Attributes:
   enable_send_metadata   | **Optional.** Whether to send check metadata e.g. states, execution time, latency etc.
   flush_interval         | **Optional.** How long to buffer data points before transfering to InfluxDB. Defaults to `10s`.
   flush_threshold        | **Optional.** How many data points to buffer before forcing a transfer to InfluxDB.  Defaults to `1024`.
-  socket_timeout         | **Optional.** How long to wait for InfluxDB to respond.  Defaults to `5s`.
 
 Note: If `flush_threshold` is set too low, this will always force the feature to flush all data
 to InfluxDB. Experiment with the setting, if you are processing more than 1024 metrics per second
index c6fa2c2c3966525ae351658a8fde54a95013d094..0a7f506fdabb816709b763f73ed68e1e6597fda3 100644 (file)
@@ -134,9 +134,9 @@ void InfluxdbWriter::ExceptionHandler(boost::exception_ptr exp)
        //TODO: Close the connection, if we keep it open.
 }
 
-Stream::Ptr InfluxdbWriter::Connect(TcpSocket::Ptr& socket)
+Stream::Ptr InfluxdbWriter::Connect()
 {
-       socket = new TcpSocket();
+       TcpSocket::Ptr socket = new TcpSocket();
 
        Log(LogNotice, "InfluxdbWriter")
            << "Reconnecting to InfluxDB on host '" << GetHost() << "' port '" << GetPort() << "'.";
@@ -423,8 +423,7 @@ void InfluxdbWriter::Flush(void)
        String body = boost::algorithm::join(m_DataBuffer, "\n");
        m_DataBuffer.clear();
 
-       TcpSocket::Ptr socket;
-       Stream::Ptr stream = Connect(socket);
+       Stream::Ptr stream = Connect();
 
        if (!stream)
                return;
@@ -462,14 +461,6 @@ void InfluxdbWriter::Flush(void)
        HttpResponse resp(stream, req);
        StreamReadContext context;
 
-       struct timeval timeout = { GetSocketTimeout(), 0 };
-
-       if (!socket->Poll(true, false, &timeout)) {
-               Log(LogWarning, "InfluxdbWriter")
-                   << "Response timeout of TCP socket from host '" << GetHost() << "' port '" << GetPort() << "'.";
-               return;
-       }
-
        try {
                resp.Parse(context, true);
        } catch (const std::exception& ex) {
index dba570c45b1d03fa982585f2a03eadc42cb8ec71..e0a149c3a1503a737fba9396f7ddcfe88ef6eb1c 100644 (file)
@@ -74,7 +74,7 @@ private:
        static String EscapeKey(const String& str);
        static String EscapeField(const String& str);
 
-       Stream::Ptr Connect(TcpSocket::Ptr& socket);
+       Stream::Ptr Connect();
 
        void AssertOnWorkQueue(void);
 
index 94188471f6426dc8e15d833a77b4d88daffc136f..a2bf1924802258209bdeb63ba356b6100035a6bc 100644 (file)
@@ -90,9 +90,6 @@ class InfluxdbWriter : ConfigObject
        [config] int flush_threshold {
                default {{{ return 1024; }}}
        };
-       [config] int socket_timeout {
-               default {{{ return 5; }}}
-       };
 };
 
 validator InfluxdbWriter {