diff --git a/src/main/scala/com/twitter/querulous/config/Database.scala b/src/main/scala/com/twitter/querulous/config/Database.scala index e3e74a8..cd8c650 100644 --- 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 index 487d95c..d8af7ef 100644 --- 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 index d73d2ab..9f89e64 100644 --- 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 index 0ca65b2..a4ccab2 100644 --- 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 index f4b2e25..c8d4201 100644 --- 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 + } + } } }