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

refactor thread and asio usage in state history plugin #627

Closed
spoonincode opened this issue Jan 9, 2023 · 2 comments · Fixed by #727
Closed

refactor thread and asio usage in state history plugin #627

spoonincode opened this issue Jan 9, 2023 · 2 comments · Fixed by #727
Assignees
Labels

Comments

@spoonincode
Copy link
Member

Some usage of asio in state history plugin looks at odds with typical asio patterns. For example: needing a connection to call an explicit close() function after a failure occurs (normally you'd just let the shared_ptr fall out of scope to dtor it), and some sort of stopping variable (normally you'd just stop() the io_context which then ensures no callbacks are running, and then just let everything dtor itself). To be fair there may be yet-undiscovered-to-me reasons these are required, but so far I haven't seen it.

state history plugin is also very loose and haphazard with how it handles its thread, oftentimes punting back and forth between main thread. Yet, it seems like state history plugin's thread usage could be awfully simplistic: build up blocks' traces & deltas on the main thread and then when on_accepted_block() simply post() over to ship's thread which then does everything it needs to entirely on the ship thread without ever needing to go back to main thread.

Resolving the above issues will make it significantly easier to reason about state history plugin's flow and will be less prone to introducing failures.

@heifner
Copy link
Member

heifner commented Jan 16, 2023

  • Currently block data for SHiP is pulled from forkdb or blocklog. SHiP does not store blocks in its log files. SHiP optionally stores traces and table deltas in its logs. Default options for SHiP actually pulls all data it serves from forkdb/blocklog.
  • forkdb was recently made thread safe. Blocklog is current not thread safe.
  • Blocklog could be made thread safe and then SHiP could pull the data it needs from the SHiP thread(s) without needing to post() onto the main thread.

@spoonincode
Copy link
Member Author

There are other benefits to allowing blocklog to be thread safe, such as net_plugin being able to pull requested blocks from it on while on net threads, so seems like this would be good improvement to hit first

@heifner heifner self-assigned this Feb 14, 2023
@heifner heifner added the OCI Work exclusive to OCI team label Feb 14, 2023
@heifner heifner moved this from Todo to In Progress in Team Backlog Feb 14, 2023
heifner added a commit that referenced this issue Feb 14, 2023
heifner added a commit that referenced this issue Feb 15, 2023
heifner added a commit that referenced this issue Feb 17, 2023
heifner added a commit that referenced this issue Feb 17, 2023
…ssion_manager is used to coordinate sending since only one session can access the trace/delta log files at a time.
heifner added a commit that referenced this issue Feb 17, 2023
heifner added a commit that referenced this issue Feb 18, 2023
heifner added a commit that referenced this issue Feb 20, 2023
heifner added a commit that referenced this issue Feb 20, 2023
@heifner heifner moved this from In Progress to Awaiting Review in Team Backlog Feb 21, 2023
heifner added a commit that referenced this issue Mar 2, 2023
SHiP move client handling off the main thread
@github-project-automation github-project-automation bot moved this from Awaiting Review to Done in Team Backlog Mar 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

4 participants