drogonframework / drogon

Drogon: A C++14/17/20 based HTTP web application framework running on Linux/macOS/Unix/Windows
MIT License
11.42k stars 1.09k forks source link

Allow thread to process other requests when waiting for asyncrinous HTTP requests #400

Closed marty1885 closed 3 years ago

marty1885 commented 4 years ago

Hi, this issue is mostly related to the maximum throughput of an application. Currently Drogon have to wait if it makes a HTTP request inside a request handler. This may be a performance bottleneck when the request takes a while to response (ex: to a server across the globe, long RTT, etc...). It would be nice to be able to process other other requests while we're waiting for response.

Now, sending requests within a handler looks like so

app().registerHandler("/foo",[](const HttpRequestPtr& req, auto &&callback) {
    // some API things
    auto client = HttpClient::newHttpClient("http://my.api.endpoint.com");
    auto req = HttpRequest::newHttpRequest();
    std::promise<bool> valid;
    req->setPath("/do_something");
    client->sendRequest(req, [&](ReqResult result, const HttpResponsePtr &response) {
        if(response == nullptr) // If no server responce
            valid.set_value(false);
        valid.set_value(true);
    });
    bool api_ok = valid.get_future().get(); // Wait for HTTP to response. Have to wait here otherwise crash the entire application
    if(api_ok == false)
        api_failed();

    // Keep doing our stuff
});

It would be great to do something like this.

client->sendRequest(validation_req, [&](ReqResult result, const HttpResponsePtr &response) {
    ...});
drogon::handleOtherRequest(valid); // Do other stuff until `valid` have been set. Don't waist time waiting

Alternatively, it would be great to let HttpClient::sendRequest itself to do everything. So HttpClient::sendRequest looks to be a blocking call from the caller's perspective but in reality it only returns when it got an response or timeouts (and handle other requests in the mean time).

bool api_ok = false;
client->sendRequestSync(req, ...); // This looks like a blocking call from the caller
// Since sendRequestSync only rerun when it have a response. The control flow is a lot easier
if(api_ok == false)
    api_failed();

Thanks

an-tao commented 4 years ago

I don't think we should block the current thread, otherwise the throughput will decrease. My suggestion is to move the subsequent steps inside the callback function, for example:

app().registerHandler("/foo",[](const HttpRequestPtr& req, auto &&callback) {
    // some API things
    auto client = HttpClient::newHttpClient("http://my.api.endpoint.com");
    auto req = HttpRequest::newHttpRequest();
    req->setPath("/do_something");
    client->sendRequest(req, [callback=std::move(callback)](ReqResult result, const HttpResponsePtr &response) {
        if (result == ReqResult::Ok)
                 {
                       // Keep doing our stuff ...
                          callback(yourResponse);
                 }
                 else
                 {
                       //make a response with error messages....
                         callback(errResponse);
                 }
    }); 
});

here is a simple reverse proxy example, you can refer to this example. and the #307 is some discusses about the reverse proxy.

marty1885 commented 4 years ago

Thanks for the quick response! Writing callbacks work but can quickly become a callback hell especially in a micro service design. And capturing every variable, even moving them, may be slow or impossible (objects allocated using a custom allocator, an allocator, large stack objects, etc). Is there a chance to resolve this?

an-tao commented 4 years ago

I think that only coroutines can provide high throughput without causing a callback hell. I will consider adding support for coroutines in the future, but may wait until c++20 is supported by most compilers.

Do you have any suggestions? thanks.

marty1885 commented 4 years ago

I think coroutines are the solution. But before C++20 is widely available, I think the ability to invoke the event loop manually would mostly solve the problem. Instead of

bool api_ok = valid.get_future().get();

We do

auto future = valid.get_future();
while(!future.valid()) {
    if(drogon::haveUnhandledRequest())
        drogon::handleRequest();
}

bool ok = future.get();

if its possible in Drogon's architecture. Or (possible better) just embedded the while loop inside sendRequest (maybe by adding a bool parameter to make it block or async).

Anyway I'm not familiar with Deogon's internals. Is my solution possible?

an-tao commented 4 years ago

I am afraid this will destroy the internal structure of the framework. And when the concurrency is high, this will cause a stack overflow (because the while loop is nested many times. And if there are always new requests coming in before the reponse of the old one being received, the call stack gets deeper and deeper until the stack overflows)

marty1885 commented 4 years ago

I see... I guess I'll have to deal with callbacks before coroutines are available. Thanks again. :smiley:

vanehu commented 4 years ago

boost.fiber?

marty1885 commented 4 years ago

@vanehu Mind to share details on how it could work? Maybe a POC?

dantti commented 4 years ago

I went here to see how do you solve this issue, but it seems you have the same problems that I had in Cutelyst, and yes I initially solved by calling the local QEventLoop and eventually got the stack overflow.

I need to study a bit more on how coroutines work to see if they fit in my use-case.

Anyway, the way I currently solved this in Cutelyst was by detaching the request from final processing, I call c->detachAsync() which will prevent headers and body to be sent, then the called function can return and rewind the stack, then until c->attachAsync() isn't called request doesn't finish, if the client closes the connection c is destroyed, not sure you can use this idea but I hope you find it useful.

marty1885 commented 4 years ago

@dantti My you share your use case? I'm also working on a way to use coroutines while having a reasonable syntax. I'm stuck right now but maybe new ideas can push the idea forward.

an-tao commented 4 years ago

@dantti Thank you for the information provided, yes, I face the same problem. I will carefully consider your solution, thank you for your help.

rbugajewski commented 4 years ago

Yes, there is a risk for getting in callback hell. The same observation applies to asynchronous database access with multiple nested queries over a transaction. But there are ways how one can cope with the devil 👹:

Now all function calls start to look like plain old regular function calls that are easier to follow. By sticking to these two rules most of the ugliness of callback hell is gone.

dantti commented 4 years ago

@marty1885 well my use case is the same, a C++ web framework, from my research boost coroutines are full stack so doen't help here, C++20 coroutines wasn't finished at the time of my research but were stackless, the thing is that it looks like you have to save the stack and iirc returning an structure to it, I really need to play with it myself but seems like the only reasonable approach besides what I already did. With Cutelyst we also have a chain of actions (in order to split pre/post processing, like auth and rendering), so once a coroutine wait is issued I need to save where in the chain should I resume.

pratikpc commented 3 years ago

@dantti if you plan to implement Coroutines, it's not Drogon code but Boost Asio code but maybe Gor's article might prove helpful

dantti commented 3 years ago

@pratikpc thanks but I'm no Boost fan, I will either wait for Qt create some helper class or write them myself, I've seem some folks creating some cool classes to deal with async network code and Qt GUI apps not sure it would be useful in my case but I'm still doing some research, also my current async model has worked well with ASql

pratikpc commented 3 years ago

@dantti No no Don't use Boost But you can write code in the exact same manner for Drogon as well.

It basically showcases what you would need to write coro based code for Drogon

It could sort of serve as a nice tutorial

marty1885 commented 3 years ago

Close as #693 provides the ability.