Skip to content
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.

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

mempool: Add usdt event tracepoints and eBPF logging tool. #24935

Closed
wants to merge 5 commits into from

Conversation

russeree
Copy link
Contributor

Added usdt tracepoints to mempool additions and eviction functions

Using the bitcoin usdt tracepoints api, two mempool event tracepoints were added to txmempool.cpp. These tracepoints are active upon tx additions and evictions to the mempool. This is useful for logging the mempool contents for analysis. Better fee estimation models and custom TXID lookup scripts are some of the first uses that come to mind,

The included bpftrace script logs all mempool additions and removals in a .csv format that can be viewed in realtime or piped to a file for analysis.

NOTES Ref.

  • code included follows the USDT guidelines (no parsing or serialization) so near 0 effect on core performance.
  • bpftace script log_mempool_activity.bt is included and generates a .csv log output of all additions and evictions within the mempool along with the reason, size, time, txid, fee, and value of the TX
  • Documentation has been updated to reflect the new mempool context
  • Tracepoint API is very reliable in testing, Logged mempool TXs for over 24 hours and the csv output was parsed with 0 errors.
  • No env vars change needed to run bpftrace script

IMPOVEMENTS Ref,

  • The included bpftrace script improves on all existing examples by using a single printf call. This enables the script to log 100s of mempool evictions without dropping events and destroying the output. This means that unlike the current log_utxos.bt whos output is corrupted while using bpftrace default parameters. The include bpftrace script uses a variety of methods to improve performance and can be ran successfully with default bpftrace environment variables.

Copy link
Contributor

@0xB10C 0xB10C left a comment

Choose a reason for hiding this comment

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

In general, concept ACK on adding mempool tracepoints for additions and evictions. Thank you for picking this up.

However, I currently don't think this PR is up to Bitcoin Core standards. I've noted a few things below. Also, the commit message could be a bit more meaningful.

Additionally, I think there should be two distinct tracepoints (i.e. calls of the TRACEx macro). One for mempool additions and one for mempool evictions. This allows people e.g. only interested in mempool additions to only hook into a mempool:added tracepoint. No need to pass -1 as eviction reason on mempool addition and no need to pass an event string ("added", "evict").

It's not yet mentioned in the documentation, but we recently started adding tracepoint interface tests to make sure the tracepoints don't break. See e.g. #24358 and #24644 for examples for tests. We should add them for mempool tracepoints too.

src/txmempool.cpp Outdated Show resolved Hide resolved
src/txmempool.cpp Outdated Show resolved Hide resolved
src/txmempool.cpp Outdated Show resolved Hide resolved
src/txmempool.cpp Outdated Show resolved Hide resolved
$reasonStr = "EXPIRY"
}
//Store the segments
unroll(4){
Copy link
Contributor

Choose a reason for hiding this comment

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

As an alternative see

/* Prints each byte of the block hash as hex in big-endian (the block-explorer format) */
$p = $hash + 31;
unroll(32) {
$b = *(uint8*)$p;
printf("%02x", $b);
$p -= 1;
}

Copy link
Contributor Author

@russeree russeree Apr 21, 2022

Choose a reason for hiding this comment

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

The above unroll loop causes the printf statement to execute on each iteration. This causes dropped trace events that show up as visual artifacts or make for an usable output file. To get a perfect output there should be one printf statement per trace event.

This PRs bpftrace script may not be the best looking but it was implemented so that under the default bpftrace env. The output data will uncompromised and always product a usable result.

Edit: I believe the python script included largely obsoletes this .bt script.

doc/tracing.md Outdated Show resolved Hide resolved
@0xB10C
Copy link
Contributor

0xB10C commented Apr 21, 2022

IIRC @glozow had mempool tracepoints on her wishlist at some point too. Not sure if this also included tracepoints for mempool package acceptance. Probably out of scope for this PR, but ideas could be added to #20981 :)

@DrahtBot
Copy link
Contributor

DrahtBot commented Apr 21, 2022

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

No conflicts as of last run.

@russeree russeree force-pushed the master branch 4 times, most recently from 7821539 to 9ebbea2 Compare April 21, 2022 15:57
@russeree
Copy link
Contributor Author

russeree commented Apr 21, 2022

In general, concept ACK on adding mempool tracepoints for additions and evictions. Thank you for picking this up.

However, I currently don't think this PR is up to Bitcoin Core standards. I've noted a few things below. Also, the commit message could be a bit more meaningful.

Additionally, I think there should be two distinct tracepoints (i.e. calls of the TRACEx macro). One for mempool additions and one for mempool evictions. This allows people e.g. only interested in mempool additions to only hook into a mempool:added tracepoint. No need to pass -1 as eviction reason on mempool addition and no need to pass an event string ("added", "evict").

It's not yet mentioned in the documentation, but we recently started adding tracepoint interface tests to make sure the tracepoints don't break. See e.g. #24358 and #24644 for examples for tests. We should add them for mempool tracepoints too.

Awesome, yeah I will work on my commit messages.

There are implemented 2 distinct tracepoints mempool:added and mempool:evicted so that people can hook into the tracepoints they need for their projects.

I will start working on mempool tracepoint tests.

Edit:

I am working on building tests right now, in the short term though I have added an additional python script to log mempool events in a .csv format. It's quite cool! This python script is optimized though the use of the map.queue(), multithreading, and python queue objects to not drop events.

@russeree russeree deleted the branch bitcoin:master August 12, 2022 21:25
@russeree russeree closed this Aug 12, 2022
@russeree russeree deleted the master branch August 12, 2022 21:25
@russeree russeree restored the master branch August 12, 2022 22:08
@russeree russeree deleted the master branch August 12, 2022 22:10
@russeree
Copy link
Contributor Author

Sorry,
The branch was renamed and the PR was immediately closed.

@bitcoin bitcoin locked and limited conversation to collaborators Aug 12, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants