Skip to content

Add bloom filter usage statistics #7685

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
jonasschnelli wants to merge 3 commits into from
Closed

Add bloom filter usage statistics #7685

jonasschnelli wants to merge 3 commits into from

Conversation

@jonasschnelli
Copy link
Contributor

@jonasschnelli jonasschnelli commented Mar 14, 2016

It can be useful to know, how many blocks and how many times the mempool was filtered.

Bloom filtering is CPU intense and thus informations about its usage can help to identify sources of resource consumption.
Collecting statistics would be required to dynamic limit the amount of accepted bloom filtering.

This PR does only collect global statistics (total value and value per timeframe of currently 1h), though, I kept the design flexible. Extending this to per-node bloom-filter statistics is possible.

Also the time consumption in milliseconds for all filter processes will be collected, although, this value depends on your system workload and might not be very representative.

Statistics can be printed with the new RPC command getfilterstats.
Example:

/src/bitcoin-cli getfilterstats
{
  "current_timeframe": {
    "time_in_timeframe": 304,
    "blocks": {
      "filteredblocks_count": 1,
      "filteredblocks_amountofbytes": 989168,
      "filteredblocks_timeconsumption": 0
    },
    "mempool": {
      "mempool_filter_count": 4,
      "mempool_filter_amountoftxes": 1788,
      "mempool_filter_timeconsumption": 113
    }
  },
  "total": {
    "blocks": {
      "filteredblocks_count": 1,
      "filteredblocks_amountofbytes": 989168,
      "filteredblocks_timeconsumption": 0
    },
    "mempool": {
      "mempool_filter_count": 4,
      "mempool_filter_amountoftxes": 1788,
      "mempool_filter_timeconsumption": 1788
    }
  }
}

@laanwj
Copy link
Member

laanwj commented Mar 15, 2016

Yes, would be interesting to keep track of some statistics regarding resource usage.
Concept ACK

@paveljanik
Copy link
Contributor

paveljanik commented Mar 15, 2016

Concept ACK.

Is this pure coincidence?

      "mempool_filter_amountoftxes": 1788,
      "mempool_filter_timeconsumption": 1788

paveljanik
paveljanik reviewed Mar 15, 2016
View reviewed changes
src/net.h Outdated
Copy link
Contributor

@paveljanik paveljanik Mar 15, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove primitives/block.h from the network layer. Compute the size of the block in main.cpp and pass the size instead of the block itself into FilterStatsProcessBlock().

Copy link
Contributor Author

@jonasschnelli jonasschnelli Mar 16, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have also thought about this during the time of writing the code. I think its more flexible the way it is now. FilterStatsProcessBlock could also count transactions or the total amount of vins/vouts per block (which is basically where the filter needs to "go through").

Including block.h should be not a problem.

@jonasschnelli jonasschnelli changed the title [WIP] Add bloom filter usage statistics Add bloom filter usage statistics Mar 16, 2016
@jonasschnelli
Copy link
Contributor Author

jonasschnelli commented Mar 16, 2016

@paveljanik: there was a bug in a previous pushed commit that mixed-up timeconsumption with amountoftxes.

I have tested this PR now on a mainnet node and it seems to work good.
Example output:

{
  "current_timeframe": {
    "time_in_timeframe": 1262,
    "timeframe": 3600,
    "blocks": {
      "filteredblocks_count": 2,
      "filteredblocks_amountofbytes": 1923228,
      "filteredblocks_timeconsumption": 24
    },
    "mempool": {
      "mempool_filter_count": 2,
      "mempool_filter_amountoftxes": 55093,
      "mempool_filter_timeconsumption": 4475
    }
  },
  "total": {
    "blocks": {
      "filteredblocks_count": 24443,
      "filteredblocks_amountofbytes": 16788519596,
      "filteredblocks_timeconsumption": 99152
    },
    "mempool": {
      "mempool_filter_count": 5929,
      "mempool_filter_amountoftxes": 60586895,
      "mempool_filter_timeconsumption": 4801408
    }
  }
}

@paveljanik
Copy link
Contributor

paveljanik commented Apr 22, 2016

Rebase needed.

@jonasschnelli jonasschnelli force-pushed the 2016/03/bf_stats branch 2 times, most recently from 69cd4b2 to 73e8d32 Compare April 25, 2016 11:21
@jonasschnelli
Copy link
Contributor Author

jonasschnelli commented Apr 25, 2016

Rebased.
Added information about how many node requested filtering (count of filterload command).
Extended default time cycle to 24h.

@paveljanik
Copy link
Contributor

paveljanik commented Apr 27, 2016

Commit message 971e3e4bdf39ff2d13f3aee73e4b8086bc235120: cytle -> cycle
Commit message 73e8d32221669f3237a2d3dc71d5930effd73088: ilterload -> filterload

Needs tests and maybe some text to be included in the release notes (low prio IMO, because this is a low level stuff - or -help-debug?).

@sipa
Copy link
Member

sipa commented Jun 2, 2016

Needs rebase and nits addressed.

@fanquake
Copy link
Member

fanquake commented Jun 24, 2016

Still needs a rebase.

@laanwj laanwj added the Feature label Jun 27, 2016
@jonasschnelli jonasschnelli force-pushed the 2016/03/bf_stats branch 5 times, most recently from 304dcc1 to 8ceb743 Compare July 20, 2016 13:20
@jonasschnelli
Copy link
Contributor Author

jonasschnelli commented Jul 20, 2016

Rebased.
Needs tests (@MacroFalke?)?

@sipa
Copy link
Member

sipa commented Aug 25, 2016

Needs rebase.

@jonasschnelli
Copy link
Contributor Author

jonasschnelli commented Aug 26, 2016

Rebased. This is something that could be integrated into the general stats collector approach (#8501).

@jonasschnelli jonasschnelli mentioned this pull request Aug 26, 2016
Closed
@jonasschnelli
Copy link
Contributor Author

jonasschnelli commented Dec 6, 2016

Closing because of inactivity.

@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

No reviews

Assignees

No one assigned

Labels

Feature P2P

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@jonasschnelli @laanwj @paveljanik @sipa @fanquake