Launchpad itself

Merge lp:~mwhudson/launchpad/log-noop-puller-runs into lp:launchpad

  1. log-noop-puller-runs
  2. Merge into devel
Proposed by Michael Hudson-Doyle
Status: Merged
Approved by: Michael Hudson-Doyle
Approved revision: not available
Merged at revision: not available
Proposed branch: lp:~mwhudson/launchpad/log-noop-puller-runs
Merge into: lp:launchpad
Diff against target: 185 lines (+36/-26)
4 files modified
lib/lp/codehosting/puller/scheduler.py (+11/-7)
lib/lp/codehosting/puller/tests/test_scheduler.py (+11/-10)
lib/lp/codehosting/puller/tests/test_worker.py (+3/-3)
lib/lp/codehosting/puller/worker.py (+11/-6)
To merge this branch: bzr merge lp:~mwhudson/launchpad/log-noop-puller-runs
Reviewer Review Type Date Requested Status
Jonathan Lange (community) Approve
Review via email: mp+16460@code.launchpad.net

Commit message

Log when the puller runs but does not transfer any revisions

To post a comment you must log in.
Revision history for this message
Michael Hudson-Doyle (mwhudson) wrote :

Hi there,

It is a fact that roughly half of the runs of the branch puller are from import branches. It is a fairly confident supposition of mine that most of these pulls do not actually move any revisions around and that we should stop the import system requesting mirrors when no revisions were imported. However, as Aaron pointed out this morning, it's pretty easy to log whether a run of the puller transferred any revisions so we can upgrade this supposition to a fact and that's what this branch does.

The net effect of the changes are that the "successfully mirrored" lines in the log now look like this:

2009-12-22 00:34:07 INFO Successfully mirrored HOSTED branch 77 lp-hosted:///~mark/gnome-terminal/trunk to lp-mirrored:///~mark/gnome-terminal/trunk to from rev null: to <email address hidden> (non-trivial)

Or this:

2009-12-22 00:34:28 INFO Successfully mirrored HOSTED branch 77 lp-hosted:///~mark/gnome-terminal/trunk to lp-mirrored:///~mark/gnome-terminal/trunk to from rev <email address hidden> to <email address hidden> (noop)

I think this will allow simple but sufficient grep based analysis of the puller log files.

Cheers,
mwh

Revision history for this message
Jonathan Lange (jml) wrote :

Looks very straightforward.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/codehosting/puller/scheduler.py'
2--- lib/lp/codehosting/puller/scheduler.py 2009-11-21 01:47:27 +0000
3+++ lib/lp/codehosting/puller/scheduler.py 2009-12-22 00:41:17 +0000
4@@ -236,10 +236,10 @@
5 def do_mirrorDeferred(self):
6 self.reported_mirror_finished = True
7
8- def do_mirrorSucceeded(self, latest_revision):
9+ def do_mirrorSucceeded(self, revid_before, revid_after):
10 def mirrorSucceeded():
11 d = defer.maybeDeferred(
12- self.listener.mirrorSucceeded, latest_revision)
13+ self.listener.mirrorSucceeded, revid_before, revid_after)
14 d.addCallback(self.reportMirrorFinished)
15 return d
16 self.runNotification(mirrorSucceeded)
17@@ -374,13 +374,17 @@
18 return self.branch_puller_endpoint.callRemote(
19 'mirrorFailed', self.branch_id, reason)
20
21- def mirrorSucceeded(self, revision_id):
22+ def mirrorSucceeded(self, revid_before, revid_after):
23+ if revid_before == revid_after:
24+ was_noop = 'noop'
25+ else:
26+ was_noop = 'non-trivial'
27 self.logger.info(
28- 'Successfully mirrored branch %d %s to %s to rev %s',
29- self.branch_id, self.source_url, self.destination_url,
30- revision_id)
31+ 'Successfully mirrored %s branch %d %s to %s to from rev %s to %s'
32+ ' (%s)', self.branch_type_name, self.branch_id, self.source_url,
33+ self.destination_url, revid_before, revid_after, was_noop)
34 return self.branch_puller_endpoint.callRemote(
35- 'mirrorComplete', self.branch_id, revision_id)
36+ 'mirrorComplete', self.branch_id, revid_after)
37
38 def log(self, message):
39 self.logger.info('From worker: %s', message)
40
41=== modified file 'lib/lp/codehosting/puller/tests/test_scheduler.py'
42--- lib/lp/codehosting/puller/tests/test_scheduler.py 2009-11-22 20:29:33 +0000
43+++ lib/lp/codehosting/puller/tests/test_scheduler.py 2009-12-22 00:41:17 +0000
44@@ -224,8 +224,8 @@
45 def startMirroring(self):
46 self.calls.append('startMirroring')
47
48- def mirrorSucceeded(self, last_revision):
49- self.calls.append(('mirrorSucceeded', last_revision))
50+ def mirrorSucceeded(self, revid_before, revid_after):
51+ self.calls.append(('mirrorSucceeded', revid_before, revid_after))
52
53 def mirrorFailed(self, message, oops):
54 self.calls.append(('mirrorFailed', message, oops))
55@@ -262,8 +262,9 @@
56 """Receiving a mirrorSucceeded message notifies the listener."""
57 self.protocol.do_startMirroring()
58 self.listener.calls = []
59- self.protocol.do_mirrorSucceeded('1234')
60- self.assertEqual([('mirrorSucceeded', '1234')], self.listener.calls)
61+ self.protocol.do_mirrorSucceeded('rev1', 'rev2')
62+ self.assertEqual(
63+ [('mirrorSucceeded', 'rev1', 'rev2')], self.listener.calls)
64 self.assertProtocolSuccess()
65
66 def test_mirrorDeferred(self):
67@@ -320,7 +321,7 @@
68 """
69 self.protocol.do_startMirroring()
70 self.clock.advance(config.supermirror.worker_timeout - 1)
71- self.protocol.do_mirrorSucceeded('rev1')
72+ self.protocol.do_mirrorSucceeded('rev1', 'rev2')
73 self.clock.advance(2)
74 return self.assertFailure(
75 self.termination_deferred, error.TimeoutError)
76@@ -476,18 +477,18 @@
77 return deferred.addCallback(checkSetStackedOn)
78
79 def test_mirrorComplete(self):
80- arbitrary_revision_id = 'rev1'
81+ arbitrary_revision_ids = ('rev1', 'rev2')
82 deferred = defer.maybeDeferred(self.eventHandler.startMirroring)
83
84- def mirrorSucceeded(ignored):
85+ def mirrorSucceeded(*ignored):
86 self.status_client.calls = []
87- return self.eventHandler.mirrorSucceeded(arbitrary_revision_id)
88+ return self.eventHandler.mirrorSucceeded(*arbitrary_revision_ids)
89 deferred.addCallback(mirrorSucceeded)
90
91 def checkMirrorCompleted(ignored):
92 self.assertEqual(
93 [('mirrorComplete', self.arbitrary_branch_id,
94- arbitrary_revision_id)],
95+ arbitrary_revision_ids[1])],
96 self.status_client.calls)
97 return deferred.addCallback(checkMirrorCompleted)
98
99@@ -797,7 +798,7 @@
100
101 check_lock_id_script = """
102 branch.lock_write()
103- protocol.mirrorSucceeded('b')
104+ protocol.mirrorSucceeded('a', 'b')
105 protocol.sendEvent(
106 'lock_id', branch.control_files._lock.peek()['user'])
107 sys.stdout.flush()
108
109=== modified file 'lib/lp/codehosting/puller/tests/test_worker.py'
110--- lib/lp/codehosting/puller/tests/test_worker.py 2009-11-21 00:28:10 +0000
111+++ lib/lp/codehosting/puller/tests/test_worker.py 2009-12-22 00:41:17 +0000
112@@ -681,11 +681,11 @@
113 self.assertSentNetstrings(['startMirroring', '0'])
114
115 def test_mirrorSucceeded(self):
116- # Calling 'mirrorSucceeded' sends the revno and 'mirrorSucceeded'.
117+ # Calling 'mirrorSucceeded' sends the revids and 'mirrorSucceeded'.
118 self.protocol.startMirroring()
119 self.resetBuffers()
120- self.protocol.mirrorSucceeded(1234)
121- self.assertSentNetstrings(['mirrorSucceeded', '1', '1234'])
122+ self.protocol.mirrorSucceeded('rev1', 'rev2')
123+ self.assertSentNetstrings(['mirrorSucceeded', '2', 'rev1', 'rev2'])
124
125 def test_mirrorFailed(self):
126 # Calling 'mirrorFailed' sends the error message.
127
128=== modified file 'lib/lp/codehosting/puller/worker.py'
129--- lib/lp/codehosting/puller/worker.py 2009-09-18 00:08:40 +0000
130+++ lib/lp/codehosting/puller/worker.py 2009-12-22 00:41:17 +0000
131@@ -105,8 +105,8 @@
132 # success or failure.
133 self.sendEvent('mirrorDeferred')
134
135- def mirrorSucceeded(self, last_revision):
136- self.sendEvent('mirrorSucceeded', last_revision)
137+ def mirrorSucceeded(self, revid_before, revid_after):
138+ self.sendEvent('mirrorSucceeded', revid_before, revid_after)
139
140 def mirrorFailed(self, message, oops_id):
141 self.sendEvent('mirrorFailed', message, oops_id)
142@@ -322,10 +322,11 @@
143 # over from previous puller worker runs. We will block on other
144 # locks and fail if they are not broken before the timeout expires
145 # (currently 5 minutes).
146+ revid_before = branch.last_revision()
147 if branch.get_physical_lock_status():
148 branch.break_lock()
149 self.updateBranch(source_branch, branch)
150- return branch
151+ return branch, revid_before
152
153
154 class PullerWorker:
155@@ -411,6 +412,10 @@
156 reporting protocol -- a "naked mirror", if you will. This is
157 particularly useful for tests that want to mirror a branch and be
158 informed immediately of any errors.
159+
160+ :return: ``(branch, revid_before)``, where ``branch`` is the
161+ destination branch and ``revid_before`` was the tip revision
162+ *before* the mirroring process ran.
163 """
164 # Avoid circular import
165 from lp.codehosting.vfs import get_puller_server
166@@ -429,7 +434,7 @@
167 """
168 self.protocol.startMirroring()
169 try:
170- dest_branch = self.mirrorWithoutChecks()
171+ dest_branch, revid_before = self.mirrorWithoutChecks()
172 # add further encountered errors from the production runs here
173 # ------ HERE ---------
174 #
175@@ -500,8 +505,8 @@
176 raise
177
178 else:
179- last_rev = dest_branch.last_revision()
180- self.protocol.mirrorSucceeded(last_rev)
181+ revid_after = dest_branch.last_revision()
182+ self.protocol.mirrorSucceeded(revid_before, revid_after)
183
184 def __eq__(self, other):
185 return self.source == other.source and self.dest == other.dest