Skip to content

HAVE_FPM_LQ is always defined #5530

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 3 commits into from
Closed

HAVE_FPM_LQ is always defined #5530

wants to merge 3 commits into from

Conversation

Girgias
Copy link
Member

@Girgias Girgias commented May 5, 2020

As can be seen HAVE_FPM_LQ is always defined: https://heap.space/xref/php-src/sapi/fpm/fpm/fpm_config.h?r=1ad08256#74

Therefore these #ifdef usages seems to be bogus.

@Girgias
Copy link
Member Author

Girgias commented May 5, 2020

2 tests on MacOS now fail:

sapi/fpm/tests/fpm_get_status_basic.phpt (FPM: Function fpm_get_status

002+ array(12) {
002- array(15) {
013-   ["listen-queue"]=>
014-   int(0)
015-   ["max-listen-queue"]=>
016-   int(0)
017-   ["listen-queue-len"]=>
018-   int(%d)
028+     array(13) {
034-     array(14) {
059-       ["last-request-cpu"]=>
060-       float(0)

sapi/fpm/tests/status-basic.phpt (FPM: Status basic test)

001+ ERROR: Expected body does not match pattern
001- Done
002+ BODY:
003+ string(302) "pool:                 unconfined
004+ process manager:      static
005+ start time:           05/May/2020:20:35:04 +0000
006+ start since:          0
007+ accepted conn:        2
008+ idle processes:       0
009+ active processes:     1
010+ total processes:      1
011+ max active processes: 1
012+ max children reached: 0
013+ slow requests:        0"
014+ PATTERN:
015+ string(345) "|pool:\s+unconfined
016+ process manager:\s+static
017+ start time:\s+\d+\/\w{3}\/\d{4}:\d{2}:\d{2}:\d{2}\s[+-]\d{4}
018+ start since:\s+\d+
019+ accepted conn:\s+\d+
020+ listen queue:\s+0
021+ max listen queue:\s+0
022+ listen queue len:\s+\d+
023+ idle processes:\s+0
024+ active processes:\s+1
025+ total processes:\s+1
026+ max active processes:\s+1
027+ max children reached:\s+0
028+ slow requests:\s+0|"
029+ ERROR: Expected body does not match pattern
030+ BODY:
031+ string(773) "<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Strict//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd">
032+ <html xmlns="http://www.w3.org/1999/xhtml" xml:lang="en" lang="en">
033+ <head><title>PHP-FPM Status Page</title></head>
034+ <body>
035+ <table>
036+ <tr><th>pool</th><td>unconfined</td></tr>
037+ <tr><th>process manager</th><td>static</td></tr>
038+ <tr><th>start time</th><td>05/May/2020:20:35:04 +0000</td></tr>
039+ <tr><th>start since</th><td>0</td></tr>
040+ <tr><th>accepted conn</th><td>3</td></tr>
041+ <tr><th>idle processes</th><td>0</td></tr>
042+ <tr><th>active processes</th><td>1</td></tr>
043+ <tr><th>total processes</th><td>1</td></tr>
044+ <tr><th>max active processes</th><td>1</td></tr>
045+ <tr><th>max children reached</th><td>0</td></tr>
046+ <tr><th>slow requests</th><td>0</td></tr>
047+ </table>
048+ </body></html>"
049+ PATTERN:
050+ string(932) "|<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Strict//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd">
051+ <html xmlns="http://www.w3.org/1999/xhtml" xml:lang="en" lang="en">
052+ <head><title>PHP-FPM Status Page</title></head>
053+ <body>
054+ <table>
055+ <tr><th>pool</th><td>unconfined</td></tr>
056+ <tr><th>process manager</th><td>static</td></tr>
057+ <tr><th>start time</th><td>\d+\/\w{3}\/\d{4}:\d{2}:\d{2}:\d{2}\s[+-]\d{4}</td></tr>
058+ <tr><th>start since</th><td>\d+</td></tr>
059+ <tr><th>accepted conn</th><td>\d+</td></tr>
060+ <tr><th>listen queue</th><td>0</td></tr>
061+ <tr><th>max listen queue</th><td>0</td></tr>
062+ <tr><th>listen queue len</th><td>\d+</td></tr>
063+ <tr><th>idle processes</th><td>0</td></tr>
064+ <tr><th>active processes</th><td>1</td></tr>
065+ <tr><th>total processes</th><td>1</td></tr>
066+ <tr><th>max active processes</th><td>1</td></tr>
067+ <tr><th>max children reached</th><td>0</td></tr>
068+ <tr><th>slow requests</th><td>0</td></tr>
069+ </table>
070+ </body></html>|"
071+ ERROR: Expected body does not match pattern
072+ BODY:
073+ string(443) "<?xml version="1.0" ?>
074+ <status>
075+ <pool>unconfined</pool>
076+ <process-manager>static</process-manager>
077+ <start-time>1588710904</start-time>
078+ <start-since>0</start-since>
079+ <accepted-conn>4</accepted-conn>
080+ <idle-processes>0</idle-processes>
081+ <active-processes>1</active-processes>
082+ <total-processes>1</total-processes>
083+ <max-active-processes>1</max-active-processes>
084+ <max-children-reached>0</max-children-reached>
085+ <slow-requests>0</slow-requests>
086+ </status>"
087+ PATTERN:
088+ string(555) "|<\?xml version="1.0" \?>
089+ <status>
090+ <pool>unconfined</pool>
091+ <process-manager>static</process-manager>
092+ <start-time>\d+</start-time>
093+ <start-since>\d+</start-since>
094+ <accepted-conn>\d+</accepted-conn>
095+ <listen-queue>0</listen-queue>
096+ <max-listen-queue>0</max-listen-queue>
097+ <listen-queue-len>\d+</listen-queue-len>
098+ <idle-processes>0</idle-processes>
099+ <active-processes>1</active-processes>
100+ <total-processes>1</total-processes>
101+ <max-ac

Which seems to be due that some feature which were enabled although not available on MacOS, what is the way forward here, skip on MacOS and make some dedicated for the platform?

nikic
nikic reviewed May 6, 2020
View reviewed changes
Copy link
Member

@nikic nikic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's check with @bukka. Problem is that the status functionality relating to "listen queue" feature was always enabled, even if it is not available.

As it has been like this for a while, maybe it's better to just drop those checks completely and always report?

sapi/fpm/fpm/fpm_status.c Outdated
@@ -522,7 +523,7 @@ int fpm_status_handle_request(void) /* {{{ */
}
}

#ifdef HAVE_FPM_LQ
#if HAVE_FPM_LQ
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder why this part is behind HAVE_FPM_LQ...

@bukka
Copy link
Member

bukka commented May 14, 2020

Yeah I would probably leave the items in status as they have been always there due to this bogus check. So agree with removing the checks completely.

@Girgias Girgias force-pushed the fpm-constant-always-defined branch from 97fcb44 to 4757323 Compare May 15, 2020 22:09
@Girgias
Copy link
Member Author

Girgias commented May 15, 2020

I dropped the preprocessing macro in the status file.

@bukka
Copy link
Member

bukka commented May 17, 2020

LGTM. Please squash and merge ;)

@php-pulls php-pulls closed this in f717ec6 May 17, 2020
@Girgias Girgias deleted the fpm-constant-always-defined branch May 17, 2020 20:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nikic nikic nikic left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Girgias @bukka @nikic