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

chore: remove abci socket implementation #214

Merged
merged 6 commits into from
Apr 15, 2021

Conversation

jinsan-line
Copy link

Epic: https://github.com/line/lbm/issues/1305

Cherry-pick: #153

Description

We aim to optimize the chain performance. One of strateges is increasing concurrency. For it, we need to revise abci interface. Currently, the biggest challenge is to remove flush() api from abci interface. This api is meaningful only for socket implementation. We do not use grpc/socket abci implementation at this moment and do not have any plan to use it. So I tried to remove socket implementation. It makes us more flexible to optimize performance.

Now abci-cli only supports grpc but not socket. All tests, using socket previously, are now revised to use grpc. So all tests are sill valid and should be passed.

Dropped first approach

I tried to remove abci grpc and socket implementations together. But I get realized that we need at least one remote abci implementation for tests. Removing all remote abci implementation makes test harder. And also I get realized that it's only need to remove socket implementation in order to remove flush() api.

ABCI implementations

  • Reference
  • local (in a process)
    • Tendermint and abci application run together in a process.
    • We're using it at this moment.
  • grpc
    • Tendermint and abci application run independently and communicate w/ other through grpc.
    • There are various examples and explanation on the official documents.
  • socket
    • It's aimed to support the language that doesn't have grpc implementation.
    • I think it's not practical implementation because grpc is already implemented for almost popular languages.
    • It's used intensively for integrate tests.

For contributor use:

  • Wrote tests
  • Updated CHANGELOG_PENDING.md
  • Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
  • Updated relevant documentation (docs/) and code comments
  • Re-reviewed Files changed in the Github PR explorer

# Conflicts:
#	abci/client/socket_client.go
#	abci/client/socket_client_test.go
#	abci/example/example_test.go
#	abci/example/kvstore/kvstore_test.go
#	abci/server/socket_server.go
#	config/config.go
#	test/app/test.sh
@jinsan-line jinsan-line added this to the initial ebony milestone Apr 15, 2021
@jinsan-line jinsan-line self-assigned this Apr 15, 2021
@jinsan-line jinsan-line removed the WIP label Apr 15, 2021
@jinsan-line jinsan-line changed the title Jinsan/v2/remove abci socket implementaion chore: remove abci socket implementaion Apr 15, 2021
@jinsan-line jinsan-line changed the title chore: remove abci socket implementaion chore: remove abci socket implementation Apr 15, 2021
@jinsan-line jinsan-line merged commit e772f8b into ebony Apr 15, 2021
@jinsan-line jinsan-line deleted the jinsan/v2/remove-abci-socket-implementaion branch April 15, 2021 11:05
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.

3 participants