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

Max execution time of request on http server #5183

Open
GuillaumeBrook opened this issue Nov 8, 2023 · 5 comments
Open

Max execution time of request on http server #5183

GuillaumeBrook opened this issue Nov 8, 2023 · 5 comments

Comments

@GuillaumeBrook
Copy link

GuillaumeBrook commented Nov 8, 2023

I know it has been asked some times already :
#4594
#3078

How can we implement a real max_execution_time that will free the worker.

One proposed solution was that :

$http->on(
    Constant::EVENT_REQUEST,
    function (Request $request, Response $response) use () {
        echo 'Http request started' . "\n";
        Timer::after(5000, function () use ($response) {
            $response->status(408);
            $response->end('Timeout');
            echo 'Timeout has been sent' . "\n";
        });

       // Actual code that take maybe forever somehow

        echo 'We reached the end' . "\n";
        $response->status(200);
        $response->end('The end');
    }
);

So indeed the client will receive a 408 timeout.
But the worker is still working and doing things right ? (things that will never reach the end user)

So another implementation could be :

$http->on(
    Constant::EVENT_REQUEST,
    function (Request $request, Response $response) {
        echo 'Http request started' . "\n";
        $killRequest = false;
        Timer::after(5000, function () use ($response, &$killRequest) {
            $response->status(408);
            $response->end('Timeout');
            echo 'Timeout has been sent' . "\n";
            $killRequest = true;
        });

        echo 'Step 1' . "\n";
        sleep(2); // code that takes 2s
        if ($killRequest) return;

        echo 'Step 2' . "\n";
        sleep(2); // code that takes 2s
        if ($killRequest) return;

        echo 'Step 3' . "\n";
        sleep(2); // code that takes 2s
        if ($killRequest) return; // we stop here

        echo 'Step 4' . "\n";
        sleep(2); // code that takes 2s
        if ($killRequest) return;

        echo 'Step 5' . "\n";
        sleep(2); // code that takes 2s
        if ($killRequest) return;


        echo 'We reached the end' . "\n";
        $response->status(200);
        $response->end('The end');
    }
);

The print will be

Http request started
Step 1
Step 2
Step 3
Timeout has been sent

OK that's fine, but that just impossible to do, because we are inside classes doing stuff.

I'm concerned about CPU usage, RAM usage and especially about the workers.

From my test with only 1 worker, if somehow I don't use the database proxy classes, the worker can only treat one request at a time.
Meaning the first request is processing, the second is waiting.
I did the same test with database proxy classes, the second request doesn't wait however.
But I'm still concerned, if somehow all workers are blocked and the server can't receive any new request.

Is this an option that could be implemented in swoole internals ? What do you think ?

(OpenSwoole implemented it but I'm not sure it really works : openswoole/ext-openswoole#136)

@catchem88
Copy link

I used both Swoole & OpenSwoole, and can confirm that both have the same behavior regarding max_execution_time,
one other workaround if you trigger an error, I see that the worker will stop.

ex:
trigger_error('Request timed out', E_USER_ERROR);

but, +1 on this request, I believe this should be handled in Swoole.

@ValiDrv
Copy link

ValiDrv commented Nov 14, 2023

Problem is, you might have to clean up some connections and whatnot, so if you just stop executing code after the timeout happens in the next context change, you end up with memory leaks and broken code branches and whatnot.

In other words, the $killRequest idea is good, but your code still needs to deal with the cleanup and whatnot.

@thor-son
Copy link

Execute your code in coroutine.

$http->on(
    Constant::EVENT_REQUEST,
    function (Request $request, Response $response) {
        echo 'Http request started' . "\n";
        $killRequest = false;
        $timerId = Timer::after(5000, function () use ($response, &$killRequest) {
            $response->status(408);
            $response->end('Timeout');
            echo 'Timeout has been sent' . "\n";
            $killRequest = true;
        });
        go(function() use (&$killRequest, $timerId) {
            echo 'Step 1' . "\n";
            sleep(2); // code that takes 2s
            if ($killRequest) return;

            echo 'Step 2' . "\n";
            sleep(2); // code that takes 2s
            if ($killRequest) return;
               
             ...

            Timer::clear($timerId);
        }

        echo 'We reached the end' . "\n";
        $response->status(200);
        $response->end('The end');
    }
);

@ValiDrv
Copy link

ValiDrv commented Feb 20, 2024

You can't really do that, since you might have 10001 more levels/coroutines/context switches in your request function, and you would be to keep track and check that $killRequest variable everywhere.

@thor-son
Copy link

thor-son commented Feb 21, 2024

If you are really worried about CPU usage and memory usage, kill the http request worker process and have it re-create itself automatically.

        $timerId = Timer::after(5000, function () use ($response, &$killRequest) {
            $response->status(408);
            $response->end('Timeout');
            echo 'Timeout has been sent' . "\n";
            exec('kill -9 ' . getmypid());
            //$killRequest = true;
        });

And if there is too much delay, it appears that the application design itself is flawed.
It seems a good idea to take appropriate handling for functions that may be delayed.

    pcntl_async_signals(TRUE);
    try {
        pcntl_alarm(1);
        pcntl_signal(SIGALRM, function() {throw new Exception;});

        your_worried_function();

    } catch (Exception $e) {
            echo "catch\n";
            return;
    }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants