Ticket #1392: brianfollowingstats.txt

<warner> use the 30 hits from find src contrib misc -type f |grep -v pyc
         |xargs grep '\bget_stats\b'| grep -v 'def get_stats'
<warner> let's walk through them

<warner> 6 in stats.py  [16:29]
<warner> (I use \b so we'll match things like callRemote("get_stats") and
         x=blah.get_stats; x() , not that anyone does the latter)
<arc`> It looks like all the references without dot prefixes are either
       comments or definitions or string literals containing get_stats  [16:30]
<warner> yeah. the string literal form is what invokes
         RIStatsProvider.get_stats  [16:31]
<warner> it's in a callRemote() argument
<arc`> I'm not seeing those I'm seeing log.msg("foofooget_stats() -> foo")

<warner> now, 5 hits are in StatsProvider, which is the internal
         aggregate-all-the-producers object that provides RIStatsProvider
                                                                        [16:32]
<warner> oh, my \b probably skipped those
<warner> the \b matches word boundaries
<warner> so blahXYZ won't match \bXYZ

<warner> another hit is in stats.StatsGatherer, which is the service that does
         callRemote('get_stats') and retrieves stats through the
         RIStatsProvider.get_stats interface
<warner> that's the receiving end of the remote protocol
<warner> er, the initiating end, but it's where the stats wind up
<warner> from line 211, you can see that the stats get sent to self.got_stats
                                                                        [16:34]
<warner> which, from the NotImplementedError on line 227, you can tell is
         meant to be overridden by subclasses, so we need to look for 'def
         got_stats' in subclasses. Fortunately, all the subclasses are in this
         same file, right after StatsGatherer, so we don't have to look far
                                                                        [16:35]
<warner> StdOutStatsGatherer.got_stats just uses pprint() on them
<warner> and PickleStatsGatherer.got_stats pickles them  [16:36]
<warner> it turns out that misc/operations_helpers/munin/tahoe_stats reads
         that pickle
<warner> line 460 is where it extracts an individual statistic  [16:38]
<warner> and on line 461 it does 'if value is not None', so it can probably
         tolerate None
<warner> also, the PLUGINS table that makes up most of the file doesn't look
         at latencies at all  [16:39]
<warner> ok, so that covers stats.py
<warner> next up: web/check_results.py
ERC> 
<warner> another hit is in stats.StatsGatherer, which is the service that does
         callRemote('get_stats') and retrieves stats through the
         RIStatsProvider.get_stats interface
<warner> that's the receiving end of the remote protocol
<warner> er, the initiating end, but it's where the stats wind up
<warner> from line 211, you can see that the stats get sent to self.got_stats
                                                                        [16:34]
<warner> which, from the NotImplementedError on line 227, you can tell is
         meant to be overridden by subclasses, so we need to look for 'def
         got_stats' in subclasses. Fortunately, all the subclasses are in this
         same file, right after StatsGatherer, so we don't have to look far
                                                                        [16:35]
<warner> StdOutStatsGatherer.got_stats just uses pprint() on them
<warner> and PickleStatsGatherer.got_stats pickles them  [16:36]
<warner> it turns out that misc/operations_helpers/munin/tahoe_stats reads
         that pickle
<warner> line 460 is where it extracts an individual statistic  [16:38]
<warner> and on line 461 it does 'if value is not None', so it can probably
         tolerate None
<warner> also, the PLUGINS table that makes up most of the file doesn't look
         at latencies at all  [16:39]
<warner> ok, so that covers stats.py
<warner> next up: web/check_results.py
<warner> that calls the get_stats() that's really ICheckResults.get_stats
                                                                        [16:40]
* arc` yanking contents of channel for more leisurely review  [16:41]
<warner> root.py: lines 199 and 212. Both look at a few specific keys and
         ignore the rest, and don't look at latencies
<warner> next up: web/status.py
<warner> four hits  [16:42]
<warner> there's a self.helper.get_stats() in
         HelperStatus.data_helper_stats.. the helper doesn't have a storage
         server, so latencies shouldn't be there
<warner> same for the second hit in HelperStatus.render_JSON
<warner> third hit is Statistics.renderHTTP (the t=="json" clause): that looks
         like an external HTTP interface for fetching the data. JSON can
         handle None, but let's add "find external HTTP callers" to the list
         of things to investigate  [16:43]
<warner> fourth hit is Statistics.data_get_stats, which is used as input to
         the series of 'render_*' functions in the rest of that
         class. Fortunately, none of them look at latency, except render_raw()
         which just pprints() the whole thing, for which None is fine
                                                                        [16:44]
<warner> ok, that's all of web/status.py
ERC> 
                                                                        [16:40]
* arc` yanking contents of channel for more leisurely review  [16:41]
<warner> root.py: lines 199 and 212. Both look at a few specific keys and
         ignore the rest, and don't look at latencies
<warner> next up: web/status.py
<warner> four hits  [16:42]
<warner> there's a self.helper.get_stats() in
         HelperStatus.data_helper_stats.. the helper doesn't have a storage
         server, so latencies shouldn't be there
<warner> same for the second hit in HelperStatus.render_JSON
<warner> third hit is Statistics.renderHTTP (the t=="json" clause): that looks
         like an external HTTP interface for fetching the data. JSON can
         handle None, but let's add "find external HTTP callers" to the list
         of things to investigate  [16:43]
<warner> fourth hit is Statistics.data_get_stats, which is used as input to
         the series of 'render_*' functions in the rest of that
         class. Fortunately, none of them look at latency, except render_raw()
         which just pprints() the whole thing, for which None is fine
                                                                        [16:44]
<warner> ok, that's all of web/status.py
<warner> last one is web/storage.py  [16:45]
<warner> three hits
<warner> first is StorageStatus.render_JSON: the output of
         self.storage.get_stats() is included in the JSON response
<warner> so that's another "find external HTTP callers" item
<warner> second hit is a command  [16:46]
<warner> last hit is in StorageStatus.data_stats, which (looking at the
         associated storage_status.xhtml template for n:data="stats"
         attributes) is used by a couple of local render_* functions
                                                                        [16:47]
<warner> but, none of those touch latency
*** kevan_ (~kevan@host-134-71-248-56.allocated.csupomona.edu) has quit: Quit:
    leaving
ERC> OK, that was a bit faster than I could keep up with, .
* arc` yanking contents of channel for more leisurely review  [16:41]
<warner> root.py: lines 199 and 212. Both look at a few specific keys and
         ignore the rest, and don't look at latencies
<warner> next up: web/status.py
<warner> four hits  [16:42]
<warner> there's a self.helper.get_stats() in
         HelperStatus.data_helper_stats.. the helper doesn't have a storage
         server, so latencies shouldn't be there
<warner> same for the second hit in HelperStatus.render_JSON
<warner> third hit is Statistics.renderHTTP (the t=="json" clause): that looks
         like an external HTTP interface for fetching the data. JSON can
         handle None, but let's add "find external HTTP callers" to the list
         of things to investigate  [16:43]
<warner> fourth hit is Statistics.data_get_stats, which is used as input to
         the series of 'render_*' functions in the rest of that
         class. Fortunately, none of them look at latency, except render_raw()
         which just pprints() the whole thing, for which None is fine
                                                                        [16:44]
<warner> ok, that's all of web/status.py
<warner> last one is web/storage.py  [16:45]
<warner> three hits
<warner> first is StorageStatus.render_JSON: the output of
         self.storage.get_stats() is included in the JSON response
<warner> so that's another "find external HTTP callers" item
<warner> second hit is a command  [16:46]
<warner> last hit is in StorageStatus.data_stats, which (looking at the
         associated storage_status.xhtml template for n:data="stats"
         attributes) is used by a couple of local render_* functions
                                                                        [16:47]
<warner> but, none of those touch latency
*** kevan_ (~kevan@host-134-71-248-56.allocated.csupomona.edu) has quit: Quit:
    leaving
<warner> so, we're down to HTTP callers of /storage?t=json
         (StorageStatus.render_JSON) and of /statistics?t=json  [16:49]
<warner> where we might be concerned that those callers will mishandle a None
         (really a null, since it's JSON)  [16:50]
<warner> at that point, I think we can stop, because I'm pretty sure there's
         no code in tahoe that calls HTTP and parses JSON for stats.. all
         tahoe's stats-consuming code is either internal, or uses foolscap
                                                                        [16:51]
<zooko> I'm on Skype with Josh.
<zooko> He had a emacs UI error and didn't realize you were still typing. :-)
<warner> so, in summary: if the tests pass, then yes, changing the storage
         server latency-reporting code to return None for percentile buckets
         that are not yet full should be ok  [16:52]
<arc`> :-)  I was in the buffer I was copying into...
* arc` blushind
* arc` blushing
<warner> want me to pastebin anything?
<arc`> I think I have it all, going to review now.  Thanks!
ERC>