[MERGE] Extend -Dhpss to emit a count of HPSS calls to stderr

Andrew Bennetts andrew at canonical.com
Thu Sep 25 08:47:01 BST 2008

This patch extends the -Dhpss option to dump a count of HPSS calls to
stderr for each SmartClientMedium that was used.

Example output:

$ ./bzr revno bzr://localhost/branch -Dhpss
HPSS calls: 8 (<bzrlib.smart.medium.SmartTCPClientMedium object at 0x8565dac>)

I know that it's a bit unusual for a debug flag to cause output
somewhere other than the log file, but I think it's helpful in this
case.  I'm open changing this behaviour, or to making a new debug flag
for this separate to the existing -Dhpss, but so far I think I like it
this way.

Unfortunately I had to resort to using an atexit handler to reliably
report on each medium that was used.  Many medium instances are not
explicitly disconnected, and garbage-collection doesn't reliably collect
the instance before the end of the process (e.g. I saw this happen if
there was an error, such as a failure to connect to the remote host).

Although it uses an atexit function as a last resort, mostly the code
reports on each medium as soon as it is garbage collected.  This is
implemented using weakref callbacks, which avoids the downsides of using
__del__, and doesn't affect the object's lifetime.

If -Dhpss is not set then the impact of this patch is almost zero; one
quick check in an infrequently called constructor, and one extra
module-global variable is defined.

I'm not sure how best to test this.  If we always explicitly
disconnected our medium instances it'd probably be straightforward, but
we don't.  We rely on garbage collection, and sometimes we rely on the
process to end, to disconnect our medium instances.

The main drawback to this code is that when -Dhpss is used, it will keep
adding hook functions for each SmartClientMedium that is instantiated.
Those hook functions are never unregistered because there's no API to do
so.  It'd probably be good to fix that, but I don't think it's a big
deal for debugging code.

I have an alternative version of this patch (also attached, as a diff
relative to this change, I hope it doesn't confuse BB), that installs a
single hook function (rather than one per medium).  The hook function
then had to find the matching weakref for the medium being used, which
didn't seem as straightforward to me.  Also that approach conflicts more
with the test suite, which resets hooks after each test, but this hook
is only installed once per process.

Anyway, that's a lot of explanation for a fairly humble feature.  Enjoy!


-------------- next part --------------
A non-text attachment was scrubbed...
Name: hpss-call-count-3735.patch
Type: text/x-diff
Size: 9439 bytes
Desc: not available
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20080925/9417a620/attachment-0002.bin 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: alternative.patch
Type: text/x-diff
Size: 4109 bytes
Desc: not available
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20080925/9417a620/attachment-0003.bin 

More information about the bazaar mailing list