Merge lp:~spiv/bzr/reconcile-cleanup-503878 into lp:bzr

Proposed by Andrew Bennetts
Status: Merged
Approved by: John A Meinel
Approved revision: not available
Merged at revision: not available
Proposed branch: lp:~spiv/bzr/reconcile-cleanup-503878
Merge into: lp:bzr
Diff against target: 120 lines (+37/-34)
2 files modified
NEWS (+3/-0)
bzrlib/reconcile.py (+34/-34)
To merge this branch: bzr merge lp:~spiv/bzr/reconcile-cleanup-503878
Reviewer Review Type Date Requested Status
John A Meinel Approve
Review via email: mp+16944@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Andrew Bennetts (spiv) wrote :

This replaces some try/finally blocks in reconcile.py with more robust add_cleanup calls using OperationWithCleanups. This fixes bug 503878 and probably a few unreported ones like it.

(This borrows the definition of OperationWithCleanups.run_simple from <https://code.edge.launchpad.net/~spiv/bzr/command-cleanup/+merge/16943>)

Revision history for this message
John A Meinel (jameinel) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'NEWS'
2--- NEWS 2010-01-13 23:26:10 +0000
3+++ NEWS 2010-01-13 23:33:13 +0000
4@@ -71,6 +71,9 @@
5 returns ``EINTR`` by calling ``PyErr_CheckSignals``. This affected the
6 optional ``_readdir_pyx`` extension. (Andrew Bennetts, #495023)
7
8+* Fix "Too many concurrent requests" in reconcile when network connection
9+ fails. (Andrew Bennetts, #503878)
10+
11 * Fixed a side effect mutation of ``RemoteBzrDirFormat._network_name``
12 that caused some tests to fail when run in a non-default order.
13 Probably no user impact. (Martin Pool, #504102)
14
15=== modified file 'bzrlib/reconcile.py'
16--- bzrlib/reconcile.py 2009-09-24 04:54:19 +0000
17+++ bzrlib/reconcile.py 2010-01-13 23:33:13 +0000
18@@ -27,12 +27,12 @@
19
20
21 from bzrlib import (
22+ cleanup,
23 errors,
24 ui,
25 repository,
26- repofmt,
27 )
28-from bzrlib.trace import mutter, note
29+from bzrlib.trace import mutter
30 from bzrlib.tsort import topo_sort
31 from bzrlib.versionedfile import AdapterFactory, FulltextContentFactory
32
33@@ -120,15 +120,16 @@
34 self.branch = a_branch
35
36 def reconcile(self):
37+ operation = cleanup.OperationWithCleanups(self._reconcile)
38+ self.add_cleanup = operation.add_cleanup
39+ operation.run_simple()
40+
41+ def _reconcile(self):
42 self.branch.lock_write()
43- try:
44- self.pb = ui.ui_factory.nested_progress_bar()
45- try:
46- self._reconcile_steps()
47- finally:
48- self.pb.finished()
49- finally:
50- self.branch.unlock()
51+ self.add_cleanup(self.branch.unlock)
52+ self.pb = ui.ui_factory.nested_progress_bar()
53+ self.add_cleanup(self.pb.finished)
54+ self._reconcile_steps()
55
56 def _reconcile_steps(self):
57 self._reconcile_revision_history()
58@@ -192,15 +193,16 @@
59 garbage_inventories: The number of inventory objects without revisions
60 that were garbage collected.
61 """
62+ operation = cleanup.OperationWithCleanups(self._reconcile)
63+ self.add_cleanup = operation.add_cleanup
64+ operation.run_simple()
65+
66+ def _reconcile(self):
67 self.repo.lock_write()
68- try:
69- self.pb = ui.ui_factory.nested_progress_bar()
70- try:
71- self._reconcile_steps()
72- finally:
73- self.pb.finished()
74- finally:
75- self.repo.unlock()
76+ self.add_cleanup(self.repo.unlock)
77+ self.pb = ui.ui_factory.nested_progress_bar()
78+ self.add_cleanup(self.pb.finished)
79+ self._reconcile_steps()
80
81 def _reconcile_steps(self):
82 """Perform the steps to reconcile this repository."""
83@@ -502,23 +504,21 @@
84 collection = self.repo._pack_collection
85 collection.ensure_loaded()
86 collection.lock_names()
87- try:
88- packs = collection.all_packs()
89- all_revisions = self.repo.all_revision_ids()
90- total_inventories = len(list(
91- collection.inventory_index.combined_index.iter_all_entries()))
92- if len(all_revisions):
93- new_pack = self.repo._reconcile_pack(collection, packs,
94- ".reconcile", all_revisions, self.pb)
95- if new_pack is not None:
96- self._discard_and_save(packs)
97- else:
98- # only make a new pack when there is data to copy.
99+ self.add_cleanup(collection._unlock_names)
100+ packs = collection.all_packs()
101+ all_revisions = self.repo.all_revision_ids()
102+ total_inventories = len(list(
103+ collection.inventory_index.combined_index.iter_all_entries()))
104+ if len(all_revisions):
105+ new_pack = self.repo._reconcile_pack(collection, packs,
106+ ".reconcile", all_revisions, self.pb)
107+ if new_pack is not None:
108 self._discard_and_save(packs)
109- self.garbage_inventories = total_inventories - len(list(
110- collection.inventory_index.combined_index.iter_all_entries()))
111- finally:
112- collection._unlock_names()
113+ else:
114+ # only make a new pack when there is data to copy.
115+ self._discard_and_save(packs)
116+ self.garbage_inventories = total_inventories - len(list(
117+ collection.inventory_index.combined_index.iter_all_entries()))
118
119 def _discard_and_save(self, packs):
120 """Discard some packs from the repository.