Evict orphans which are included or precluded by accepted blocks. #8179
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| map<COutPoint, set<uint256> >::iterator itByOutPoint = mapOrphanTransactionsByOutPoint.find(tx.vin[j].prevout); | ||
| if (itByOutPoint != mapOrphanTransactionsByOutPoint.end()) { | ||
| for (set<uint256>::iterator mi = itByOutPoint->second.begin(); mi != itByOutPoint->second.end(); ++mi) { | ||
| const uint256& orphanHash = *mi; |
There was a problem hiding this comment.
can you fix the whitespace here
|
ACK 70b5f60c761f56e0e84583e77d20a81db3fe1424 modulo whitespace nit |
|
So, I'm not super happy with the behavior-- the issue is that it removes included or conflicted orphans, but if those orphans themselves have orphaned children, those won't get removed. The behavior I'm seeing in testing is that it initially removes many transactions but over time seems to remove fewer or fewer, and I think it's because the orphanmap gets full of double-orphans that were conflicted. I'm not super keen on the performance implications of recursively eliminating there. |
|
@gmaxwell Agreed this behavior is not ideal, but this is an improvement. |
| @@ -94,6 +94,7 @@ struct COrphanTx { | |||
| }; | |||
| map<uint256, COrphanTx> mapOrphanTransactions GUARDED_BY(cs_main); | |||
| map<uint256, set<uint256> > mapOrphanTransactionsByPrev GUARDED_BY(cs_main); | |||
There was a problem hiding this comment.
It would be more efficient to store map<uint256, COrphanTx>::iterators instead of uint256's.
|
@gmaxwell Here is a commit that switches the mapOrphansByPrev entirely to be by-COutPoint: https://github.com/sipa/bitcoin/commits/reworkorphans |
always grow to its maximum size. This does not go so far as to attempt to connect orphans made connectable by a new block. Keeping the orphan map less full helps improve the reliability of relaying chains of transactions.
| if (nNextSweep <= nNow) { | ||
| // Sweep out expired orphan pool entries: | ||
| int nErased = 0; | ||
| int64_t nMinExpTime = nNow + 15 * 60; |
There was a problem hiding this comment.
Can you turn this into a named constant?
|
utACK with nit |
This prevents higher order orphans and other junk from holding positions in the orphan map. Parents delayed twenty minutes are more are unlikely to ever arrive. The freed space will improve the orphan matching success rate for other transactions.
…ted. An orphan whos parents were rejected is never going to connect, so there is little utility in keeping it. Orphans also helpfully tell us what we're missing, so go ahead and treat it as INVed.
Although this increases node memory usage in the worst case by perhaps 30MB, the current behavior causes severe issues with dependent tx relay.
|
@sipa Nit picked. |
laanwj
added a commit
that referenced
this pull request
Jun 20, 2016
…d blocks. 54326a6 Increase maximum orphan size to 100,000 bytes. (Gregory Maxwell) 8c99d1b Treat orphans as implicit inv for parents, discard when parents rejected. (Gregory Maxwell) 11cc143 Adds an expiration time for orphan tx. (Gregory Maxwell) db0ffe8 This eliminates the primary leak that causes the orphan map to always grow to its maximum size. (Gregory Maxwell) 1b0bcc5 Track orphan by prev COutPoint rather than prev hash (Pieter Wuille)
|
utACK 54326a6 |
This was referenced Jun 22, 2016
Closed
codablock
pushed a commit
to codablock/dash
that referenced
this pull request
Dec 28, 2017
…accepted blocks. 54326a6 Increase maximum orphan size to 100,000 bytes. (Gregory Maxwell) 8c99d1b Treat orphans as implicit inv for parents, discard when parents rejected. (Gregory Maxwell) 11cc143 Adds an expiration time for orphan tx. (Gregory Maxwell) db0ffe8 This eliminates the primary leak that causes the orphan map to always grow to its maximum size. (Gregory Maxwell) 1b0bcc5 Track orphan by prev COutPoint rather than prev hash (Pieter Wuille)
andvgal
pushed a commit
to energicryptocurrency/gen2-energi
that referenced
this pull request
Jan 6, 2019
…accepted blocks. 54326a6 Increase maximum orphan size to 100,000 bytes. (Gregory Maxwell) 8c99d1b Treat orphans as implicit inv for parents, discard when parents rejected. (Gregory Maxwell) 11cc143 Adds an expiration time for orphan tx. (Gregory Maxwell) db0ffe8 This eliminates the primary leak that causes the orphan map to always grow to its maximum size. (Gregory Maxwell) 1b0bcc5 Track orphan by prev COutPoint rather than prev hash (Pieter Wuille)
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This eliminates the primary leak that causes the orphan map to
always grow to its maximum size.
This does not go so far as to attempt to connect orphans made
connectable by a new block.
Keeping the orphan map less full helps improve the reliability
of relaying chains of transactions.