Tail lock revisited #1328
Merged
Tail lock revisited #1328
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
ARR4N
reviewed
Sep 2, 2024
ARR4N
reviewed
Sep 2, 2024
| @@ -159,15 +155,19 @@ func (indexer *txIndexer) loop(chain *BlockChain) { | |||
| stop = make(chan struct{}) | |||
| done = make(chan struct{}) | |||
| indexer.chain.wg.Add(1) | |||
| go indexer.run(rawdb.ReadTxIndexTail(indexer.db), head.Block.NumberU64(), stop, done) | |||
| go func() { | |||
There was a problem hiding this comment.
If this must always be the same as the equivalent go routine then consider abstracting it into shared code, otherwise all LGTM.
ARR4N
approved these changes
Sep 2, 2024
…into tail-lock-revisited
ceyonur
added a commit
that referenced
this pull request
Sep 2, 2024
* all changes from v.13.2-x branch * squash initial changes * remove unneeded files * fix lint * gas estimation fix * accounts/keystore: fix test with sleep * rpc fixes * try with a smaller limit * logging * change the trim fn * remove main_test from rpc * txpool: remove unused "PendingFrom" * set local-txs-enabled in load tests * ready to format * format: remove upstream go-ethereum * format: rename packages as fork * Revert "format: remove avalanche header" This reverts commit 09d505bd5e906ac186643061437e665b116b5bf6. * fix logging * deadcode linting * more linting * fix mock * fix unused * expecteds * passing UTs * add comment * unused * resolve some open issues * undo script change * test resillience * fix compile * wip * test is fixed * fix expecteds * nits * fix merge * remove todos * try * try * try * docker stuff * fix * does this work * fix * hardcode for now * fix * hardcode for now * remove TODO comments * fix * make txindexer more similar to upstream * peer/network: move ctx check * enforce tips in miner * align nits with arr4n * remove use of deprecated type core.GenesisAccount * Revert "peer/network: move ctx check" This reverts commit 147da5f. * fix ctx check * fix ctx again * undo vm_test changes * reduce diffs with ARR4N's branch * update for enforcing gasTip * unbuffer chan * update to use commit(true) * use TestOnlyAllowDuplicateBlocks * Revert "update to use commit(true)" This reverts commit 71d949e. * overflow in state_transition err * add http body limit flag (#1327) * Tail lock revisited (#1328) * avoid modifying the pointer * improve readability * prevent race in tail reading * improve repairTxIndexTail readability * use shared func --------- Co-authored-by: Ceyhun Onur <[email protected]>
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.
Why this should be merged
There is a potential race between txindexer and state sync (ResetToStateSyncedBlock/repairTxIndexTail), where reading can occur before repairTxIndexTail (which can increment the tail) and txindexer can work on an outdate tail (less than actual).
How this works
Uses txindexer's
runfunction after grabbing lock and then do the actual db read for tail forrun.How this was tested
Not sure how to test this race
How is this documented
no need