-
Notifications
You must be signed in to change notification settings - Fork 37.7k
tracing: utxocache tracepoints follow up for #22902 #23907
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
tracing: utxocache tracepoints follow up for #22902 #23907
Conversation
Previously, the `utxocache:flush` tracepoint was in the wrong scope and reached every time `CChainState::FlushStateToDisk` was called, even when there was no flushing of the cache. The tracepoint is now properly scoped and will be reached during a full flush. Inside the scope, the `fDoFullFlush` value will always be `true`, so it doesn't need to be logged separately. Hence, it's dropped from the tracepoint arguments.
Concept ACK |
…ilds with USDT tracepoints 6200fbf build: rename --enable-ebpf to --enable-usdt (0xb10c) e158a2a build: add systemtap's sys/sdt.h as depends (0xb10c) Pull request description: There has been light conceptual agreement on including the Userspace, Statically Defined Tracing tracepoints in Bitcoin Core release builds. This, for example, enables user to hook into production deployments, if they need to. Binaries don't have to be switched out. This is possible because we don't do [expensive computations](https://github.com/bitcoin/bitcoin/blob/master/doc/tracing.md#no-expensive-computations-for-tracepoints) only needed for the tracepoints. The tracepoints are NOPs when not used. Systemtap's `sys/sdt.h` header is required to build Bitcoin Core with USDT support. The header file defines the `DTRACE_PROBE` macros used in [`src/util/trace.h`](https://github.com/bitcoin/bitcoin/blob/master/src/util/trace.h). This PR adds Systemtap 4.5 (May 2021) as dependency. GUIX builds for Linux hosts now include the tracepoints. Closes #23297. ACKs for top commit: fanquake: ACK 6200fbf - tested enabling / disabling and with/without SDT from depends. We can follow up with #23819, #23907 and #23296, and if any serious issues arise before feature freeze, it is easy for us to flip depends such that USDT becomes opt-in, rather than opt-out, and thus, releases would be tracepoint free. Tree-SHA512: 0263f44892bf8450e8a593e4de7a498243687f8d81269e1c3283fa8354922c7cf93fddef4b92cf5192d33798424aa5812e03e68ef8de31af078a32dd34021382
…GUIX builds with USDT tracepoints 6200fbf build: rename --enable-ebpf to --enable-usdt (0xb10c) e158a2a build: add systemtap's sys/sdt.h as depends (0xb10c) Pull request description: There has been light conceptual agreement on including the Userspace, Statically Defined Tracing tracepoints in Bitcoin Core release builds. This, for example, enables user to hook into production deployments, if they need to. Binaries don't have to be switched out. This is possible because we don't do [expensive computations](https://github.com/bitcoin/bitcoin/blob/master/doc/tracing.md#no-expensive-computations-for-tracepoints) only needed for the tracepoints. The tracepoints are NOPs when not used. Systemtap's `sys/sdt.h` header is required to build Bitcoin Core with USDT support. The header file defines the `DTRACE_PROBE` macros used in [`src/util/trace.h`](https://github.com/bitcoin/bitcoin/blob/master/src/util/trace.h). This PR adds Systemtap 4.5 (May 2021) as dependency. GUIX builds for Linux hosts now include the tracepoints. Closes bitcoin#23297. ACKs for top commit: fanquake: ACK 6200fbf - tested enabling / disabling and with/without SDT from depends. We can follow up with bitcoin#23819, bitcoin#23907 and bitcoin#23296, and if any serious issues arise before feature freeze, it is easy for us to flip depends such that USDT becomes opt-in, rather than opt-out, and thus, releases would be tracepoint free. Tree-SHA512: 0263f44892bf8450e8a593e4de7a498243687f8d81269e1c3283fa8354922c7cf93fddef4b92cf5192d33798424aa5812e03e68ef8de31af078a32dd34021382
Added a bitcoind.observer dashboard based on the flush and add/spent/uncache tracepoints to observe their behavior a bit: https://bitcoind.observer/d/beTN4K27z/utxo-cache (this is currently running a pruned IBD and the UTXO cache is flushed for pruning before it reaches it's maximum capacity -- see #20827) |
|
Could add a note in the |
Okay, will update the docs 👍 |
Feel free to squash this commit 0xB10C@287351a documenting this #23907 (comment) into the I've cherry picked the commits from #24358 (tracepoint tests) onto this PR (currently |
- mention 'Lost X events' workaround - clarify flush tracepoint docs - fix typo in tracepoint context - clarify flush for prune The documentation and examples for the `fFlushForPrune` argument of the utxocache flush tracepoint weren't clear without looking at the code. See these comments: bitcoin#22902 (comment) - doc: note that there can be temporary UTXO caches Bitcoin Core uses temporary clones of it's _main_ UTXO cache in some places. The utxocache:add and :spent tracepoints are triggered when temporary caches are changed too. This is documented.
261db41
to
799968e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
…coin#22902 799968e tracing: misc follow-ups to 22902 (0xb10c) 36a6584 tracing: correctly scope utxocache:flush tracepoint (Arnab Sen) Pull request description: This PR is a follow-up to the [bitcoin#22902](bitcoin#22902). Previously, the tracepoint `utxocache:flush` was called, even when it was not flushing. So, the tracepoint is now scoped to write only when coins cache to disk. ACKs for top commit: 0xB10C: ACK 799968e Tree-SHA512: ebb096cbf991c551c81e4339821f10d9768c14cf3d8cb14d0ad851acff5980962228a1c746914c6aba3bdb27e8be53b33349c41efe8bab5542f639916e437b5f
Summary: ``` The tracepoints are further documented in docs/tracing.md and the usage is shown via the two newly added example scripts in contrib/tracing/. ``` Backport of [[bitcoin/bitcoin#22902 | core#22902]]. Backport of [[bitcoin/bitcoin#23907 | core#23907]]. Notes: - The amount is passed as string because our type can't be converted to int - CTxOut and CCoin accessors are used where applicable - I removed the casts that are both useless and even mixing stding and posix types Test Plan: Build with tracing, run bitcoind, then: sudo ../contrib/tracing/log_utxocache_flush.py ./src/bitcoind sudo bpftrace ../contrib/tracing/log_utxos.bt Reviewers: #bitcoin_abc, PiRK Reviewed By: #bitcoin_abc, PiRK Subscribers: PiRK Differential Revision: https://reviews.bitcoinabc.org/D12715
This PR is a follow-up to the #22902.
Previously, the tracepoint
utxocache:flush
was called, even when it was not flushing. So, the tracepoint is now scoped to write only when coins cache to disk.