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

Consensus optimizations taking the advantage of ChangeViewReason #988

Open
shargon opened this issue Aug 2, 2019 · 7 comments
Open

Consensus optimizations taking the advantage of ChangeViewReason #988

shargon opened this issue Aug 2, 2019 · 7 comments
Labels
consensus Module - Changes that affect the consensus protocol or internal verification logic discussion Initial issue state - proposed but not yet accepted enhancement Type - Changes that may affect performance, usability or add new features to existing modules.

Comments

@shargon
Copy link
Member

shargon commented Aug 2, 2019

Now, thanks to #926 we are able to know why CN decide to change the view.

If the last view change was because the other CN haven't got some transactions, there are more chance to get a new block if the next proposal have instead of 500 tx, 250. If you reduce the number of transactions in the next block, you increase the chance of the agreement.

Possible implementation #933

@lock9 lock9 added consensus Module - Changes that affect the consensus protocol or internal verification logic discussion Initial issue state - proposed but not yet accepted enhancement Type - Changes that may affect performance, usability or add new features to existing modules. and removed potential enhancement labels Aug 9, 2019
@shargon
Copy link
Member Author

shargon commented Sep 7, 2019

This was not decided yet, my mistake when we choose it @lock9

@shargon shargon changed the title Consensus optimizations Consensus optimizations taking the advantage of ChangeViewReason Sep 7, 2019
@shargon
Copy link
Member Author

shargon commented Sep 9, 2019

Acording to the comments in the pull request #933, the proposed test that @vncoelho introduced in our talks on Neo Community could help to test the benefits of this kind of issues. (I don't know if the tool has a name yet)

For me the gains are very clear because in the past, @belane and me made a lot of tests with high overload in the network.

We should define rules for improve the behavior/performance acording to ChangeViewReason, and for me, if the view was changed because some consensus nodes haven't got enough transactions, if your proposal is lighter, will be easier for ensure the distribution. As @vncoelho said, is something strategically, hard to test it.

We can test it if we simulate the perfect environment, but is not something real.

@shargon
Copy link
Member Author

shargon commented Sep 9, 2019

In order to increase the TPS, if we want to increase the transaction per block to 10k (for example) we need a strategics for prevent/deal with view changes.

@belane
Copy link
Member

belane commented Sep 30, 2019

Then the idea is to reduce the number of transactions per block after a change of view, if this has been produced by some transaction that the Primary knows but not most Backups, right?

But if it has changed view and the role of Primary, the context has changed too, I do not understand how it can help to change the number of transactions per block in the next block.

@eryeer
Copy link
Contributor

eryeer commented Dec 30, 2019

I agree with @belane, if ChangeView occurs, the primary node will be switched, then ConsensusContext is different from before. It is possible that even if consensus transactions are reduced, transactions between consensus nodes may not be synchronized too.

From another perspective, improving the synchronization speed of consensus transaction may be a more direct method. As mentioned in #1382, the most time consuming during consensus transaction synchronization is knownHashes.ExceptWith (payload.Hashes) in TaskManager OnRestartTasks();
It is extremely inefficient under high load transaction volume synchronization. After this part is optimized, Consensus transaction synchronization speed is around 0.005s each, that is, 200 tx in 1s.

@shargon
Copy link
Member Author

shargon commented Dec 30, 2019

The context is switched, and maybe the case will be the same (and you have another view change) or maybe not, this is a must.

@eryeer
Copy link
Contributor

eryeer commented Dec 30, 2019

Under high overload, will this change put the pressure on the memory pool? Because there are fewer transactions packaged in consensus context, more transactions will be left in memory pool. If the memory pool becomes full, more transactions will be lost, resulting in the consensus transaction of the next block is still difficult to synchronize.

Thacryba pushed a commit to simplitech/neo that referenced this issue Feb 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
consensus Module - Changes that affect the consensus protocol or internal verification logic discussion Initial issue state - proposed but not yet accepted enhancement Type - Changes that may affect performance, usability or add new features to existing modules.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants