php / php-src

The PHP Interpreter
https://www.php.net
Other
38.17k stars 7.75k forks source link

CONF Var log_limit and fpm_log_write show error "the log buffer is full ..." #12302

Open 0bi-w6n-K3nobi opened 1 year ago

0bi-w6n-K3nobi commented 1 year ago

Description

Hello everybody.

Hum... I trying understand why so old bug/unexpected-behavior yet reside FPM software slice. I can see that Config Variable log_limit can be set, but ... none code exist for using it value!

Several and several reports at foruns... but no definitive solution for that. I'm crazy, or blind... or something like that.... But, for me ... a simple patch would be solve years of complaints about this almost "miserable" behavior in Log generation.

Yes... I looked into code for every version at Git... I be using 8.1, but lasted version seems the same problem.

So... Thinking simply, and perhaps stupid... My patch for that to would be:

--- a/fpm_log.c 2023-09-05 16:37:17.740605434 -0300
+++ b/fpm_log.c 2023-09-26 11:16:43.204109918 -0300
@@ -19,11 +19,19 @@
 #include "fastcgi.h"
 #include "zlog.h"

+#  Really?! MAX_LINE_LENGTH is no always defined...
+#     And Else conditional below limits everything!
+/*
 #ifdef MAX_LINE_LENGTH
 # define FPM_LOG_BUFFER MAX_LINE_LENGTH
 #else
 # define FPM_LOG_BUFFER 1024
 #endif
+*/
+
+#  Well.... It is not better code, but will be work...
+
+#define FPM_LOG_BUFFER  fpm_global_config.log_limit

 static char *fpm_log_format = NULL;
 static int fpm_log_fd = -1;
@@ -98,7 +106,7 @@
 int fpm_log_write(char *log_format) /* {{{ */
 {
    char *s, *b;
-   char buffer[FPM_LOG_BUFFER+1];
+   char *buffer = alloca(FPM_LOG_BUFFER+1);
    int token, test;
    size_t len, len2;
    struct fpm_scoreboard_proc_s proc, *proc_p;

Well... alloca function maybe not be too Standard in all system. And is possible too add assert() statement in case some fault. Or, use some like static buffer ... , that it will turn be on global with local access only.

Please... tell me that I'm wrong... Some solution is needed for this problem, that has been going on since version 7 (before ?).

bukka commented 1 year ago

The log_limit is currently used only for worker output buffer handled by zlog. We actually never allocate full limit unless the message is that long. The buffer is extended only to cover the longest mesage so limit can be set very high if people want it unlimited - theoretical it can be up to INT_MAX. So allocating this in the stack frame would not be exactly ideal.

I have got a plan to use zlog for access logs and use common logic for both which will have some other advantages.

piotrekkr commented 2 months ago

The log_limit is currently used only for worker output buffer handled by zlog. We actually never allocate full limit unless the message is that long. The buffer is extended only to cover the longest mesage so limit can be set very high if people want it unlimited - theoretical it can be up to INT_MAX. So allocating this in the stack frame would not be exactly ideal.

I have got a plan to use zlog for access logs and use common logic for both which will have some other advantages.

@bukka Hello. Did you create some PR for access log length config? I would happily vote on it if possible. Thanks

bukka commented 2 months ago

@piotrekkr No, I still didn't get to it. You can vote on this issue. I usually give a bit higher priority to the issues with higher number of :+1:

piotrekkr commented 2 months ago

@bukka Did not know that :+1: are taken into account. Good to know. Upvoted this issue. Thanks.

RoSk0 commented 1 month ago

Hi Team,

I've been wondering why my access logs are getting truncated despite log_limit being set quite high.

Documentation is poorly describes log_limit and it would be awesome if it would say that it is applicable only to error log. I can fix the documentation, but instead I would love to see this issue fixed and the patch from the description applied as that is a real problem, not documentation.