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

WIP: Revert time intependent consensus #29

Closed
wants to merge 1 commit into from
Closed

WIP: Revert time intependent consensus #29

wants to merge 1 commit into from

Conversation

stanislav-tkach
Copy link
Contributor

Revert #6 pull request because of degrading performance. In order to preserve #22, some changes are manual and not pure revert-commit.

Copy link
Contributor

@alekseysidorov alekseysidorov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Contributor

@dj8yfo dj8yfo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Checked this vs previous revision. 5 empty blocks per minute -> 37 empty blocks per minute on default config. (6 validators).

@vldm
Copy link
Contributor

vldm commented Apr 12, 2017

Icebox it for future tests

@dj8yfo
Copy link
Contributor

dj8yfo commented Apr 15, 2017

@defuz, so, we came to understanding so far that we don't need to revert time handling to initial state, because the issue with empty blocks on multiple (6-8) nodes is easily handled with status_timeout set to a lower value (5 -> 3 sec e.g.). And the performance of 4 nodes shows similar values to the performance of reverted version, though 3 times lower than that, achieved quite before.

@defuz defuz changed the title Revert time intependent consensus WIP: Revert time intependent consensus Apr 27, 2017
@dj8yfo dj8yfo added the icebox label May 3, 2017
@vldm
Copy link
Contributor

vldm commented May 21, 2017

Can we close this pr? @defuz @gisochre

@stanislav-tkach stanislav-tkach self-assigned this Jun 2, 2017
@stanislav-tkach
Copy link
Contributor Author

I agree that this should be closed: because of the conflicts it would be easier to implement it from the scratch (if we ever need this).

@dj8yfo
Copy link
Contributor

dj8yfo commented Jun 6, 2017

@defuz should we close this as too far behind?

@defuz
Copy link
Contributor

defuz commented Jun 6, 2017

Closed as outdated

@defuz defuz closed this Jun 6, 2017
@defuz defuz deleted the revert-time-independent-consensus branch June 6, 2017 15:14
@defuz defuz restored the revert-time-independent-consensus branch June 6, 2017 15:15
@stanislav-tkach stanislav-tkach deleted the revert-time-independent-consensus branch July 17, 2017 11:06
stanislav-tkach pushed a commit to stanislav-tkach/exonum that referenced this pull request Feb 17, 2018
Added new end-point for outputting wallet(s) info
stanislav-tkach pushed a commit to stanislav-tkach/exonum that referenced this pull request Apr 4, 2018
stanislav-tkach added a commit to stanislav-tkach/exonum that referenced this pull request Apr 23, 2018
pavel-mukhanov pushed a commit to pavel-mukhanov/exonum that referenced this pull request Apr 5, 2019
New identifiers pool implementation [ECR-2807]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants