tracing: utxocache tracepoints follow up for #22902 #23907

1. 
   arnabsen1729 commented at 2:03 pm on December 30, 2021: contributor

   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.

2. tracing: correctly scope utxocache:flush tracepoint

   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.

   36a6584703
3. DrahtBot added the label Scripts and tools on Dec 30, 2021
4. DrahtBot added the label Validation on Dec 30, 2021
5. 
   fanquake commented at 2:18 am on December 31, 2021: member

   cc @0xB10C @jb55 @laanwj
6. 
   0xB10C commented at 9:33 am on January 4, 2022: member

   Concept ACK. These are important fixups and followups to #22902. Will review and test this.

   The utxocache tracepoints still require manual testing before this PR is merged as #22902 didn’t receive throughout testing.

7. 
   jb55 commented at 8:06 pm on January 4, 2022: member

   Concept ACK
8. fanquake referenced this in commit 542e405a85 on Jan 10, 2022
9. sidhujag referenced this in commit 5ca60a8fdc on Jan 10, 2022
10. 
    0xB10C commented at 5:20 pm on January 17, 2022: member

    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)
11. fanquake added this to the milestone 23.0 on Jan 18, 2022
12. 
    0xB10C commented at 9:48 pm on January 25, 2022: member

    My site currently is currently only picking up flushes related to a prune event. I’ll investigate. Was an error on my side. The tracepoints are working as expected.
13. 
    0xB10C commented at 4:44 pm on February 9, 2022: member

    Could add a note in the utxoscache:uncache, utxoscache:add, and utxocache:spent tracepoint documentation that we don’t have only one ’the utxo cache’. We sometimes work with temporary UTXO caches. For example, in TestBlockValidity() after before mining or during mempool consistency checks (frequent on regtest). Tracepoints are triggered for temporary caches too. Noticed this while writing functional tests for the tracepoints.
14. 
    arnabsen1729 commented at 1:02 am on February 10, 2022: contributor

    Could add a note in the utxoscache:uncache, utxoscache:add, and utxocache:spent tracepoint documentation that we don’t have only one ’the utxo cache’. We sometimes work with temporary UTXO caches. For example, in TestBlockValidity() after mining or during mempool consistency checks (frequent on regtest). Tracepoints are triggered for temporary caches too. Noticed this while writing functional tests for the tracepoints.

    Okay, will update the docs :+1:

15. 
    0xB10C commented at 1:31 pm on February 17, 2022: member

    Feel free to squash this commit https://github.com/0xB10C/bitcoin/commit/287351a80193a24c84a245989ffa2403c15c78fe documenting this #23907 (comment) into the tracing: misc follow-ups to 22902 too (while copying the commit subject and description into the squash commit description).

    I’ve cherry picked the commits from #24358 (tracepoint tests) onto this PR (currently git cherry-pick 1e8aa02ec5cc2819c67ef40a7573c4b23a4c11cc..db3a05f50ee5d3cafa226882eade531a7ebff6b1; 0xB10C/bitcoin:2022-02-23907+tracepoint-tests) to run the tracepoint tests against this PR. Tests pass!

16. tracing: misc follow-ups to 22902

    - 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: https://github.com/bitcoin/bitcoin/pull/22902#issuecomment-987094612
    
    - 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.

    799968e8b3
17. arnabsen1729 force-pushed on Feb 18, 2022
18. 0xB10C approved
19. 
    0xB10C commented at 5:38 pm on February 19, 2022: member

    ACK 799968e8b38833dc7fd7b6d488a66a14580ef674

    Code and doc changes reviewed. Tested with #24358.

20. fanquake merged this on Feb 20, 2022
21. fanquake closed this on Feb 20, 2022

    ---

22. sidhujag referenced this in commit ce2594239b on Feb 20, 2022
23. Fabcien referenced this in commit e2cc0f927a on Dec 1, 2022
24. DrahtBot locked this on Feb 20, 2023

Contributors
arnabsen1729 fanquake 0xB10C jb55

Labels
Validation Scripts and tools

Milestone
23.0