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

[3.2] Backporting state history plugin improvements and fixes #340

Merged
merged 32 commits into from
Jul 22, 2022

Conversation

vladtr
Copy link
Contributor

@vladtr vladtr commented Jun 2, 2022

This PR is mainly targeting performance improvements with addition of threading support plus Unix socket support. A separate Issues / PR's for other backports that would be considered valuable will be created.

Backported in this PR:

  • Move all network operations to another thread (8f693d3ed720056867e2bb2864ee6bde83e2570c) and all dependencies
  • ported session_manager_t to make add/remove thread safe
  • Unix socket support
  • SHiP post send_update to main thread to avoid overwhelming the main
  • Added threaded trace history/chain state history writes
  • Make compatible with boost < 1.70
  • Add comment why include_delta of code_object just returns false
  • Pop back a delta with empty rows
  • Fixed support for boolean args in unit tests (as a side bug while bringing in ut for unix socket)
  • Included test for local socket connection and added support o ship_client to connect via domain socket

@vladtr vladtr linked an issue Jun 2, 2022 that may be closed by this pull request
@vladtr vladtr changed the title Backported threading, session manager, unix socket Backporting state history plugin improvements and fixes Jun 2, 2022
@vladtr vladtr self-assigned this Jun 2, 2022
@vladtr vladtr marked this pull request as ready for review June 19, 2022 19:17
@vladtr vladtr requested review from arhag and heifner June 19, 2022 19:17
@spoonincode
Copy link
Member

The original PR adding ship unix socket support included adding unit test support for it. It seems like that might be something worthwhile keeping in at least one of the ship tests.

@vladtr
Copy link
Contributor Author

vladtr commented Jun 22, 2022

The original PR adding ship unix socket support included adding unit test support for it. It seems like that might be something worthwhile keeping in at least one of the ship tests.

Added and it fails lol Debugging it now...

@vladtr
Copy link
Contributor Author

vladtr commented Jun 23, 2022

The original PR adding ship unix socket support included adding unit test support for it. It seems like that might be something worthwhile keeping in at least one of the ship tests.

Added and it fails lol Debugging it now...

Looks like ship_client needs to support unix sockets for this test to work, This is added as a separate issue:

#508

currently there is a workaround to make UT passable in ship_client, that makes it switch to ws communication when unix+ws requested. This will be removed upon 508 resolution

@vladtr
Copy link
Contributor Author

vladtr commented Jun 27, 2022

Proper unix_socket support implemented for ship_client, since it is closely connected to this issue, it is committed in this PR

@vladtr vladtr linked an issue Jun 27, 2022 that may be closed by this pull request
@vladtr vladtr requested review from spoonincode June 30, 2022 18:28
@heifner heifner changed the title Backporting state history plugin improvements and fixes [3.2] Backporting state history plugin improvements and fixes Jul 7, 2022
plugins/state_history_plugin/state_history_plugin.cpp Outdated Show resolved Hide resolved
plugins/state_history_plugin/state_history_plugin.cpp Outdated Show resolved Hide resolved
plugins/state_history_plugin/state_history_plugin.cpp Outdated Show resolved Hide resolved
plugins/state_history_plugin/state_history_plugin.cpp Outdated Show resolved Hide resolved
tests/ship_client.cpp Outdated Show resolved Hide resolved
tests/CMakeLists.txt Outdated Show resolved Hide resolved
@@ -119,9 +132,37 @@ class state_history_log {
vacuum();
}
}

thr = std::thread([this] {
Copy link
Member

Choose a reason for hiding this comment

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

An eosio::chain::named_thread_pool (even with a size of 1) may allow you to remove some of this boilerplate. Not critical, judgement up to you, just pointing it out.

Copy link
Member

Choose a reason for hiding this comment

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

eh the whole write_thread_has_exception thing may torpedo that, dunno. Still worth thinking over.

@vladtr vladtr merged commit 10b6513 into main Jul 22, 2022
@vladtr vladtr deleted the state-history-improvements branch July 22, 2022 22:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants