Fix pool++limits #1767
Merged
Fix pool++limits #1767
Conversation
This file contains 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
andreibancioiu
requested review from
iulianpascalau,
AdoAdoAdo,
SebastianMarian and
LucianMincu
# Conflicts: # dataRetriever/factory/txpool/txPoolFactory_test.go # dataRetriever/txpool/shardedTxPool.go # dataRetriever/txpool/shardedTxPool_test.go # storage/txcache/config.go # storage/txcache/eviction_test.go # storage/txcache/txCache_test.go # storage/txcache/txListForSender_test.go
sasurobert
requested review from
sasurobert
and removed request for
iulianpascalau and
SebastianMarian
sasurobert
approved these changes
May 22, 2020
| SizeInBytes = 524288000 | ||
| SizeInBytesPerSender = 12288000 |
There was a problem hiding this comment.
only 12MBs per sender ?
There was a problem hiding this comment.
Actually, this is a much larger limit (x20) than we originally thought of (~600kb). I've used this (high) limit to ease testing via TxGen. After some rounds of testing, we can lower it to ~600kb & 1000 txs per sender. @AdoAdoAdo
There was a problem hiding this comment.
I was thinking of having bigger limits. In case of smart contracts, oracle's this limit is too small.
AdoAdoAdo
requested changes
May 25, 2020
AdoAdoAdo
approved these changes
May 25, 2020
sasurobert
reviewed
May 25, 2020
sasurobert
approved these changes
May 25, 2020
LucianMincu
approved these changes
May 26, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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 branch will contain fixes applied on top of
pool++limits.Original
pool++limitsPR: #1716Helpers to diff with original PR:
pool++limitscarve-outpool++limitscarve-outgit diff b94e466a827e78b7663f45ccfe1af0fa721680b2 -- dataRetriever/txpool/ pool++limits-fixed -- dataRetriever/txpool/git diff b94e466a827e78b7663f45ccfe1af0fa721680b2 -- storage/txcache/ pool++limits-fixed -- storage/txcache/Narrative of the bug on
pool++limitsBug behavior: missing transaction
0xabbais requested, resolved, but never added to cache, which results in it being requested again - functional deadlock.Two conflicting goroutines concur to add a transaction the same one -
hash=0xabba- in the cache. Say this is the first transaction of a sender (for example, due to a recent sender sweeping).txListBySenderMap.getOrAddListForSender, obtains a list objectID=0xABCDand inserts the transactionID=0xAAAAwithhash=0xabbatxListBySenderMap.getOrAddListForSender, obtains (a different) a list objectID=0xDCBAand inserts the transactionID=0xBBBBwithhash=0xabbaID=0xAAAA.ID=0xABCD[(ID=0xAAAA, hash=0xabba)]will be lost forever from memory.ID=0xDCBA[(ID=0xBBBB, hash=0xabba)]will be saved in the map by senders.hash=0xabba. This is found in the map by hashes, with pointerID=0xAAAA, and removed. ObjectID=0xAAAAis then searched in the list objectID=0xDCBA. The pointer comparison intxListForSender.findListElementWithTxfails to find the transaction withhash=0xabba. Not removed - bug,(ID=0xBBBB, hash=0xabba)stays there - bug.hash=0xabba. This is not found in the map by hashes (recently deleted, previous step), thus the transaction is requested from others. The transaction arrives.hash=0xabbacannot be inserted in the cache, as the de-duplication logic intxListForSender.findInsertionPlace()detects duplication.Fixes
Independent transaction removal in both structuresnot possible.bytes.Equal(hash of a, hash of b)to compare transactions.txListForSender.Can be tested using these log levels: