libvips / php-vips

php binding for libvips
MIT License
615 stars 25 forks source link

Frequent execution of Vips:cache_set_max() causes crash #203

Closed muckee closed 1 year ago

muckee commented 1 year ago

Environment

Ubuntu 23.04 (daily) PHP 8.2 w/ FPM NGINX libvips42 w/ php-vips

Problem

I have an API built with Slim Framework which uses vips to perform CRUD operations on a folder of images.

Within the API middleware, I use VipsConfig::cacheSetMax(13);, also tried with lower values.

Earlier had issues making concurrent requests to writeToBuffer() when using sequential mode. Didn't manage to resolve it, just stopped using sequential mode.

Now in production I'm seeing upstream timed out... in php-fpm logs when making consecutive requests to the API, without executing any vips command.

At the time of writing, you can test by cURLing https://api.thugnerdz.com/. You should see the response Hello, nerd!, but sending multiple requests in quick succession will return a 502 error.

Sample

Middleware

    $app->add(function (Request $request, RequestHandlerInterface $handler): Response {
        VipsConfig::cacheSetMax(13);
        return $handler->handle($request);
    });

Route

$app->get('/', function (Request $request, Response $response) {
    $response->getBody()->write('Hello, nerd!'); // `root`
    return $response;
});

Installed libvips via apt and installed the PECL extension. Composer library has jcuppit/vips: ^2.0 installed.

muckee commented 1 year ago

UPDATE

Was attempting to reproduce last night with a clean implementation of slim/slim-skeleton but was not able to. Creating a 'Hello, world!' endpoint which only executes cacheSetMax() within its middleware can be executed many times rapidly without failure.

There are other php services running in the background of the production server and the frontend I'm developing can make other calls to the API which also utilise VIPS, so I guess something there is the issue.

This morning I cleaned up the logs and noticed I was getting the error server reached pm.max_children setting (5), consider raising it in the FPM logs.

After raising the value from 5 to 10, I am no longer seeing this error but I still get an upstream timed out error in the application's nginx logs if I send more than one request in a short period of time.

What confuses me is that the first request I made just now was immediately successful (the second one timed out) and no VIPS processing has occurred on the server for several hours.

The settings mentioned here seem to be working: https://serverfault.com/a/520895

Although without updating those settings, I'm waiting for the first request to complete before issuing the second and it breaks anyway, so it feels like a workaround and not a solution, but I don't have a great deal of experience with vips so the problem might just be my ignorance.

Happy to invite you to the offending repo (it's private, but don't mind if you copy), but feel free to close this if it sounds like the solution is good enough.

jcupitt commented 1 year ago

Hi @muckee,

I'm glad you found a workaround. It's not clear if this is a libvips issue, so I'm inclined to close.

Earlier had issues making concurrent requests to writeToBuffer() when using sequential mode. Didn't manage to resolve it, just stopped using sequential mode.

This ought to work. You cannot reuse images opened in sequential mode, could that be the issue?

For example, this code will fail:

$image = Vips\Image::newFromFile("x.jpg", ["access" => "sequential"]);
$a = $image->writeToBuffer(".png");
$b = $image->writeToBuffer(".png");

But this will work:

$image = Vips\Image::newFromFile("x.jpg", ["access" => "sequential"]);
$a = $image->writeToBuffer(".png");
$image = Vips\Image::newFromFile("x.jpg", ["access" => "sequential"]);
$b = $image->writeToBuffer(".png");

This is because sequential mode will stream pixels from the source to the destination and will never load the entire image. This means that you cannot use an image more than once -- after you've processed it, there are no pixels left.

There's a chapter in the docs with the background:

https://www.libvips.org/API/current/How-it-opens-files.html

tldr: sequential mode will be faster and use less memory, but you can't reuse images.

VipsConfig::cacheSetMax(13);

I'm not sure I'd call this at all. If you really want to change this value, I would do some careful experiments first.