Parcourir la source

Improve postgres connection attempts in edge cases

Nick Babcock il y a 6 ans
Parent
commit
5cd669bb0f
1 fichiers modifiés avec 87 ajouts et 84 suppressions
  1. 87 84
      OhmGraphite/TimescaleWriter.cs

+ 87 - 84
OhmGraphite/TimescaleWriter.cs

@@ -16,7 +16,6 @@ namespace OhmGraphite
         private readonly string _connStr;
         private readonly string _localHost;
         private readonly bool _setupTable;
-        private NpgsqlConnection _conn;
         private bool _failure = true;
 
         public TimescaleWriter(string connStr, bool setupTable, string localHost)
@@ -24,7 +23,6 @@ namespace OhmGraphite
             _connStr = connStr;
             _localHost = localHost;
             _setupTable = setupTable;
-            _conn = new NpgsqlConnection(_connStr);
         }
 
         public Task ReportMetrics(DateTime reportTime, IEnumerable<ReportedValue> sensors)
@@ -33,100 +31,105 @@ namespace OhmGraphite
             {
                 if (_failure)
                 {
-                    _conn.Close();
-                    _conn = new NpgsqlConnection(_connStr);
-                    Logger.Debug("New timescale connection");
-                    _conn.Open();
-
-                    // The reason behind unpreparing is a doozy.
-                    //
-                    // Npgsql persists prepared statements across connections, reason: "This allows
-                    // you to benefit from statements prepared in previous lifetimes, providing all
-                    // the performance benefits to applications using connection pools" -
-                    // http://www.roji.org/prepared-statements-in-npgsql-3-2. I have found this to
-                    // be the correct behavior in 99% situations when either client or server is
-                    // restarted, as the normal flow of exceptions reported on the client when the
-                    // server restarts seems to be:
-                    //
-                    // - System.IO.EndOfStreamException: Attempted to read past the end of the stream
-                    // - 57P03: the database system is starting up
-                    // - Back to normal
-                    //
-                    // However, on 2018-11-29 while upgrading timescale db (0.12.1 to 1.0.0) I
-                    // encountered a bizarre sequence of events
-                    //
-                    // - <start upgrade by restarting server>
-                    // - System.IO.EndOfStreamException: Attempted to read past the end of the stream
-                    // - 57P03: the database system is starting up
-                    // - 58P01: could not access file "timescaledb-0.12.1": No such file or directory
-                    // - <finished with: "ALTER EXTENSION timescaledb UPDATE;">
-                    // - 26000: prepared statement "_p1" does not exist
-                    //
-                    // OhmGraphite could never recover because Npgsql seemed adamant that the
-                    // prepared statement existed. And since Npgsql persists prepared statements in
-                    // it's connection pool all future connections are "poisioned" with this
-                    // prepared statement. The best solution appears to be unpreparing everything on
-                    // db failure. For our use case, recreating these prepared statements is a small
-                    // price to pay even if preparation is redundant.
-                    _conn.UnprepareAll();
+                    Logger.Debug("Clearing connection pool");
+                    NpgsqlConnection.ClearPool(new NpgsqlConnection(_connStr));
+                }
 
-                    if (_setupTable)
+                using (var conn = new NpgsqlConnection(_connStr))
+                {
+                    conn.Open();
+                    if (_failure)
                     {
-                        var setupSql = Resourcer.Resource.AsString("schema.sql");
-                        using (var cmd = new NpgsqlCommand(setupSql, _conn))
+                        // The reason behind unpreparing is a doozy.
+                        //
+                        // Npgsql persists prepared statements across connections, reason: "This allows
+                        // you to benefit from statements prepared in previous lifetimes, providing all
+                        // the performance benefits to applications using connection pools" -
+                        // http://www.roji.org/prepared-statements-in-npgsql-3-2. I have found this to
+                        // be the correct behavior in 99% situations when either client or server is
+                        // restarted, as the normal flow of exceptions reported on the client when the
+                        // server restarts seems to be:
+                        //
+                        // - System.IO.EndOfStreamException: Attempted to read past the end of the stream
+                        // - 57P03: the database system is starting up
+                        // - Back to normal
+                        //
+                        // However, on 2018-11-29 while upgrading timescale db (0.12.1 to 1.0.0) I
+                        // encountered a bizarre sequence of events
+                        //
+                        // - <start upgrade by restarting server>
+                        // - System.IO.EndOfStreamException: Attempted to read past the end of the stream
+                        // - 57P03: the database system is starting up
+                        // - 58P01: could not access file "timescaledb-0.12.1": No such file or directory
+                        // - <finished with: "ALTER EXTENSION timescaledb UPDATE;">
+                        // - 26000: prepared statement "_p1" does not exist
+                        //
+                        // OhmGraphite could never recover because Npgsql seemed adamant that the
+                        // prepared statement existed. And since Npgsql persists prepared statements in
+                        // it's connection pool all future connections are "poisioned" with this
+                        // prepared statement. The best solution appears to be unpreparing everything on
+                        // db failure. For our use case, recreating these prepared statements is a small
+                        // price to pay even if preparation is redundant.
+                        conn.UnprepareAll();
+
+                        if (_setupTable)
                         {
-                            cmd.ExecuteNonQuery();
+                            var setupSql = Resourcer.Resource.AsString("schema.sql");
+                            using (var cmd = new NpgsqlCommand(setupSql, conn))
+                            {
+                                cmd.ExecuteNonQuery();
+                            }
                         }
                     }
-                }
 
-                var values = sensors.ToList();
-                using (var cmd = new NpgsqlCommand(BatchedInsertSql(values), _conn))
-                {
-                    // Note that all parameters must be set before calling Prepare()
-                    // they are part of the information transmitted to PostgreSQL
-                    // and used to effectively plan the statement. You must also set
-                    // the DbType or NpgsqlDbType on your parameters to unambiguously
-                    // specify the data type (setting the value isn't support)
-                    for (int i = 0; i < values.Count; i++)
+                    var values = sensors.ToList();
+                    using (var cmd = new NpgsqlCommand(BatchedInsertSql(values), conn))
                     {
-                        cmd.Parameters.Add($"time{i}", NpgsqlDbType.TimestampTz);
-                        cmd.Parameters.Add($"host{i}", NpgsqlDbType.Text);
-                        cmd.Parameters.Add($"hardware{i}", NpgsqlDbType.Text);
-                        cmd.Parameters.Add($"hardware_type{i}", NpgsqlDbType.Text);
-                        cmd.Parameters.Add($"identifier{i}", NpgsqlDbType.Text);
-                        cmd.Parameters.Add($"sensor{i}", NpgsqlDbType.Text);
-                        cmd.Parameters.Add($"sensor_type{i}", NpgsqlDbType.Text);
-                        cmd.Parameters.Add($"value{i}", NpgsqlDbType.Real);
-                        cmd.Parameters.Add($"sensor_index{i}", NpgsqlDbType.Integer);
-                    }
+                        // Note that all parameters must be set before calling Prepare()
+                        // they are part of the information transmitted to PostgreSQL
+                        // and used to effectively plan the statement. You must also set
+                        // the DbType or NpgsqlDbType on your parameters to unambiguously
+                        // specify the data type (setting the value isn't support)
+                        for (int i = 0; i < values.Count; i++)
+                        {
+                            cmd.Parameters.Add($"time{i}", NpgsqlDbType.TimestampTz);
+                            cmd.Parameters.Add($"host{i}", NpgsqlDbType.Text);
+                            cmd.Parameters.Add($"hardware{i}", NpgsqlDbType.Text);
+                            cmd.Parameters.Add($"hardware_type{i}", NpgsqlDbType.Text);
+                            cmd.Parameters.Add($"identifier{i}", NpgsqlDbType.Text);
+                            cmd.Parameters.Add($"sensor{i}", NpgsqlDbType.Text);
+                            cmd.Parameters.Add($"sensor_type{i}", NpgsqlDbType.Text);
+                            cmd.Parameters.Add($"value{i}", NpgsqlDbType.Real);
+                            cmd.Parameters.Add($"sensor_index{i}", NpgsqlDbType.Integer);
+                        }
 
-                    // A majority of the time, the same number of sensors will be
-                    // reported on, so it's important to prepare the statement
-                    cmd.Prepare();
+                        // A majority of the time, the same number of sensors will be
+                        // reported on, so it's important to prepare the statement
+                        cmd.Prepare();
 
-                    for (int i = 0; i < values.Count; i++)
-                    {
-                        var sensor = values[i];
-                        cmd.Parameters[$"time{i}"].Value = reportTime;
-                        cmd.Parameters[$"host{i}"].Value = _localHost;
-                        cmd.Parameters[$"hardware{i}"].Value = sensor.Hardware;
-                        cmd.Parameters[$"hardware_type{i}"].Value = Enum.GetName(typeof(HardwareType), sensor.HardwareType);
-                        cmd.Parameters[$"identifier{i}"].Value = sensor.Identifier;
-                        cmd.Parameters[$"sensor{i}"].Value = sensor.Sensor;
-                        cmd.Parameters[$"sensor_type{i}"].Value = Enum.GetName(typeof(SensorType), sensor.SensorType);
-                        cmd.Parameters[$"value{i}"].Value = sensor.Value;
-                        cmd.Parameters[$"sensor_index{i}"].Value = sensor.SensorIndex;
-                    }
+                        for (int i = 0; i < values.Count; i++)
+                        {
+                            var sensor = values[i];
+                            cmd.Parameters[$"time{i}"].Value = reportTime;
+                            cmd.Parameters[$"host{i}"].Value = _localHost;
+                            cmd.Parameters[$"hardware{i}"].Value = sensor.Hardware;
+                            cmd.Parameters[$"hardware_type{i}"].Value = Enum.GetName(typeof(HardwareType), sensor.HardwareType);
+                            cmd.Parameters[$"identifier{i}"].Value = sensor.Identifier;
+                            cmd.Parameters[$"sensor{i}"].Value = sensor.Sensor;
+                            cmd.Parameters[$"sensor_type{i}"].Value = Enum.GetName(typeof(SensorType), sensor.SensorType);
+                            cmd.Parameters[$"value{i}"].Value = sensor.Value;
+                            cmd.Parameters[$"sensor_index{i}"].Value = sensor.SensorIndex;
+                        }
 
-                    cmd.ExecuteNonQuery();
-                }
+                        cmd.ExecuteNonQuery();
+                    }
 
-                _failure = false;
+                    _failure = false;
 
-                // The synchronous versions of npgsql are more battle tested than asynchronous:
-                // https://github.com/npgsql/npgsql/issues/2266
-                return Task.CompletedTask;
+                    // The synchronous versions of npgsql are more battle tested than asynchronous:
+                    // https://github.com/npgsql/npgsql/issues/2266
+                    return Task.CompletedTask;
+                }
             }
             catch (Exception)
             {