Remove tally thread and make obsolete tally blocking. #934
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
iFoggz
reviewed
Feb 10, 2018
| printf("ConnectBlock[] : Researchers Reward results in deficit of %f for CPID %s with trust level of %f - (Submitted Research Subsidy %f vs calculated=%f) Hash: %s", | ||
| deficit, bb.cpid.c_str(), (double)strUntrustedHost.Accuracy, bb.ResearchSubsidy, | ||
| printf("ConnectBlock[] : Researchers Reward results in deficit of %f for CPID %s with trust level of %f - (Submitted Research Subsidy %f vs calculated=%f) Hash: %s", | ||
| deficit, bb.cpid.c_str(), (double)strUntrustedHost.Accuracy, bb.ResearchSubsidy, |
There was a problem hiding this comment.
strUntrustedHost.Accuracy is already double. could clean that up abit :)
iFoggz
approved these changes
Feb 10, 2018
denravonska
force-pushed
the
remove-retired-tally-thread
branch
from
e320f93
to
2cb8949
Compare
|
While I like the removal of tally thread, the change is too involved for me to immediately approve. |
denravonska
force-pushed
the
remove-retired-tally-thread
branch
from
2cb8949
to
564f919
Compare
|
@tomasbrod I've reverted some of the parts to make the diff smaller and to make it mimic the old behavior more. It's not entirely possible since it was a mix of busy waits and just flagging for tally. We absolutely must clean up the tally invocations in an upcoming leisure. It is way, way too scattered now. |
denravonska
commented
Feb 11, 2018
| @@ -4431,7 +4421,7 @@ void GridcoinServices() | |||
| { | |||
| if (fDebug) printf("SVC: set off Tally (v3 B) height %d\n",nBestHeight); | |||
| if (fDebug10) printf("#TIB# "); | |||
| bDoTally_retired = true; | |||
| TallyResearchAverages(pindexBest); | |||
There was a problem hiding this comment.
I am considering removing these two tallies. The previous calls were not busy waiting so the previous behavior was undefined.
Edit: Hmmm, maybe not.
This speeds up syncing and there is no need to tally old blocks.
denravonska
added a commit
that referenced
this pull request
Mar 1, 2018
Fixed: - Move context sensitive DPoR block checks to ConnectBlock, #912 (@tomasbrod). - Check incoming blocks for malformed DPoR signature, #912. - Corect tally height on init, #917 (@denravonska). - Prevent staking of a block with a failed signature, #948 (@Foggyx420). - Fix UI and RPC slowdown regression, #961 (@denravonska). - Fix Debian lint errors, #886, #885, #884, #883 (@caraka). - Fix fork issue due to research age calculation inconsistencies, #939 (@denravonska). - Fix crashes when tallying, #934 (@denravonska). - Revert reorganize of the chain trust becomes less than what it was, #957 (@tomasbrod). - Fix sync issues with incorrectly accepted v8 beacons, #979 (@tomasbrod). Changed: - Double check PoS kernel, #958 (@tomasbrod). - Don't tally until V9 to speed up syncing, #943 (@denravonska). - Double check proof of stake kernel, #958 (@tomasbrod).
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.
State: Testing
Remove the retired tally thread and make the tally blocking instead. This changes the old behavior but as long as it syncs up we should be good. I am syncing my dev station and a Ubuntu 14.04 docker to see if this solves Coinomi's problems.
Edit: This seems to slow down syncing a bit as tallies are blocking. As long as it syncs up fine we can live with that and optimize in the dev branch, most noticeably
UnpackBinarySuperblockand its string conversions.