Added some new timeout stats. These measure timeouts on active databa…

…se operations, not on queued operations.
twitter-archive · Jan 4, 2011 · d93ffe3 · d93ffe3
1 parent a97fe9c
commit d93ffe3
Show file tree

Hide file tree

Showing 5 changed files with 63 additions and 13 deletions.
diff --git a/src/main/scala/com/twitter/querulous/config/Database.scala b/src/main/scala/com/twitter/querulous/config/Database.scala
@@ -46,10 +46,6 @@ class Database {
         apacheConfig.minEvictableIdle)
     ).getOrElse(new SingleConnectionDatabaseFactory)
 
-    if (stats ne NullStatsCollector) {
-      factory = new StatsCollectingDatabaseFactory(factory, stats)
-    }
-
     timeout.foreach { timeoutConfig =>
       factory = new TimingOutDatabaseFactory(factory,
         timeoutConfig.poolSize,
@@ -58,6 +54,10 @@ class Database {
         timeoutConfig.poolSize)
     }
 
+    if (stats ne NullStatsCollector) {
+      factory = new StatsCollectingDatabaseFactory(factory, stats)
+    }
+
     autoDisable.foreach { disable =>
       factory = new AutoDisablingDatabaseFactory(factory, disable.errorCount, disable.interval)
     }

diff --git a/src/main/scala/com/twitter/querulous/database/StatsCollectingDatabase.scala b/src/main/scala/com/twitter/querulous/database/StatsCollectingDatabase.scala
@@ -16,14 +16,26 @@ class StatsCollectingDatabase(database: Database, stats: StatsCollector)
   extends Database {
 
   override def open(): Connection = {
-    stats.time("database-open-timing") {
-      database.open()
+    stats.time("db-open-timing") {
+      try {
+        database.open()
+      } catch {
+        case e: SqlDatabaseTimeoutException =>
+          stats.incr("db-open-timeout-count", 1)
+          throw e
+      }
     }
   }
 
   override def close(connection: Connection) = {
-    stats.time("database-close-timing") {
-      database.close(connection)
+    stats.time("db-close-timing") {
+      try {
+        database.close(connection)
+      } catch {
+        case e: SqlDatabaseTimeoutException =>
+          stats.incr("db-close-timeout-count", 1)
+          throw e
+      }
     }
   }
 }
diff --git a/src/main/scala/com/twitter/querulous/query/StatsCollectingQuery.scala b/src/main/scala/com/twitter/querulous/query/StatsCollectingQuery.scala
@@ -15,7 +15,16 @@ class StatsCollectingQuery(query: Query, queryClass: QueryClass, stats: StatsCol
   override def delegate[A](f: => A) = {
     stats.incr("db-" + queryClass.name + "-count", 1)
     stats.time("db-" + queryClass.name + "-timing") {
-      stats.time("db-timing")(f)
+      stats.time("db-timing") {
+        try {
+          f
+        } catch {
+          case e: SqlQueryTimeoutException =>
+            stats.incr("db-query-timeout-count", 1)
+            stats.incr("db-query-" + queryClass.name + "-timeout-count", 1)
+            throw e
+        }
+      }
     }
   }
 }
diff --git a/src/test/scala/com/twitter/querulous/unit/StatsCollectingDatabaseSpec.scala b/src/test/scala/com/twitter/querulous/unit/StatsCollectingDatabaseSpec.scala
@@ -4,7 +4,7 @@ import scala.collection.mutable.Map
 import java.sql.Connection
 import org.specs.Specification
 import org.specs.mock.{ClassMocker, JMocker}
-import com.twitter.querulous.database.StatsCollectingDatabase
+import com.twitter.querulous.database.{SqlDatabaseTimeoutException, StatsCollectingDatabase}
 import com.twitter.querulous.test.{FakeStatsCollector, FakeDatabase}
 import com.twitter.util.Time
 import com.twitter.util.TimeConversions._
@@ -21,14 +21,29 @@ class StatsCollectingDatabaseSpec extends Specification with JMocker with ClassM
       "when closing" >> {
         Time.withCurrentTimeFrozen { time =>
           pool(s => time.advance(latency)).close(connection)
-          stats.times("database-close-timing") mustEqual latency.inMillis
+          stats.times("db-close-timing") mustEqual latency.inMillis
         }
       }
 
       "when opening" >> {
         Time.withCurrentTimeFrozen { time =>
           pool(s => time.advance(latency)).open()
-          stats.times("database-open-timing") mustEqual latency.inMillis
+          stats.times("db-open-timing") mustEqual latency.inMillis
+        }
+      }
+    }
+
+    "collect timeout stats" in {
+      val e = new SqlDatabaseTimeoutException("foo", 0.seconds)
+      "when closing" >> {
+        pool(s => throw e).close(connection) must throwA[SqlDatabaseTimeoutException]
+        stats.counts("db-close-timeout-count") mustEqual 1
+      }
+
+      "when opening" >> {
+        Time.withCurrentTimeFrozen { time =>
+          pool(s => throw e).open() must throwA[SqlDatabaseTimeoutException]
+          stats.counts("db-open-timeout-count") mustEqual 1
         }
       }
     }

diff --git a/src/test/scala/com/twitter/querulous/unit/StatsCollectingQuerySpec.scala b/src/test/scala/com/twitter/querulous/unit/StatsCollectingQuerySpec.scala
@@ -3,7 +3,7 @@ package com.twitter.querulous.unit
 import java.sql.ResultSet
 import org.specs.Specification
 import org.specs.mock.JMocker
-import com.twitter.querulous.query.{QueryClass, StatsCollectingQuery}
+import com.twitter.querulous.query.{QueryClass, SqlQueryTimeoutException, StatsCollectingQuery}
 import com.twitter.querulous.test.{FakeQuery, FakeStatsCollector}
 import com.twitter.util.Time
 import com.twitter.util.TimeConversions._
@@ -29,6 +29,20 @@ class StatsCollectingQuerySpec extends Specification with JMocker {
         stats.times("db-timing") mustEqual latency.inMillis
       }
     }
+
+    "collect timeout stats" in {
+      Time.withCurrentTimeFrozen { time =>
+        val stats = new FakeStatsCollector
+        val testQuery = new FakeQuery(List(mock[ResultSet]))
+        val statsCollectingQuery = new StatsCollectingQuery(testQuery, QueryClass.Select, stats)
+        val e = new SqlQueryTimeoutException(0.seconds)
+
+        statsCollectingQuery.select { _ => throw e } must throwA[SqlQueryTimeoutException]
+
+        stats.counts("db-query-timeout-count") mustEqual 1
+        stats.counts("db-query-" + QueryClass.Select.name + "-timeout-count") mustEqual 1
+      }
+    }
   }
 }