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

[core] Use single asio context along the IO path #49172

Merged
merged 1 commit into from
Dec 11, 2024

Conversation

dentiny
Copy link
Contributor

@dentiny dentiny commented Dec 9, 2024

IIUC, we should use one single io_context, which represents one eventloop along the async operation path.

Signed-off-by: hjiang <dentinyhao@gmail.com>
@dentiny dentiny requested review from jjyao, rynewang and dayshah December 9, 2024 19:43
@dentiny dentiny requested a review from a team as a code owner December 9, 2024 19:43
@dentiny dentiny added the go add ONLY when ready to merge, run all tests label Dec 9, 2024
using namespace boost::asio;
io_context ctx;
ip::tcp::resolver resolver(ctx);
ip::tcp::resolver resolver(io_service);
Copy link
Contributor

Choose a reason for hiding this comment

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

would this call poll on the io_service in a different thread, and is this the main_io_context for the gcs. I actually just made the other pr with the concurrency hint for main_io_context which assumes io ops would only happen on a single thread

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes I understand the implication, please notice all other redis related operations are scheduled on the same eventloop.

@dentiny dentiny requested a review from dayshah December 9, 2024 20:17
@@ -560,10 +562,11 @@ Status ConnectRedisSentinel(RedisContext &context,
}
}

std::vector<std::string> ResolveDNS(const std::string &address, int port) {
std::vector<std::string> ResolveDNS(instrumented_io_context &io_service,
Copy link
Collaborator

Choose a reason for hiding this comment

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

If I'm calling ResolveDNS from the io_service thread, will this be a problem (deadlock)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not a problem, my test script.

#include <boost/asio.hpp>
#include <boost/asio/ip/tcp.hpp>
#include <iostream>
#include <memory>

void resolveAddress(boost::asio::io_context& io_context, const std::string& host, const std::string& port) {
    try {
        // Create a resolver
        boost::asio::ip::tcp::resolver resolver(io_context);

        // Resolve the address
        boost::asio::ip::tcp::resolver::results_type endpoints = resolver.resolve(host, port);

        std::cout << "Resolved endpoints:" << std::endl;
        for (const auto& endpoint : endpoints) {
            std::cout << endpoint.endpoint() << std::endl;
        }
    } catch (const std::exception& e) {
        std::cerr << "Error: " << e.what() << std::endl;
    }
}

int main() {
    try {
        boost::asio::io_context io_context;

        // Post a functor into the io_context
        io_context.post([&io_context]() {
            std::cout << "Functor running in io_context." << std::endl;

            // Use the io_context with a TCP resolver
            resolveAddress(io_context, "example.com", "80");
        });

        // Run the io_context
        io_context.run();
    } catch (const std::exception& e) {
        std::cerr << "Exception: " << e.what() << std::endl;
    }

    return 0;
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Output for the above code snippet.

Functor running in io_context.
Resolved endpoints:
93.184.215.14:80
[2606:2800:21f:cb07:6820:80da:af6b:8b2c]:80

@dentiny dentiny requested a review from jjyao December 11, 2024 10:46
@jjyao jjyao merged commit 6d73f25 into ray-project:master Dec 11, 2024
5 checks passed
simonsays1980 pushed a commit to simonsays1980/ray that referenced this pull request Dec 12, 2024
ujjawal-khare pushed a commit to ujjawal-khare-27/ray that referenced this pull request Dec 17, 2024
Signed-off-by: hjiang <dentinyhao@gmail.com>
Signed-off-by: ujjawal-khare <ujjawal.khare@dream11.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
go add ONLY when ready to merge, run all tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants