Restore MemoryPool transactions from saved consensus context. #575

jsolman · 2019-01-25T02:54:51Z

This causes the saved transactions from the ConsensusContext to be sent to the Blockchain before the LocalNode is started. Also, I included some clean-ups for saving and restoring the consensus context.

If the node saves the consensus context after sending commit. It should upon restoring it still have the transactions in its memory pool to be able to send to other nodes. With the consensus recovery mechanism it can recover other nodes and should be able to give them the transactions in the case it is recovering their node state.

In a worst case scenario where only 1 consensus node saved commit, and all other consensus nodes had to start over from scratch from a chain file, when they were synced, the one remaining node that had its consensus context saved in the commit state would be able to recover all the other nodes including the primary to the view in which the commit was saved, and allow the primary to create the block.

…to correct view on restart).

erikzhang · 2019-01-25T03:13:33Z

If you let a consensus node start on the view it left off, you'd better clear the ConsensusContext.Transactions. Because the memory pool has changed and may contains conflicting transactions.

I think the best way is to not handle the view changing with recovery log and leave it to the message replay mechanism.

jsolman · 2019-01-25T03:23:50Z

How about I save and restore the memory pool? I was planning to make that change soon anyway. Actually, I guess that doesn’t work well since it will only be saved on a clean shutdown. What if we add the ones back to the be memory pool that are saved in the consensus context?

jsolman · 2019-01-25T03:25:22Z

@erikzhang if you want to leave it to message replay system that is fine too. Are you working on that PR now?

erikzhang · 2019-01-25T03:27:22Z

I will start the work for message replay system next week.

jsolman · 2019-01-25T03:28:54Z

Ok, no problem. I’ll make one other change here that loads the transactions into the memory pool from the consensus context. You can decide later whether to take or leave this.

vncoelho · 2019-01-25T03:43:20Z

I will check with careful, @jsolman.
But I prefer to merge every little piece in the Improved_DBFT now with carefully, because we already have a different template for doing the job (which we can surely be benefited by in the future).
Let's wait for Erik's insights on the code and then we keep checking carefully for ensuring a template that will proportionate a long term evolution smooth and in an well biased evolutionary direction.

…the Store.

jsolman · 2019-01-25T03:53:31Z

One more change coming to load the pool initially with transactions in the ConsensusContext on start-up.

jsolman · 2019-01-25T04:01:20Z

@erikzhang Have a look, the MemoryPool now starts with the transactions from the saved ConsensusContext.

vncoelho

Sounds good to me @jsolman, I prefer to let @erikzhang decide about, if necessary, when and how to merge.

I am worrying more about the regeneration during the operation and not very much when hardware fails, but this PR looks like as an additional useful mechanism.
A critical point of the Regeneration is when we just lose messages and reach another view different than those who are committed.

jsolman · 2019-01-27T19:56:41Z

@vncoelho
These changes are mainly to speed up consensus in the case CN nodes are restarted for some reason (crash or otherwise) after having changed view.

vncoelho

Sounds import in case of a quick restart.

neo/NeoSystem.cs

neo/Persistence/LevelDB/Prefixes.cs

…kchain subsystem.

jsolman · 2019-02-19T20:04:20Z

But the only problem is that it increases the coupling between mempool and consensus. Because mempool reads the contents of the serialized ConsensusContext directly from the store. This is not a good design.

This is addressed now. It reads the transactions form the serialized ConsensusContext just before starting the LocalNode and tells the Blockchain about them.

neo/Persistence/Store.cs

jsolman · 2019-02-20T20:30:07Z

This should be ready now. I’ve tested these changes with the change from #596

neo/NeoSystem.cs

jsolman · 2019-02-21T07:16:05Z

I’ll update the summary again since commits for synchronously writing the state were picked out directly from #575 into #547

We are going to implement #598

jsolman · 2019-02-21T18:03:43Z

Closing in favor of #598

* Add commit phase to consensus algorithm (#534) * Add commit phase to consensus algorithm * fix tests * Prevent repeated sending of `Commit` messages * RPC call gettransactionheight (#541) * getrawtransactionheight Nowadays two calls are need to get a transaction height, `getrawtransaction` with `verbose` and then use the `blockhash`. Other option is to use `confirmations`, but it can be misleading. * Minnor fix * Shargon's tip * modified * Allow to use the wallet inside a RPC plugin (#536) * Clean code * Clean code * Minor fix on mempoolVerified * Add MemoryPool Unit tests. Fix bug on initital start of Persisting the Genesis block. * Prevent `ConsensusService` from receiving messages before starting (#573) * Prevent `ConsensusService` from receiving messages before starting * fixed tests - calling OnStart now * Consensus recovery log (#572) * Pass store to `ConsensusService` * Implement `ISerializable` in `ConsensusContext` * Start from recovery log * Fix unit tests due to constructor taking the store. * Add unit tests for serializing and deserializing the consensus context. * Combine `ConsensusContext.ChangeView()` and `ConsensusContext.Reset()` * Add `PreparationHash` field to `PrepareResponse` to prevent replay attacks from malicious primary (#576) * Fixed a problem where `PrepareResponse.PreparationHash` was not assigned. * Load context from store only when height matches * Recover nodes requesting ChangeView when possible (#579) * Fixes bug in `OnPrepareRequestReceived()` * Send `RecoveryMessage` only when `message.NewViewNumber <= context.ViewNumber` * Fix and optimize view changing (#590) * Allow to ignore the recovery logs * Add `isRecovering` (#594) * Fix accepting own prepare request (#596) * Pick some changes from #575. * Fixes `Prefixes` * Restore transactions from saved consensus context (#598) * Refactoring * AggressiveInlining (#606) * Reset Block reference when consensus context is initialized after block persist. (#608) * Change `ConsensusPayload` for compatibility (#609)

Save consensus context each time we change view (so we will be close …

999251c

…to correct view on restart).

jsolman requested review from erikzhang, igormcoelho and vncoelho January 25, 2019 02:54

jsolman added the Critical Issues (bugs) that need to be fixed ASAP label Jan 25, 2019

This was referenced Jan 25, 2019

dBFT 2.0 #547

Merged

Consensus recovery log #572

Merged

Add extension methods for loading and saving the ConsensusContext to …

2ce8b71

…the Store.

Load MemoryPool initially with transactions from saved consensus.

ae56b38

jsolman and others added 2 commits January 24, 2019 20:09

Fix tests.

232e2fd

Merge branch 'consensus/improved_dbft' into consensus/recoveryLog2

d930e2c

vncoelho reviewed Jan 25, 2019

View reviewed changes

Remove unused 'using' statements.

4ae84f7

jsolman added potential enhancement and removed Critical Issues (bugs) that need to be fixed ASAP labels Jan 27, 2019

Merge branch 'consensus/improved_dbft' into consensus/recoveryLog2

947aa53

vncoelho previously approved these changes Jan 27, 2019

View reviewed changes

jsolman added Enhancement Type - Changes that may affect performance, usability or add new features to existing modules. and removed potential enhancement labels Jan 28, 2019

erikzhang reviewed Jan 28, 2019

View reviewed changes

neo/NeoSystem.cs Outdated Show resolved Hide resolved

shargon reviewed Jan 28, 2019

View reviewed changes

neo/Persistence/LevelDB/Prefixes.cs Outdated Show resolved Hide resolved

Decouple restoring memory pool from the consensus state from the Bloc…

82a0f2b

…kchain subsystem.

Remove unused using statement.

3dd2928

jsolman changed the title ~~Restore MemoryPool transactions from restored consensus context.~~ Write consensus context synchronously. Restore MemoryPool transactions from saved consensus context. Feb 19, 2019

jsolman and others added 6 commits February 19, 2019 12:20

Clean-up.

2fc2139

More clean-up. Remove unused using statements.

fc4cfe8

Remove one more unused using statement.

14a0a2c

Merge branch 'consensus/improved_dbft' into consensus/recoveryLog2

7e0f518

Minor changes

3d6a2b1

Add method PutSync() to class Store

438c105

jsolman commented Feb 20, 2019

View reviewed changes

neo/Persistence/Store.cs Outdated Show resolved Hide resolved

Merge branch 'consensus/improved_dbft' into consensus/recoveryLog2

e362f55

erikzhang added 2 commits February 21, 2019 13:16

Merge branch 'consensus/improved_dbft' into consensus/recoveryLog2

465e587

Renaming

3ebb75e

jsolman commented Feb 21, 2019

View reviewed changes

neo/NeoSystem.cs Outdated Show resolved Hide resolved

jsolman and others added 2 commits February 20, 2019 21:55

Remove unused variable.

75c81d9

Optimize the serialization of ConsensusContext

becf0f5

erikzhang added a commit that referenced this pull request Feb 21, 2019

Pick some changes from #575.

bbcfc68

erikzhang added 2 commits February 21, 2019 14:53

Merge branch 'consensus/improved_dbft' into consensus/recoveryLog2

0f9856e

Merge branch 'consensus/improved_dbft' into consensus/recoveryLog2

b957d1b

jsolman changed the title ~~Write consensus context synchronously. Restore MemoryPool transactions from saved consensus context.~~ Restore MemoryPool transactions from saved consensus context. Feb 21, 2019

shargon previously approved these changes Feb 21, 2019

View reviewed changes

jsolman closed this Feb 21, 2019

shargon deleted the consensus/recoveryLog2 branch February 22, 2019 12:29

Thacryba pushed a commit to simplitech/neo that referenced this pull request Feb 17, 2020

formatting (neo-project#575)

b8de274

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Restore MemoryPool transactions from saved consensus context. #575

Restore MemoryPool transactions from saved consensus context. #575

jsolman commented Jan 25, 2019 •

edited

Loading

erikzhang commented Jan 25, 2019 •

edited

Loading

jsolman commented Jan 25, 2019 •

edited

Loading

jsolman commented Jan 25, 2019

erikzhang commented Jan 25, 2019

jsolman commented Jan 25, 2019

vncoelho commented Jan 25, 2019 •

edited

Loading

jsolman commented Jan 25, 2019

jsolman commented Jan 25, 2019 •

edited

Loading

vncoelho left a comment •

edited

Loading

jsolman commented Jan 27, 2019

vncoelho left a comment

jsolman commented Feb 19, 2019

jsolman commented Feb 20, 2019 •

edited

Loading

jsolman commented Feb 21, 2019 •

edited

Loading

jsolman commented Feb 21, 2019

Restore MemoryPool transactions from saved consensus context. #575

Restore MemoryPool transactions from saved consensus context. #575

Conversation

jsolman commented Jan 25, 2019 • edited Loading

erikzhang commented Jan 25, 2019 • edited Loading

jsolman commented Jan 25, 2019 • edited Loading

jsolman commented Jan 25, 2019

erikzhang commented Jan 25, 2019

jsolman commented Jan 25, 2019

vncoelho commented Jan 25, 2019 • edited Loading

jsolman commented Jan 25, 2019

jsolman commented Jan 25, 2019 • edited Loading

vncoelho left a comment • edited Loading

Choose a reason for hiding this comment

jsolman commented Jan 27, 2019

vncoelho left a comment

Choose a reason for hiding this comment

jsolman commented Feb 19, 2019

jsolman commented Feb 20, 2019 • edited Loading

jsolman commented Feb 21, 2019 • edited Loading

jsolman commented Feb 21, 2019

jsolman commented Jan 25, 2019 •

edited

Loading

erikzhang commented Jan 25, 2019 •

edited

Loading

jsolman commented Jan 25, 2019 •

edited

Loading

vncoelho commented Jan 25, 2019 •

edited

Loading

jsolman commented Jan 25, 2019 •

edited

Loading

vncoelho left a comment •

edited

Loading

jsolman commented Feb 20, 2019 •

edited

Loading

jsolman commented Feb 21, 2019 •

edited

Loading