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

Make orphan processing interruptible #15644

Merged
merged 3 commits into from Apr 1, 2019

Conversation

@sipa
Copy link
Member

@sipa sipa commented Mar 22, 2019

As individual orphan transactions can be relatively expensive to handle, it's undesirable to process all of them (max 100) as soon as the parent becomes available, as it pegs the net processing the whole time.

Change this by interrupting orphan handling after every transactions, and continue in the next processing slot of the peer that gave us the parent - similar to how getdata processing works now. Messages from other peers arriving in the mean time are processed normally, but other messages from the peer that gave us the parent have to wait until all orphan processing is done.

@fanquake fanquake added the P2P label Mar 22, 2019
@DrahtBot
Copy link
Contributor

@DrahtBot DrahtBot commented Mar 22, 2019

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #15606 ([experimental] UTXO snapshots by jamesob)
  • #15437 (p2p: Remove BIP61 reject messages by MarcoFalke)
  • #15253 (Net: Consistently log outgoing INV messages by Empact)
  • #15141 (Rewrite DoS interface between validation and net_processing by sdaftuar)

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.

src/net_processing.cpp Outdated Show resolved Hide resolved
@sipa sipa force-pushed the 201903_interruptibleorphans branch from ebf3331 to 358a4f8 Mar 22, 2019
sipa added 3 commits Mar 23, 2019
This makes orphan processing work like handling getdata messages:
After every actual transaction validation attempt, interrupt
processing to deal with messages arriving from other peers.
@sipa sipa force-pushed the 201903_interruptibleorphans branch from 358a4f8 to 866c805 Mar 23, 2019
@gmaxwell
Copy link
Contributor

@gmaxwell gmaxwell commented Mar 25, 2019

ACK

@TheBlueMatt
Copy link
Contributor

@TheBlueMatt TheBlueMatt commented Mar 26, 2019

utACK

@sdaftuar
Copy link
Member

@sdaftuar sdaftuar commented Mar 28, 2019

ACK 866c805

@MarcoFalke
Copy link
Member

@MarcoFalke MarcoFalke commented Mar 28, 2019

utACK 866c805

@MarcoFalke MarcoFalke added this to the 0.18.0 milestone Mar 28, 2019
MarcoFalke added a commit to MarcoFalke/bitcoin-core that referenced this issue Mar 28, 2019
MarcoFalke added a commit to MarcoFalke/bitcoin-core that referenced this issue Mar 28, 2019
MarcoFalke added a commit to MarcoFalke/bitcoin-core that referenced this issue Mar 28, 2019
This makes orphan processing work like handling getdata messages:
After every actual transaction validation attempt, interrupt
processing to deal with messages arriving from other peers.

Github-Pull: bitcoin#15644
Rebased-From: 866c805
Copy link
Member

@promag promag left a comment

Refactor LGTM, some comments though.

src/net_processing.cpp Outdated Show resolved Hide resolved
src/net_processing.cpp Outdated Show resolved Hide resolved
src/net_processing.cpp Show resolved Hide resolved
@promag
Copy link
Member

@promag promag commented Mar 29, 2019

utACK 866c805. Verified refactor in 9453018 and moved code in 6e051f3. Not so sure about change in 866c805 just because I'm not familiar with net processing.

MarcoFalke added a commit to MarcoFalke/bitcoin-core that referenced this issue Apr 1, 2019
866c805 Interrupt orphan processing after every transaction (Pieter Wuille)
6e051f3 [MOVEONLY] Move processing of orphan queue to ProcessOrphanTx (Pieter Wuille)
9453018 Simplify orphan processing in preparation for interruptibility (Pieter Wuille)

Pull request description:

  As individual orphan transactions can be relatively expensive to handle, it's undesirable to process all of them (max 100) as soon as the parent becomes available, as it pegs the net processing the whole time.

  Change this by interrupting orphan handling after every transactions, and continue in the next processing slot of the peer that gave us the parent - similar to how getdata processing works now. Messages from other peers arriving in the mean time are processed normally, but other messages from the peer that gave us the parent have to wait until all orphan processing is done.

ACKs for commit 866c80:
  sdaftuar:
    ACK 866c805
  MarcoFalke:
    utACK 866c805
  promag:
    utACK 866c805. Verified refactor in 9453018 and moved code in 6e051f3. Not so sure about change in 866c805 just because I'm not familiar with net processing.

Tree-SHA512: d8e8a1ee5f2999446cdeb8fc9756ed9c24f3d5cd769a7774ec4c317fc8d463fdfceec88de97266f389b715a5dfcc2b0a3abaa573955ea451786cc43b870e8cde
@MarcoFalke MarcoFalke merged commit 866c805 into bitcoin:master Apr 1, 2019
2 checks passed
@fanquake
Copy link
Member

@fanquake fanquake commented Apr 2, 2019

Backported in #15691.

HashUnlimited pushed a commit to HashUnlimited/chaincoin that referenced this issue Apr 19, 2019
HashUnlimited pushed a commit to HashUnlimited/chaincoin that referenced this issue Apr 19, 2019
HashUnlimited pushed a commit to HashUnlimited/chaincoin that referenced this issue Apr 19, 2019
This makes orphan processing work like handling getdata messages:
After every actual transaction validation attempt, interrupt
processing to deal with messages arriving from other peers.

Github-Pull: bitcoin#15644
Rebased-From: 866c805
HashUnlimited pushed a commit to HashUnlimited/chaincoin that referenced this issue Aug 23, 2019
HashUnlimited pushed a commit to HashUnlimited/chaincoin that referenced this issue Aug 23, 2019
HashUnlimited pushed a commit to HashUnlimited/chaincoin that referenced this issue Aug 23, 2019
This makes orphan processing work like handling getdata messages:
After every actual transaction validation attempt, interrupt
processing to deal with messages arriving from other peers.

Github-Pull: bitcoin#15644
Rebased-From: 866c805
deadalnix pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this issue May 22, 2020
…uptibility

Summary:
bitcoin/bitcoin@9453018

---

Partial backport of Core [[bitcoin/bitcoin#15644 | PR15644]]

Test Plan:
  ninja check
  ./test/functional/test_runner.py --extended

Reviewers: #bitcoin_abc, deadalnix, nakihito

Reviewed By: #bitcoin_abc, deadalnix, nakihito

Subscribers: nakihito, deadalnix

Differential Revision: https://reviews.bitcoinabc.org/D6216
deadalnix pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this issue May 23, 2020
…sOrphanTx

Summary:
bitcoin/bitcoin@6e051f3

---

Depends on D6216

Partial backport of Core [[bitcoin/bitcoin#15644 | PR15644]]

Test Plan:
  ninja check

Reviewers: #bitcoin_abc, nakihito, deadalnix

Reviewed By: #bitcoin_abc, nakihito, deadalnix

Subscribers: deadalnix

Differential Revision: https://reviews.bitcoinabc.org/D6217
deadalnix pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this issue May 23, 2020
Summary:
This makes orphan processing work like handling getdata messages:
After every actual transaction validation attempt, interrupt
processing to deal with messages arriving from other peers.

---

Depends on D6217

Concludes backport of Core [[bitcoin/bitcoin#15644 | PR15644]]

Test Plan:
  ninja check-all

Reviewers: #bitcoin_abc, deadalnix

Reviewed By: #bitcoin_abc, deadalnix

Subscribers: deadalnix

Differential Revision: https://reviews.bitcoinabc.org/D6235
Munkybooty added a commit to Munkybooty/dash that referenced this issue Sep 27, 2021
866c805 Interrupt orphan processing after every transaction (Pieter Wuille)
6e051f3 [MOVEONLY] Move processing of orphan queue to ProcessOrphanTx (Pieter Wuille)
9453018 Simplify orphan processing in preparation for interruptibility (Pieter Wuille)

Pull request description:

  As individual orphan transactions can be relatively expensive to handle, it's undesirable to process all of them (max 100) as soon as the parent becomes available, as it pegs the net processing the whole time.

  Change this by interrupting orphan handling after every transactions, and continue in the next processing slot of the peer that gave us the parent - similar to how getdata processing works now. Messages from other peers arriving in the mean time are processed normally, but other messages from the peer that gave us the parent have to wait until all orphan processing is done.

ACKs for commit 866c80:
  sdaftuar:
    ACK 866c805
  MarcoFalke:
    utACK 866c805
  promag:
    utACK 866c805. Verified refactor in 9453018 and moved code in 6e051f3. Not so sure about change in 866c805 just because I'm not familiar with net processing.

Tree-SHA512: d8e8a1ee5f2999446cdeb8fc9756ed9c24f3d5cd769a7774ec4c317fc8d463fdfceec88de97266f389b715a5dfcc2b0a3abaa573955ea451786cc43b870e8cde
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

9 participants