modify (simplify) state_history plugin's thread logic; add get_status_request_v1
#96
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Note:start |
spoonincode
commented
May 31, 2024
arhag
dismissed
their stale review
Will be addressed in another PR (refactoring log code) that must be merged as a pair with this one.
spoonincode
force-pushed
the
ship_thread_refactor
branch
from
078145e to
18bea83
Compare
|
really sorry for the force push at the end; pushed the log changes here that I wanted to keep as a separate PR |
|
actually, since review hasn't started, I'm just going to reopen a PR without the force pushes... sorry about that... |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
User Facing Changes
Add
get_status_request_v1andget_status_result_v1for completeness and consistency. It's a minor thing, this just provides access to the range of blocks with finality data available.Some state_history log messages have been removed, most notably the log that occurs on every block. That seemed rather unnecessary (and was not present in 2.0/3.1) but this change can certainly be revisited.
Internal Changes
The entirety of the connection code has been reworked to simplify the threading logic. This new design makes all decisions about what blocks to send on the main thread, and then conveys that information (while holding appropriate handles) to the ship thread which does the log reading, decompression, and websocket operations. This rework is expected to fix #275 because the prior implementation would sometimes not send forks properly due to thread timing.
Blocking Issues
Unfortunately with the way the log implementation relies on mutexes the new approach is currently subject to deadlocks. Log implementation will need to be refactored to be free of mutexes next.