shish/shimmie2

TimeoutException during upload and the problem of passing php.ini settings

Closed this issue · 5 comments

Is your feature request related to a problem? Please describe.
Coming back to #1058 and the problem ob how to pass PHP internal settings: I am still playing around with the newly added MAX_FILE_UPLOADS env variable (thank you again btw) and noticed the following:
When uploading either a large amount of small files, e.g. 500 .jpg files, or a small amount of larger video files, the upload sometimes fails due to a TimeoutException being thrown in core/send_event.php -> send_event():

2024/02/25 21:30:44 [notice] 26#26 [unit] #27: php message: Shimmie Error: Timeout while sending ThumbnailGenerationEvent (Query: )
#0 /app/ext/image/main.php(142): Shimmie2\send_event()
#1 /app/core/send_event.php(161): Shimmie2\ImageIO->onImageAddition()
#2 /app/core/extension.php(371): Shimmie2\send_event()
#3 /app/core/send_event.php(161): Shimmie2\DataHandlerExtension->onDataUpload()
#4 /app/ext/upload/main.php(316): Shimmie2\send_event()
#5 /app/core/database.php(143): Shimmie2\Upload->Shimmie2\{closure}()
#6 /app/ext/upload/main.php(315): Shimmie2\Database->with_savepoint()
#7 /app/ext/upload/main.php(239): Shimmie2\Upload->try_upload()
#8 /app/core/send_event.php(161): Shimmie2\Upload->onPageRequest()
#9 /app/index.php(91): Shimmie2\send_event()
#10 {main}

(ran into timeouts with other events in other tests as well, like trying to send MediaCheckPropertiesEvent when uploading a large amount of images, or MediaResizeEvent when uploading a few larger videos (~300MB each).

As far as I understand it, send_event() throws a TimeoutException based on _shm_timeout which is set by shm_set_timeout, which in turn is only called if PHP's max_execution_time setting is detected. The timeout kicks in after about 30s which is also the default for max_execution_time.

This shows me we may need a way to set max_execution_time. However, depending on what kind and amount of files the user wants to upload, we may run into other PHP limits like max_input_time, max_input_vars or max_multipart_body_parts...I can now see why you are considering to just hardcode limits entirely :)

Describe the solution you'd like

You mentioned the approach of accepting environment vars like PHP_ - I think this may be a flexible enough way to allow for the configuration of PHP settings without being tied to which webserver is currently being used. Question there is if it's worth it to write conversions for each and every possible php.ini setting, or if it's better to just leave it up to the admin not to misspell the part in any PHP_-environment vars they might use.

#1096 goes ahead and turns each PHP_INI_FOO_BAR=123 environment variable into into a foo_bar=123 php.ini setting - can you let me know if that covers all the cases you need?

(Minor update: the calculation of maximum total upload size was broken by the above change, #1101 fixes it)

Awesome thank you - I may not be able to test it before tomorrow, but this looks to be a nice solution

finally had time to test this:

  • this works for pretty much any PHP-ini setting I needed to allow more uploads - I tested with the following:
    • upload_max_filesize=1000M
    • max_file_uploads=1000
    • max_execution_time=180
    • post_max_size=1024M
    • max_memory_limit=2048M

One thing I noticed:
When I included PHP_INI_POST_MAX_SIZE=1024M, I kept getting "connection reset" errors when trying to upload even one image, as well as HTTP 413 (request too large) when e.g. pressing the "save" button in Board Config.

When I set PHP_INI_POST_MAX_SIZE=1073741824 (1024M in bytes), the limit is applied and everything works fine.

I see that you calculate post_max_size, if it is not set in the environment, by multiplying max_file_uploads and upload_max_filesize - run.php:22:

$php_ini['post_max_size'] ??= (string)(
    ini_parse_quantity($php_ini['upload_max_filesize']) *
    intval($php_ini['max_file_uploads'])
);

this makes sense as it always assumes a safe maximum post size. You use ini_parse_quantity() to parse the ini shorthand e.g. "1024M" into an int amount of bytes.

However, the above only runs if post_max_size is not explicitly set (which is fine), but that means that in run.php:94:

max_body_size" => (int)$php_ini['post_max_size'],

the value set for post_max_size is interpreted to int as is, meaning e.g. "1024M" is interpreted to int 1024, so max_body_size is set to 1024 bytes. Can be seen when checking max_body_size in the Unit config:

root@4d71b20cefca:/app# grep max_body_size /var/lib/unit/conf.json 
            "max_body_size": 1024,

I was able to fix it by using ini_parse_quantity() when setting max_body_size:

max_body_size" => ini_parse_quantity($php_ini['post_max_size']),

One last thing is there should maybe better handling of TimeoutException which may be thrown in core/send_even.php:155 - just to show an error the user may be able to understand better than the PHP stacktrace. This is just a cosmetic thing though and I think it's up to the admin to configure and test a proper max_execution_time (and probably max_input_time) - we could write up a small wiki page on these settings, but it can also get more complex depending on what users upload (think ffmpeg runtime, thumbnail generation, also proxy read timeout etc).

Anyways, thank you for this.

specifying post_max_size in shorthand format is now possible and overall I haven't noticed any drawbacks from defining PHP ini settings using PHP_INI_xxx environment variables. Looks like a nice and flexible approach to me.

Thanks for implementing this :)