[MERGE/RFC][1.9] Better behaviour when calling stop_searching_any with a revision from an earlier iteration

Andrew Bennetts andrew.bennetts at canonical.com
Thu Oct 30 06:46:16 GMT 2008

My recent change to _walk_to_common_revisions (bzr.dev r3795) turns out to have
a bug, due to the way _BreadthFirstSearcher.stop_searching_any behaves.  We
either need to fix this or revert r3795 for 1.9, as this would be a pretty
serious regression.

The change iterates the searcher several times so that the client can batch
get_parent_map requests.  But for this to work correctly it needs to instruct
the searcher to stop searching for hits from the previous iterations.  I assumed
that stop_searching_any already handled this nicely, but it turns out it
doesn't do what I assumed.

In particular, it will leave the revisions in the final search result, as can
been seen with searcher.get_result().get_keys().  This causes fetch to transmit
revisions that have already been seen, which is obviously quite wasteful.

This patch attempts to address that.  I've added a note to the docstring to be
explicit about what does happen (the revision will be excluded from the final
result's keys) and what doesn't (revisions found via that revision will still
continue to be searched unless they are stopped as well).  This perhaps isn't
perfect, but it is adequate for the way _walk_to_common_revisions uses it (when
it finally does check a batch, it will stop all revisions found in that batch,
which should stop the searcher chasing unnecessary parts of the graph in the
next batch).

The graph code is pretty complex, so I would definitely appreciate more eyeballs
and brains applied to this.  I'm pretty sure it'll give the right result for
get_keys() on the result, but it doesn't give what you might expect for
get_recipe().  Maybe that's ok?

I've added tests for this behaviour that I hope are adequately clear.  The
second one fails, due to the returned recipe not being what you'd intuitively
expect (which is what the test currently expects).  Perhaps this is acceptable
though.  I would appreciate your thoughts.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: graph-stop-3811.patch
Type: text/x-diff
Size: 8711 bytes
Desc: not available
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20081030/70c0e16e/attachment.bin 

More information about the bazaar mailing list