launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/launchpad:number-cruncher-transactions into launchpad:master.

Commit message:
Abort transaction after each number-cruncher loop

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/393546

We only read from the database, but if we don't end our transaction from time to time then we can end up with incorrect graphs and very long transactions.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:number-cruncher-transactions into launchpad:master.

diff --git a/lib/lp/services/statsd/numbercruncher.py b/lib/lp/services/statsd/numbercruncher.py
index 0a0552c..5955513 100644
--- a/lib/lp/services/statsd/numbercruncher.py
+++ b/lib/lp/services/statsd/numbercruncher.py
@@ -10,6 +10,7 @@ __all__ = ['NumberCruncher']
 
 import logging
 
+import transaction
 from twisted.application import service
 from twisted.internet import (
     defer,
@@ -79,6 +80,7 @@ class NumberCruncher(service.Service):
                 self.logger.debug("{}: {}".format(gauge_name, value[0]))
                 self.statsd_client.gauge(gauge_name, value[0])
         self.logger.debug("Build queue stats update complete.")
+        transaction.abort()
 
     def _updateBuilderCounts(self):
         """Update statsd with the builder statuses.
@@ -117,6 +119,7 @@ class NumberCruncher(service.Service):
         """Statistics that require builder knowledge to be updated."""
         self.builder_factory.update()
         self._updateBuilderCounts()
+        transaction.abort()
 
     def startService(self):
         self.logger.info("Starting number-cruncher service.")
diff --git a/lib/lp/services/statsd/tests/test_numbercruncher.py b/lib/lp/services/statsd/tests/test_numbercruncher.py
index c5a4ff6..ede74e1 100644
--- a/lib/lp/services/statsd/tests/test_numbercruncher.py
+++ b/lib/lp/services/statsd/tests/test_numbercruncher.py
@@ -20,6 +20,7 @@ from lp.buildmaster.enums import BuilderCleanStatus
 from lp.buildmaster.interactor import BuilderSlave
 from lp.buildmaster.interfaces.processor import IProcessorSet
 from lp.buildmaster.tests.mock_slaves import OkSlave
+from lp.services.database.isolation import is_transaction_in_progress
 from lp.services.statsd.numbercruncher import NumberCruncher
 from lp.services.statsd.tests import StatsMixin
 from lp.testing import TestCaseWithFactory
@@ -46,6 +47,7 @@ class TestNumberCruncher(StatsMixin, TestCaseWithFactory):
         manager.builder_factory.update()
         manager.updateBuilderStats()
 
+        self.assertFalse(is_transaction_in_progress())
         self.assertEqual(8, self.stats_client.gauge.call_count)
         for call in self.stats_client.mock.gauge.call_args_list:
             self.assertIn('386', call[0][0])
@@ -61,6 +63,7 @@ class TestNumberCruncher(StatsMixin, TestCaseWithFactory):
         manager.builder_factory.update()
         manager.updateBuilderStats()
 
+        self.assertFalse(is_transaction_in_progress())
         self.assertEqual(12, self.stats_client.gauge.call_count)
         i386_calls = [c for c in self.stats_client.gauge.call_args_list
                       if '386' in c[0][0]]
@@ -80,6 +83,7 @@ class TestNumberCruncher(StatsMixin, TestCaseWithFactory):
         manager.builder_factory.update()
         manager.updateBuilderStats()
 
+        self.assertFalse(is_transaction_in_progress())
         self.assertEqual(12, self.stats_client.gauge.call_count)
         calls = [c[0] for c in self.stats_client.gauge.call_args_list
                  if 'amd64' in c[0][0]]
@@ -113,6 +117,7 @@ class TestNumberCruncher(StatsMixin, TestCaseWithFactory):
         manager.builder_factory.update()
         manager.updateBuilderQueues()
 
+        self.assertFalse(is_transaction_in_progress())
         self.assertEqual(2, self.stats_client.gauge.call_count)
         self.assertThat(
             [x[0] for x in self.stats_client.gauge.call_args_list],