This PR adds a statistics collector class which aims to collect various types of statistics up to the configurable maximum memory target. At the moment, only mempool statistics will be collected.

Motivation

Adding more statistics and visualization to the GUI would leverage its usage. To do so, we need stats that are collected even when the visualization is not visible (example: the GUI network graph will only draw data when it's visible which is kinda unusable)

How it works

This PR adds a simple stats manager that polls stats over a repetitive CScheduler task.

The samples are not guaranteed to be const-time. Each sample contains a time delta to the last one (uint16_t).

API

• -statsenable default disabled
• -statsmaxmemorytarget 10MB default maximal memory target to use for statistics.

• RPC: getmempoolstats

[
  {
    "percision_interval": 2,
    "time_from": 1494252401,
    "samples": [
      [
        11, 
        1, 
        1008, 
        0
      ], ....
   }...
]

Features

-> CScheduler driven sample collecting (current interval 2000ms) -> Relevant mempool data (size, memory requirement, minfee) gets written to an atomic cache (no additional locking required) -> Multiple precision levels (currently three, 2s, 60s, 1800s) -> Memory target that will calculate how many samples do fit in the given target -> Sample do only have a 2byte time-delta to the last sample, allowing to save some memory -> Flexible design, adding more data-points should be simple (bandwidth, utxo-stats, etc.).

Hi Jonas,

I understand the concern in your description for the addMempoolSample() stat bookkeeping designed to be as lightweight as possible in the critical execution path. However, I have a few (perhaps under-informed, neophyte) questions which would help me understand the design considerations better:

1. the comment in rpc_stats.cpp hints that the overhead of the JSON string generation is best optimized to be this 'flat' encoding as opposed to some encoding like:

{ "fieldNames" : ["delta_in_secs", "tx_count", "dynamic_mem_usage", "min_fee_per_k"], "samples" : [[val1, val2, val3, val4], [val1, val2, val3, val4], ] }

Is the 'flat' encoding strictly needed? or is there some other concern with outputting a slightly more convenient format than 'flat'?

2. It appears possible to set the maximum memory target very hight such that many, many samples are collected and the overhead of the computation mempoolGetValuesInRange() inside the lock might become onerus (assuming I am correctly understanding how the lock works and the implications of holding it too long). Have you considered taking a copy of 'mempoolStats' in a way that lets you return the lock earlier, and doing the dataset computation outside the lock? (Is that even currently possible under the current execution model?)

Cheers,

Isle

@isle2983 Welcome to github. Thanks for your feedback and your nitpicks. I really appreciate this and i'll process them during the next hours.

For your questions/inputs:

1. My idea with the JSON flat output was to bypass the JSON encoding/decoding.[val1, val2, val3, val4], [val1, val2, val3, val4], should also work. I just though a single string would result in faster encoding and decoding performance. But your approach seems to be the better choice, although not sure if we want to use UniValue for encoding or just appending strings... maybe we should start with the first and use a more optimized encoding if the JSON overhead is a problem.

2. Yes. That's a good point. Copying the samples vector could result in a memory peak when using large amount of -maxmemorytarget.

I have been playing around making my own changes off these commits (isle2983:getmempoolstats). Mostly to just to get some hands on with the code and try to get my C++ up to par.

But anyway, I made the rpc output of the samples full JSON:

{
  "enabled": true,
  "maxmemorytarget": 10485760,
  "currentusage": 1734416,
  "time_from": 1471573271,
  "time_to": 1471657376,
  "sampleCount": 27131,
  "sampleFieldNames": [
    "timeDelta", 
    "txCount", 
    "dynMemUsage", 
    "minFeePerK"
  ],
  "samples": [
    [
      0, 
      1, 
      1728, 
      0
    ], 
    [
      4, 
      11, 
      15232, 
      0
    ], 
    ...
    (snip)
    ]
}

The JSON 'pretty' print through bitcoin-cli is definitely unwieldy. However, the computational overhead in doing the wrangling doesn't seem so bad.

The 1.7MM of stat data is from collecting just overnight. With that data, I can pull it off the node, parse and convert the JSON into CSV with a python script and plot it in gnuplot in under a second.

$ time myJunk/plotTxCount.sh 

real    0m0.966s
user    0m0.460s
sys     0m0.128s

Not sure what the comparable is with the qt gui stuff branch that is running, but this doesn't seem too bad on the face of it.

Also, if getting this info from the node to the UI quickly is a concern, perhaps a more dense, binary-like format is worth considering i.e:

{"stats_blob":"8b292cf....."}

One could imagine it being more efficient than even the 'flat' format, depending on the sophistication.

Thanks @isle2983 for the testing, benchmarks and improvements. I have switched to the proposed array format for the samples (rather then the flat structure). A more performant binary format (inside of the JSON format) would be a hack. More performance would probably be possible over ZMQ.. but its currently a push only channel.

I also though again about copy the samples hash before filtering it. I came to the conclusion that it's not worth generating a memory peak (by a copy of the whole samples vector) in order to allow a faster release of the LOCK. The filtering should be very fast because it only compares some uint32 and does construct a new vector with a from-/to-iterators (should also preform fast).

Needed rebase.

Rebased.

Assigning "would be nice to have" for 0.14 per meeting today.

Just saw @gmaxwell's comment on #8550 (which I completely agree with) and it reminded to look at that PR and this one. Sorry for not getting involved sooner, but I really like the idea. Unfortunately I can think of many many stats (dozens and dozens) that we might want to collect, both to potentially show to users in whiz-bangy gui's and also would be useful for developers and businesses trying to understand unusual behavior on the network.

If we envision that there might be 1 KB of different stats data, then maybe rather than just saving sample data points and trimming when they hit memory usage, we should be smart about saving it along different time frames. For instance we could have second, minute, hour, day sampling intervals and we could save 1000 points or more on each and still have quite reasonable memory usage, but they could be auto trimmed.. so if you wanted to look at data from hours ago, you couldn't look at it on the second time frame...

@morcos: thanks for the comment. Yes. I completely agree. I think this is a first start and the current design allows features like you mentioned. I once started with interpolating values instead of just trimming the back, you could in theory just reduce the "density" of the sample and interpolate the in-between values (to a point where this could make sense). But yes, adding more stats probably require individual limits and trim-behaviours.

@jonasschnelli Well I guess what I was thinking was that one general framework might fit all stats. You log it with whatever frequency you want. And it's stored in up to 4 different histories (by second, minute, hour, day) and each of those is trimmed to some limit (say 1000 or 2000 data points each). Is there any type of stat that such a general framework might not work well with?

@morcos: the original idea I was trying to follow was to not collect in a fixed frequency. A) To avoid locking a thread for just collecting stats samples. B) To not collect over and over the same value if it was unchanged. Take the traffic report as an example. If you like to collect stats of all peers traffic segmented into all available p2p commands, then you would probably "loose" plenty of memory by storing samples with identical values.

I had the idea of recording samples in the most restrained way possible. Collect lock free and only if values have changes; collect the according timestamp. If you want to retrieve data with a fixed frequency/step-size, interpolate.

But not sure if this is stupid.

Rebased (main split)

c0af3664360e26333f8e16f20b0e21efa6ce2f1a has unresolved conflicts :(

@luke-jr All checks have passed ;-)

@paveljanik They're removed in the subsequent commit.

Yes (I noticed that), this is why we should take Travis' results as a help only.

@jonasschnelli whatever happened to this plan?

09:19:49 < jonasschnelli> I think you convinced me to do the 1000s 1000m 1000h 1000d approach.
09:19:59 < jonasschnelli> maybe the 1000 is configurable.
09:20:04 < morcos> doesn't matter to me how we do it... i think a delta version coudl be just as good
09:20:14 < morcos> and you could just be smart about trimming the delta list or something
09:20:28 < morcos> yes, 1000 should be configurable i thik... actually maybe isn't enough for a default
09:20:45 < jonasschnelli> also, I liked the configurability of the buffer in MB.
09:20:52 < jonasschnelli> That's what you probably care about.
09:20:56 < jonasschnelli> Not the 1000
09:20:59 < morcos> 1000 secs is just 16 minutes...  you would not want to have to only have 16 data points
09:21:12 < jonasschnelli> You would say, I reserve 300MB for stats.
09:21:37 < jonasschnelli> Right... just en 
09:21:45 < jonasschnelli> just as an example
09:22:14 < jonasschnelli> So,.. you convinced me for high frequency recent and low frequency long time horizon,...
09:22:21 < morcos> ok cool...   any approach that automatically keeps both recent fine-grained and long time horizon bigger step, is fine with me

Concept ACK

Needs rebase / reply to @morcos' review comment

Completely rewrote this PR.

Current features are -> CScheduler driven sample collecting (current interval 2000ms) -> Relevant mempool data (size, memory requirement, minfee) gets written to an atomic cache (no additional locking required) -> Multiple precision levels (currently three, 2s, 60s, 1800s) -> Memory target that will calculate how many samples do fit in the given target -> Sample do only have a 2byte time-delta to the last sample, allowing to save some memory -> Flexible design, adding more data-points should be simple (bandwidth, utxo-stats, etc.).

ping @morcos

Started looking at this PR, left a few comments.

Addressed @ryanofsky points.

Fixed @paveljanik points and nits.

Few more comments

Rebased on my repo.

git checkout stats_rpc && git fetch git://github.com/luke-jr/bitcoin stats_rpc && git reset --hard FETCH_HEAD && git push ...

Rebased (rebased @luke-jr's version).

I think a table would be more human readable. To also make it easier for other software to parse, you could make the sample interval an argument and add the timestamp to each sample:

bitcoin-cli getmempoolstats 2
timestamp  | tx_count | dynamic_mem_usage | min_fee_per_k  
------------------------------------------------------------
1515419437 | 1042     | 1797344           | 0       
1515419439 | 1050     | 1837344           | 0       
...

You could add another argument for the output format (CSV, JSON, etc).

Multiple precision intervals seems redundant, or at least something that could be added later, if users want long term charts without storing too much detail.

<!--5fd3d806e98f4a0ca80977bb178665a0-->There hasn't been much activity lately and the patch still needs rebase, so I am closing this for now. Please let me know when you want to continue working on this, so the pull request can be re-opened.