Rev 3812: Improve behaviour of stop_searching_any. Fixes regression in in file:///home/pqm/archives/thelove/bzr/%2Btrunk/

Canonical.com Patch Queue Manager pqm at pqm.ubuntu.com
Thu Oct 30 23:53:46 GMT 2008


At file:///home/pqm/archives/thelove/bzr/%2Btrunk/

------------------------------------------------------------
revno: 3812
revision-id: pqm at pqm.ubuntu.com-20081030235342-3ttqulydt0irv350
parent: pqm at pqm.ubuntu.com-20081030145221-fog9qrw59bobguy0
parent: john at arbash-meinel.com-20081030152000-pzfowc2mapr41kut
committer: Canonical.com Patch Queue Manager <pqm at pqm.ubuntu.com>
branch nick: +trunk
timestamp: Thu 2008-10-30 23:53:42 +0000
message:
  Improve behaviour of stop_searching_any. Fixes regression in
  	_walk_to_common_revisions that causes unnecessary data to be
  	fetched. (Andrew Bennetts, John Arbash Meinel)
modified:
  bzrlib/graph.py                graph_walker.py-20070525030359-y852guab65d4wtn0-1
  bzrlib/remote.py               remote.py-20060720103555-yeeg2x51vn0rbtdp-1
  bzrlib/repository.py           rev_storage.py-20051111201905-119e9401e46257e3
  bzrlib/tests/test_graph.py     test_graph_walker.py-20070525030405-enq4r60hhi9xrujc-1
    ------------------------------------------------------------
    revno: 3808.1.4
    revision-id: john at arbash-meinel.com-20081030152000-pzfowc2mapr41kut
    parent: andrew.bennetts at canonical.com-20081030064227-t7d1zcgd4ieywssy
    committer: John Arbash Meinel <john at arbash-meinel.com>
    branch nick: stop_searching
    timestamp: Thu 2008-10-30 10:20:00 -0500
    message:
      make _walk_to_common responsible for stopping ancestors
      and assume that callers that call 'stop' late will handle ancestors,
      so don't bother testing what happens if they don't.
    modified:
      bzrlib/graph.py                graph_walker.py-20070525030359-y852guab65d4wtn0-1
      bzrlib/remote.py               remote.py-20060720103555-yeeg2x51vn0rbtdp-1
      bzrlib/repository.py           rev_storage.py-20051111201905-119e9401e46257e3
      bzrlib/tests/test_graph.py     test_graph_walker.py-20070525030405-enq4r60hhi9xrujc-1
    ------------------------------------------------------------
    revno: 3808.1.3
    revision-id: andrew.bennetts at canonical.com-20081030064227-t7d1zcgd4ieywssy
    parent: andrew.bennetts at canonical.com-20081030062957-5bg2nthfdlzylwjv
    committer: Andrew Bennetts <andrew.bennetts at canonical.com>
    branch nick: graph-stop
    timestamp: Thu 2008-10-30 17:42:27 +1100
    message:
      Add another test, this one currently failing.  Perhaps the current behaviour should be considered ok, rather than a failure?
    modified:
      bzrlib/tests/test_graph.py     test_graph_walker.py-20070525030405-enq4r60hhi9xrujc-1
    ------------------------------------------------------------
    revno: 3808.1.2
    revision-id: andrew.bennetts at canonical.com-20081030062957-5bg2nthfdlzylwjv
    parent: andrew.bennetts at canonical.com-20081030062236-nivsb8pvyxu1ry85
    committer: Andrew Bennetts <andrew.bennetts at canonical.com>
    branch nick: graph-stop
    timestamp: Thu 2008-10-30 17:29:57 +1100
    message:
      Improve docstring.
    modified:
      bzrlib/graph.py                graph_walker.py-20070525030359-y852guab65d4wtn0-1
    ------------------------------------------------------------
    revno: 3808.1.1
    revision-id: andrew.bennetts at canonical.com-20081030062236-nivsb8pvyxu1ry85
    parent: pqm at pqm.ubuntu.com-20081029232846-uni2g702k01jmgvh
    committer: Andrew Bennetts <andrew.bennetts at canonical.com>
    branch nick: graph-stop
    timestamp: Thu 2008-10-30 17:22:36 +1100
    message:
      Possible fix for bug in new _walk_to_common_revisions.
    modified:
      bzrlib/graph.py                graph_walker.py-20070525030359-y852guab65d4wtn0-1
      bzrlib/tests/test_graph.py     test_graph_walker.py-20070525030405-enq4r60hhi9xrujc-1
=== modified file 'bzrlib/graph.py'
--- a/bzrlib/graph.py	2008-07-16 18:14:23 +0000
+++ b/bzrlib/graph.py	2008-10-30 15:20:00 +0000
@@ -1331,7 +1331,13 @@
         Remove any of the specified revisions from the search list.
 
         None of the specified revisions are required to be present in the
-        search list.  In this case, the call is a no-op.
+        search list.
+
+        It is okay to call stop_searching_any() for revisions which were seen
+        in previous iterations. It is the callers responsibility to call
+        find_seen_ancestors() to make sure that current search tips that are
+        ancestors of those revisions are also stopped.  All explicitly stopped
+        revisions will be excluded from the search result's get_keys(), though.
         """
         # TODO: does this help performance?
         # if not revisions:
@@ -1368,6 +1374,7 @@
                     stop_parents.add(rev_id)
             self._next_query.difference_update(stop_parents)
         self._stopped_keys.update(stopped)
+        self._stopped_keys.update(revisions - set([revision.NULL_REVISION]))
         return stopped
 
     def start_searching(self, revisions):

=== modified file 'bzrlib/remote.py'
--- a/bzrlib/remote.py	2008-10-29 23:28:46 +0000
+++ b/bzrlib/remote.py	2008-10-30 15:20:00 +0000
@@ -973,7 +973,7 @@
         args = (path,) + tuple(keys)
         try:
             response = self._call_with_body_bytes_expecting_body(
-                verb, args, self._serialise_search_recipe(recipe))
+                verb, args, body)
         except errors.UnknownSmartMethod:
             # Server does not support this method, so get the whole graph.
             # Worse, we have to force a disconnection, because the server now

=== modified file 'bzrlib/repository.py'
--- a/bzrlib/repository.py	2008-10-27 06:15:46 +0000
+++ b/bzrlib/repository.py	2008-10-30 15:20:00 +0000
@@ -2439,11 +2439,14 @@
                     raise errors.NoSuchRevision(
                         self.source, ghosts_to_check.pop())
                 missing_revs.update(next_revs - have_revs)
-                searcher.stop_searching_any(have_revs)
+                # Because we may have walked past the original stop point, make
+                # sure everything is stopped
+                stop_revs = searcher.find_seen_ancestors(have_revs)
+                searcher.stop_searching_any(stop_revs)
             if searcher_exhausted:
                 break
         return searcher.get_result()
-   
+
     @deprecated_method(one_two)
     @needs_read_lock
     def missing_revision_ids(self, revision_id=None, find_ghosts=True):

=== modified file 'bzrlib/tests/test_graph.py'
--- a/bzrlib/tests/test_graph.py	2008-07-16 16:54:06 +0000
+++ b/bzrlib/tests/test_graph.py	2008-10-30 15:20:00 +0000
@@ -1082,6 +1082,33 @@
         search = graph._make_breadth_first_searcher(['head'])
         self.assertSeenAndResult(expected, search, search.next_with_ghosts)
 
+    def test_breadth_first_stop_searching_late(self):
+        # A client should be able to say 'stop node X' and have it excluded
+        # from the result even if X was seen in an older iteration of the
+        # search.
+        graph = self.make_graph({
+            'head':['middle'],
+            'middle':['child'],
+            'child':[NULL_REVISION],
+            NULL_REVISION:[],
+            })
+        search = graph._make_breadth_first_searcher(['head'])
+        expected = [
+            (set(['head']), (set(['head']), set(['middle']), 1),
+             ['head'], None, None),
+            (set(['head', 'middle']), (set(['head']), set(['child']), 2),
+             ['head', 'middle'], None, None),
+            # 'middle' came from the previous iteration, but we don't stop
+            # searching it until *after* advancing the searcher.
+            (set(['head', 'middle', 'child']),
+             (set(['head']), set(['middle', 'child']), 1),
+             ['head'], None, ['middle', 'child']),
+            ]
+        self.assertSeenAndResult(expected, search, search.next)
+        # using next_with_ghosts:
+        search = graph._make_breadth_first_searcher(['head'])
+        self.assertSeenAndResult(expected, search, search.next_with_ghosts)
+
     def test_breadth_first_get_result_ghosts_are_excluded(self):
         graph = self.make_graph({
             'head':['child', 'ghost'],




More information about the bazaar-commits mailing list