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

do not leak thread objects, and minor fixes mostly to comments #2

Merged
merged 9 commits into from
Feb 23, 2017

Conversation

gandrewstone
Copy link

No description provided.

Peter Tschipper and others added 9 commits January 31, 2017 10:49
- add 3 more script check queues
- Create semaphores to control and handle queue selection and throughput
- Update locking - we do not need to hold cs_main when checking inputs
- Python test for Parallel Validation

- Ability to Quit scriptcheck threads

When  4 blocks are running in parallel and a
new, smaller block shows up, we need to be able to interrupt the
script threads that are currently validating for a one of the  blocks
so that we can free up a script check queue for the new smaller  block.

- Update the nBlockSequenceId after the block has advanced the tip

This is important for Parallel Validation.  By decreasing the sequence id
we are indicating that this block represents the current pindexMostWork. This
prevents the losing block from having the pindexMostWork point to it rather
than to the winning block.

- Continously check for new blocks to connect

With Parallel Validation new blocks can be accepted while we are connecting at
tip therefore we need to check after we connect each block whether a potentially
longer more work chain now exists. If we don't do this then we can at times end
up with a temporarily unconnected block until the next block gets mined and
another attempt is made to connect the most work chain.

- Terminate any PV threads during a re-org

PV can only operate with blocks that will advance the current chaintip (fork1). Therefore,
if we are needing to re-org to another chain (fork 2)  then we have to kill any currently
running PV threads assoicated with the current chain tip on fork1. This is the
solution to the problem of having two forks being mined while there is an attack
block processing on fork1. If fork2 continues to be mined and eventually pulls
ahead of fork1 in proof of work, then a re-org to fork2 will be initiated causing
the PV threads on fork1 to be terminated and fork2 blocks then connected and fork2 then
becoming the chain active tip.

- Use Chain Work instead of nHeight to self terminate a PV thread

If the Chain Work has changed either positve or negative to where it
was when we started the PV thread then we will exit the thread.  Previously
we were using Chain Height, which worked fine, but this is more understantable
from a coding perspective and also we added the feature to check for when
Chain Work has decreased from the starting point which would indicate that
a re-org was underway.

-Move ZMQ notifications to ActivateBestChainStep

We must notify ZMQ after each tip is connected rather
than after ActivateBestChain otherwise we can miss a block
when there are several to connect together at once.

This particularly seems to help sync issues on regest where
we're mining many blocks all at the same time.

-Completely remove cs_main locks from checkinputs and sigs during PV

During the loop whre we check inputs and signatures we can completely
remove the cs_main locks by making sure to add an internal lock
cs_main lock to ChainWorkHasChanged().  This function rarely will
get invoked and only if the thread is about to exit so there is no
problem here of adding any additional overhead.

-Create SetLocks() used to set the locking order when returning from PV

If there is an error during PV then we have to make sure the locks
are set in the right order before returning.  cs_main must always
be locked when we return from ConnectBlock() as cs_main is recursive
but we must also ensure that the scriptlock is unlocked before
re-locking cs_main to avoid a potential deadlock.

By creating SetLocks() we can abstract away the setting of the locking
order and prevent any developer confusion or possible introduction of  errors
into the code if future changes are made in the ConnectBlock() function.

-Consolidate and simplify the sript check thread pool creation

In the past the code for creaating the script check threads and pools
was all over the place in several files but is now consolidated in
parallel.cpp and parallel.h.  Also is is much easier to make any
changes to the number of scriptcheckqueue's by just editing two
lines in  parallel.cpp.

-Add StartShutdown when closing QT Gui

StartShutdown() was never getting called which sets the shutdown
flag to true.  Generally this would be rare to cause a problem  but in PV
it can become an issue fairly easily if a user wants
to shutdown while many blocks are being connected during IBD.

-Correctly notify the UI of block tip changes

When many blocks are being connected during IBD it was
possible the the UI was not getting updated.  This is a Core
bug which happened rarely but in PV it can happen frequently.

-update IsChainNearlySyncdInit after each block is processed

-Only print out error message when state.Invalid() or state.IsError()

When activatebestchain fails during PV due to a competing block being
beaten, we don't want to print out an error because it really isn't an
error. The block validation was terminated but the block will be store
on disk for the future in the event of a re-org.

-Do not allow two thinblocks to be validating at the same time

- Update the UI if we are close to being syncd

Update the block Current Number of Blocks in the UI if we
are close to finishing our sync. This way when users turn off
their node for a while and turn it on they can see each block
being added as it finishes processing.

-Track whether the current block is acutally validating inputs

We need to do this in the event that two of the same block arrive
and have launched validation threads.  If this should happen then
one of those blocks will begin checking inputs before the other and
we can set a flag which indicates that fact.  Then when the other
block beginns checking inputs it can check to see if the same
block is also currently validatin and if so then exit.

-Use nChainWork to determine whether we have any work to do

When a newblock arrives we check whether the chaintip already matches
pindexMostWork.  However in PV, pindexMostWork may not necessarily point
to the chaintip, therefore we use pindexMostWork->nChainWork to determine
if an attempt should be made to connect this block.

Simplify ConnectBlock and remove unnecessary if(fParallel) statments

-Re-enable SENDHEADERS when Xthins is not on
In order to remove our dependence on cs_main further
and prepare the way for multithreaded transaction processing
we need a set of locks on the UTXO and the UTXO in memory
cache.

Furthermore, since the reads are fast (once the UTXO is in
memory) there is no value in using a Read/Write lock system
as that would entail more overhead and reduce performance.
However we do need recursive locks since serveral of the
methods can be invoked either by themselves or from within
other methods.
and a few comments removed and unnecessary code
fix some documentation, and change the pv init to not malloc
@ptschip ptschip merged commit a7b21f4 into ptschip:dev_parallel Feb 23, 2017
ptschip pushed a commit that referenced this pull request May 2, 2017
documentation and code changes for clarity
ptschip pushed a commit that referenced this pull request May 4, 2017
daf1285 Merge pull request #2 from jgarzik/master
d9e62d3 Merge pull request bitcoin#24 from MarcoFalke/Mf1608-cleanup
faf260f Rem unused vars and prefer prefix operator for non-primitive type
09a2693 Merge pull request bitcoin#22 from laanwj/2016_04_unicode
c74a04c Merge pull request bitcoin#23 from paveljanik/20160527_Wshadow
fceb4f8 Do not shadow variables

git-subtree-dir: src/univalue
git-subtree-split: daf1285
ptschip pushed a commit that referenced this pull request Jul 21, 2017
Add sunset clause to REQ-6-1 implementation.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants