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

[Vincent Ladeuil]

>>>>> "Andrew" == Andrew Bennetts <andrew at canonical.com> writes:

<snip/>

    >> 
    >> Hmm, it doesn't scale as a debug mean... we always ask for
    >> .bzr.log in case of bugs.
    >> 
    >> '-Dhttp' uses stderr for pycurl because I couldn't find a way to
    >> use .bzr.log and it's quite a pain to explain to users: "Use
    >> '-Dhttp' and send .bzr.log. Err, wait, are you using pycurl ?
    >> Then please send stderr output too".

    Andrew> I mainly want to use this personally, to help me see
    Andrew> immediately how many RPCs a bzr invocation does.  I
    Andrew> don't think a total number of calls is much help for
    Andrew> diagnosing bug reports (especially if we get the
    Andrew> .bzr.log file anyway).  It is helpful for me doing
    Andrew> interactive testing, and also as a reminder of how
    Andrew> we're doing in my day-to-day use.

    Andrew> Reporting to stderr rather than a log file also makes
    Andrew> it a little easier for something like usertest to
    Andrew> capture this statistic.

No problem, using note() as John proposed, seems to address both :)

<snip/>

    Andrew> I don't think there's any great controversy with
    Andrew> using weakref.  It doesn't have much impact on the
    Andrew> rest of the code (much less than adding a __del__
    Andrew> method!).  The main issue I see is that code using it
    Andrew> should be careful to be safe in face of the reference
    Andrew> going away (i.e. checking for “ref() is None”).

Ok. And yes, that should be used with care.


<snip/>

    Andrew> The main drawback to this code is that when -Dhpss is
    Andrew> used, it will keep adding hook functions for each
    Andrew> SmartClientMedium that is instantiated.

vila ! He told you ! 'keep adding...for each' silly.

<snip/>
    >> 
    >> How come params.medium can be different from self.medium_ref() ?

    Andrew> Because one hook function is installed per client medium.

Not only, see below.

    >> The toy implementation below gives really different results for
    >> (in a clone of your branch):
    >> ./bzr missing lp:bzr
    >> 
    >> HPSS calls: 40 (<bzrlib.smart.medium.SmartSSHClientMedium object at 0x8ae21cc>)
    >> HPSS my calls: 413 (<bzrlib.smart.medium.SmartSSHClientMedium object at 0x8ae21cc>)
    >> 
    >> I probably missed something here... which doesn't allow me to
    >> vote :-/

    Andrew> Because your call_on_medium hook function is installed for each client
    Andrew> medium that is instantiated, it is getting called multiple times per
    Andrew> call.

Right, I realized that during dinner. But if that explained the
huge difference in counters, that doesn't explain all the mediums
with 0 calls. Or is it an artefact of the multiple inheritance ?
But in that case why ?

    Andrew> This will be less of an issue when RemoteTransport
    Andrew> stops inheriting from SmartClientMedium.

I'll try applying my python-2.6 patch.

    Andrew> If you want to see which is right, count the calls in
    Andrew> your ~/.bzr.log by hand :)

Hehe, mine was obviously wrong ! I read somewhere that asking
questions was a good way to use the review process so I fished
for two reasons: the answer had to be easier for you than me and
with a bit luck I could have pointed you to a weird detail.

I failed for the second part but look at the result I get now
(having fixed my toy implementation):

HPSS calls: 42 (<bzrlib.smart.medium.SmartSSHClientMedium object at 0x87dcc6c>)
Some mediums are still there:
HPSS my calls: 0 (<bzrlib.transport.remote.RemoteSSHTransport url=bzr+ssh://vila@bazaar.launchpad.net/%7Ebzr/bzr/trunk/.bzr/>)
HPSS my calls: 0 (<bzrlib.transport.remote.RemoteSSHTransport url=bzr+ssh://vila@bazaar.launchpad.net/%7Ebzr/bzr/trunk/.bzr/repository/indices/>)
HPSS my calls: 42 (<bzrlib.smart.medium.SmartSSHClientMedium object at 0x87dcc6c>)
HPSS my calls: 0 (<bzrlib.transport.remote.RemoteSSHTransport url=bzr+ssh://vila@bazaar.launchpad.net/%7Ebzr/bzr/trunk/>)
HPSS my calls: 0 (<bzrlib.transport.remote.RemoteSSHTransport url=bzr+ssh://vila@bazaar.launchpad.net/%7Ebzr/bzr/trunk/.bzr/repository/packs/>)
HPSS my calls: 0 (<bzrlib.transport.remote.RemoteSSHTransport url=bzr+ssh://vila@bazaar.launchpad.net/%7Ebzr/bzr/trunk/.bzr/branch/>)
HPSS my calls: 0 (<bzrlib.transport.remote.RemoteSSHTransport url=bzr+ssh://vila@bazaar.launchpad.net/%7Ebzr/bzr/trunk/.bzr/repository/upload/>)
HPSS my calls: 0 (<bzrlib.transport.remote.RemoteSSHTransport url=bzr+ssh://vila@bazaar.launchpad.net/%7Ebzr/bzr/trunk/.bzr/repository/>)

Why does these transport objects show here at all ? I can't
believe repr() is random...

And, no, 42 is not the answer :D

    Vincent

-------------- next part --------------
A non-text attachment was scrubbed...
Name: toy.patch
Type: text/x-diff
Size: 2705 bytes
Desc: not available
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20080926/49f5f189/attachment.bin