Add a new peer state tracking class to reduce cs_main contention. #16174

1. 
   TheBlueMatt commented at 11:37 am on June 9, 2019: member

   CNodeState was added for validation-state-tracking, and thus, logically, was protected by cs_main. However, as it has grown to include non-validation state (taking state from CNode), and as we’ve reduced cs_main usage for other unrelated things, CNodeState is left with lots of cs_main locking in net_processing.

   This starts the process of moving things out of cs_main (and into a new CPeerState) starting with nDoS and rejects.

   This also solves the lowest-hanging fruit by wiping out 10+ (!) cs_main locks that are trivial to remove! It further removes a cs_main lock which is taken on every ProcessMessages iteration, which makes a future rebase of #12934 much more effective by being able to move onto the next peer for processing (at least sometimes) while a block is validating.

2. 
   sipa commented at 11:40 am on June 9, 2019: member

   Big concept ACK
3. 
   practicalswift commented at 11:42 am on June 9, 2019: contributor

   Excellent!

   Concept ACK

4. 
   DrahtBot commented at 11:44 am on June 9, 2019: member

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

   Conflicts

   Reviewers, this pull request conflicts with the following ones:

   • #15921 (Tidy up ValidationState interface by jnewbery)
   • #15606 ([experimental] UTXO snapshots by jamesob)
   • #15206 (Immediately disconnect on invalid net message checksum by jonasschnelli)
   • #14046 (net: Refactor message parsing (CNetMessage), adds flexibility by jonasschnelli)
   • #14032 (Add p2p layer encryption with ECDH/ChaCha20Poly1305 by jonasschnelli)

   If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

5. Add a new peer state tracking class to reduce cs_main contention.

   CNodeState was added for validation-state-tracking, and thus,
   logically, was protected by cs_main. However, as it has grown to
   include non-validation state (taking state from CNode), and as
   we've reduced cs_main usage for other unrelated things, CNodeState
   is left with lots of cs_main locking in net_processing.
   
   In order to ease transition to something new, this adds only a
   dummy CPeerState which is held as a reference for the duration of
   message processing.

   54b3431c40
6. Move nDoS counters to CPeerState (and, thus, out of cs_main) ce89efbc54
7. TheBlueMatt force-pushed on Jun 9, 2019
8. 
   TheBlueMatt commented at 11:52 am on June 9, 2019: member

   Next step/further justification can be seen in the form of #16175.
9. DrahtBot added the label P2P on Jun 9, 2019
10. 
    naumenkogs commented at 5:36 pm on June 9, 2019: member

    Concept ACK
11. 
    TheBlueMatt commented at 7:20 pm on June 9, 2019: member

    Oops, this is actually not possible to do on its own. See #16175.
12. TheBlueMatt closed this on Jun 9, 2019

    ---

13. 
    ryanofsky commented at 4:26 pm on June 11, 2019: member

    Oops, this is actually not possible to do on its own. See #16175.

    Relevant comment is #16175 (comment)

14. DrahtBot locked this on Dec 16, 2021

Contributors
TheBlueMatt sipa practicalswift DrahtBot naumenkogs ryanofsky

Labels
P2P