Skip to content

Internal node io context#4618

Merged
pwojcikdev merged 15 commits intonanocurrency:developfrom
pwojcikdev:node-io-context-6
May 14, 2024
Merged

Internal node io context#4618
pwojcikdev merged 15 commits intonanocurrency:developfrom
pwojcikdev:node-io-context-6

Conversation

@pwojcikdev
Copy link
Copy Markdown
Contributor

Currently all nodes, rpc and ipc servers share common IO context. This PR changes it, so that each node owns and runs its own io_context. This ensures that any io operations scheduled to run using that executor can't outlive the node itself. This greatly simplifies system design.

It's likely that this will uncover many race conditions in tests. The most obvious ones should already be fixed, but please keep in mind that if a test starts failing after this PR, it likely wasn't fully correct to start with.

@@ -139,13 +140,15 @@ nano::node::node (std::shared_ptr<boost::asio::io_context> io_ctx_a, uint16_t pe
}

nano::node::node (std::shared_ptr<boost::asio::io_context> io_ctx_a, std::filesystem::path const & application_path_a, nano::node_config const & config_a, nano::work_pool & work_a, nano::node_flags flags_a, unsigned seq) :
Copy link
Copy Markdown
Contributor Author

@pwojcikdev pwojcikdev May 13, 2024

Choose a reason for hiding this comment

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

A reference to io_ctx is still passed, unnecessarily. Removing it now would add noise. Such cleanup can be easily done in a subsequent PR.

Copy link
Copy Markdown
Contributor

@clemahieu clemahieu left a comment

Choose a reason for hiding this comment

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

Comments are just observations. This fixes a locally consistently failing test socket.max_connections.

Even though some parts are rough around the edges, ipc_service::stop, I'd much rather get this change in as it's been attempted many times and never finished.

@qwahzi qwahzi added this to the V27 milestone May 14, 2024
@pwojcikdev pwojcikdev merged commit 45d612a into nanocurrency:develop May 14, 2024
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