Page MenuHomePhabricator

Audit and sync INI settings as needed between HHVM and PHP 7
Closed, ResolvedPublic

Description

Following T121597#4652873 and T121597#4681823 I talked with @Joe and we agreed that we need to take a close look at our INI settings and make sure they are as identical as possible, with any differences either fixed or justified+documented.

With the very limited use of PHP 7 so far, we've already encountered one major difference which is that our current PHP 7 settings partially disable error_reporting, which is a problem. See also T187147 about that.


Todo from T211488#5102911:

  • Differences in APC settings.
  • Differences in Filesytem settings.
  • Differences in Serialisation settings.
  • Differences in Error handling.
  • Differences in File Uploads settings.
  • Differences in Session settings.
  • Differences in Mail settings.
  • Intl settings.
  • OPCache settings.
  • Redis settings.
  • Mysql settings.
  • Memcached settings.
  • Decide on PCRE JIT.
  • Decide on enable_dl. See T220681
  • Fix include_path.
  • Fix doc_root .
  • curl.namedPools
  • Hard limit
  • Stat cache

Related Objects

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes
Krinkle triaged this task as Normal priority.Dec 9 2018, 12:02 AM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptDec 9 2018, 12:02 AM
jcrespo added a subscriber: jcrespo.EditedJan 25 2019, 7:32 AM

Check T214248 - this is a known issue (unrelated to php7) but it is not considered a bug, just a configuration weakness (https://dev.mysql.com/doc/refman/8.0/en/load-data-local.html). Consider it enabling it on any client that doesn't use LOAD DATA LOCAL.

Joe added a comment.Jan 25 2019, 9:00 AM

After some digging:

  • HHVM has no error_reporting INI value set, meaning we use the default value, which is, according to my tests, 16807935. Please note this value cannot be obtained by combining the various error constants php offers. This is, according to https://github.com/facebook/hhvm/blob/master/hphp/runtime/base/runtime-error.h#L64-L76 corresponds roughly to E_ALL & ~E_STRICT in php, plus FATAL_ERROR which is a HHVM-only feature.
  • We don't set the value in PHP 7 either, and that means we've basically settled on leaving the ini setting to the default value of 22519, which is E_ALL & ~E_DEPRECATED & ~ E_STRICT & ~ E_NOTICE

So we probably want to set

error_reporting = E_ALL & ~E_STRICT; in php 7.x

Joe added a comment.Jan 25 2019, 10:38 AM

Confirmed that's the case, by running this code in CLI and in a browser

<?php
// This constant was introduced in HHVM 3.20
define('E_HHVM_FATAL_ERROR', 16777217);
if (defined('HHVM_VERSION')) {
	$hphp_all = error_reporting();
	error_reporting($hphp_all & ~E_HHVM_FATAL_ERROR);
} else {
        error_reporting(E_ALL & ~ E_STRICT);
}
var_dump(error_reporting());

I'll submit a patch shortly.

Change 486485 had a related patch set uploaded (by Giuseppe Lavagetto; owner: Giuseppe Lavagetto):
[operations/puppet@production] profile::mediawiki::php: conform error reporting levels to HHVM

https://gerrit.wikimedia.org/r/486485

Change 486485 merged by Giuseppe Lavagetto:
[operations/puppet@production] profile::mediawiki::php: conform error reporting levels to HHVM

https://gerrit.wikimedia.org/r/486485

Found this old issue - T157306, should that be closed, or still something we need/want for HHVM during the migration?

[..] a PHP Warning was logged directly into the HTML output stream as part of a regular response. This must never happen. Presumably, this is due to the display_errors INI settings from PHP7 being set incorrectly.

Tgr added a subscriber: Tgr.Feb 6 2019, 3:46 AM

Presumably, this is due to the display_errors INI settings from PHP7 being set incorrectly.

Yup.

tgr@mwdebug1002:~$ php7.2 -S localhost:8000 info.php 
PHP 7.2.8-1+0~20180725124257.2+stretch~1.gbp571e56 Development Server started at Wed Feb  6 03:43:07 2019
Listening on http://localhost:8000
Document root is /home/tgr
Press Ctrl-C to quit.
tgr@mwdebug1002:~$ curl -s 'http://localhost:8000' | grep display_errors
<tr><td class="e">display_errors</td><td class="v">On</td><td class="v">On</td></tr>

Change 497024 had a related patch set uploaded (by Krinkle; owner: MaxSem):
[operations/puppet@production] Disable display_errors in FPM mode

https://gerrit.wikimedia.org/r/497024

Change 497024 merged by Giuseppe Lavagetto:
[operations/puppet@production] profile::mediawiki::php: disable display_errors in FPM mode

https://gerrit.wikimedia.org/r/497024

Joe added a comment.Mar 25 2019, 5:08 PM

I just finished going through all settings.

Given a lot of settings don't correspond 1:1, or have no correspondence at all between the two application servers, only a few different settings could be verified.

There are only two clear outstanding differences, of which one is intentional and the other will be fixed soon:

  • HHVM uses a max_execution_time of 60 seconds on the appservers, which is then overridden with set_time_limit for individual requests. PHP has a less effective way to set wall-time timeouts, and we're using Excimer to reach a similar behaviour. The hard limit (set in php-fpm) is right now at 201 seconds for the application servers. Overall, the behaviour is expected to be similar
  • Mysql connection timeout is 3 seconds in HHVM (hhvm.mysql.connect_timeout) and 1 second in php (mysql.connect_timeout). This needs to be fixed.

Change 499143 had a related patch set uploaded (by Giuseppe Lavagetto; owner: Giuseppe Lavagetto):
[operations/puppet@production] profile::mediawiki::php: raise mysql.connect_timeout to 3 seconds

https://gerrit.wikimedia.org/r/499143

jijiki added a subscriber: jijiki.Mar 26 2019, 3:35 PM

Change 499143 merged by Giuseppe Lavagetto:
[operations/puppet@production] profile::mediawiki::php: raise mysql.connect_timeout to 3 seconds

https://gerrit.wikimedia.org/r/499143

Exported from mwdebug1001 in plain text and sorted. Full dumps at P8387 and P8386.

Differences

  • APC

This seems worth looking at. Especially because this is effectively a very major change to how APC works. On HHVM it has essentially unlimited space and no expiry or garbage collection. It's strictly TTL based. This is now going to be very different, and I would expect the defaults to be far too conservative for us.

  • Why is apc.enable_cli different. What do we want? Does it matter?
  • What did apc.stat do for us on HHVM?
  • Figure out good settings for segments, size, etc.
-apc.enable_cli = 1
+apc.enable_cli = 0

-apc.stat = 1

+apc.coredump_unmap = 0
+apc.entries_hint = 4096
+apc.gc_ttl = 3600
+apc.mmap_file_mask = 
+apc.preload_path = 
+apc.serializer = php
+apc.shm_segments = 1
+apc.shm_size = 32M
+apc.slam_defense = 0
+apc.smart = 0
+apc.ttl = 0
+apc.use_request_time = 1
  • Filesytem

Much lower. Don't know if it matters?

-default_socket_timeout = 60 # seconds
+default_socket_timeout = 1 # seconds
  • Intl

Decide whether to emit errors and/or throw exceptions when ICU functions encounter issues in PHP.

By default this is disabled, which seems typical for PHP functions. It means stuff returns null/false instead.

But, I couldn't find right away what HHVM does. Worth double-checking to see what we expect in MediaWiki and confirm that it is fine. Otherwise might need to update settings on PHP 7, or fix MW.

+intl.error_level = 0
+intl.use_exceptions = 0

Most are obvious defaults that HHVM simply did differently but in a way we don't have much choice over and seemed fine. (See bottom for details).

But, there were a few I think are worth thinking about:

  • opcache.validate_timestamps = 0 – Enabled by default, but disabled for us. Unsure why. The fact that this is currently disable for us, might explain why I'm editing files locally on mwdebug1001 usually does not result in it taking affect on PHP 7.2.
  • +opcache.force_restart_timeout = 180 – Skimmed the docs and if this controls discarding of opcache after a certain time, might be worth increasing. Especially for mwdebug.
  • +opcache.huge_code_pages = 0 – Allegedly a way to improve performance. Disabled by default because it is hard to detect whether the Kernel supports it. I imagine ours do. Might be useful.
  • +opcache.max_accelerated_files = 24,000 – No idea how many files we have in total on a multi-version deployment. Worth checking. What would happen if we were to set this to 100,000, or the max – 1,000,000?
  • +opcache.memory_consumption = 256 (megabytes) – Is this enough? I don't know.

A few more interesting ones:

+opcache.interned_strings_buffer = 8
+opcache.max_wasted_percentage = 10
+opcache.file_cache = 
+opcache.file_cache_consistency_checks = 1
  • enable_dl = 1 (php72 only)

https://www.php.net/manual/en/info.configuration.php#ini.enable-dl

Might make sense to disable dynamic enabling of extensions. ("The feature is deprecated, and will be removed in a future release".)

  • include_path

Should match.

-include_path = .:/usr/share/php
+include_path = .:/usr/share/php:/srv/mediawiki/php
  • doc_root

I don't know what this does for us. But would be nice to rule out as potential source of obscure issues by matching.

-doc_root = /srv/mediawiki/docroot/
+doc_root = 
  • Resource limits
-memory_limit = 524288000
+memory_limit = 500M

-max_execution_time = 60
-maximum_execution_time = 60

+max_execution_time = 180

Already talked about elsewhere. PHP 7 measures it differently from HHVM. We've fixed all timeout things in wmf-config/timeout.php.

  • Serialisation.

TODO.

Looks like we have the igbinary extension installed. Not sure why, looks cool though (docs). But seems like it doesn't do much by default. It has the ability to be enabled for session serialisation, and/or for APC serialisation. Afaik we don't though.

MediaWiki does has an ability to opt-in via MemcachedPeclBagOStuff.php and RedisConnectionPool.php, but afaik we don't do that currently. Those classes say "igbinary is slow if compact_strings is enabled".

The below value surprised me because in Puppet (manifests/mediawiki/php.pp) I see 'compact_strings' => 'Off', and yet, on mwdebug1001, it appears to be on. I checked both via web requests and via CLI.

+igbinary.compact_strings = 1

Also:

+serialize_precision = -1

+precision = 14

TODO: I don't know about these. Quick check suggests HHVM might be using 12 here. Impact?

  • Error handling

A few differences:

-display_errors = 1
+display_errors = 0

-error_log = 
+error_log = syslog

-error_reporting = 16807935
+error_reporting = 30719

-html_errors = 
+html_errors = 1

-log_errors = 
+log_errors = 1

+log_errors_max_len = 1024
  • File Uploads & Data Input
-upload_tmp_dir = /tmp
+upload_tmp_dir = 

+max_input_nesting_level = 64
+max_input_vars = 1000

+upload_max_filesize = 100M

+post_max_size = 100M
  • PCRE

Do we want the PCRE JIT to be on or off?

+pcre.jit = 1
  • Session

We don't use the default session storage in PHP. But, we do use some of its functions and legacy variables like $_SESSION. I don't know whether some of those might vary their behaviour through these.

Docs at https://www.php.net/manual/en/session.security.ini.php.

-session.save_path = 
+session.save_path = /tmp

-session.entropy_file = 
-session.entropy_length = 0
-session.hash_bits_per_character = 4
-session.hash_function = 0

+session.lazy_write = 1
+session.sid_bits_per_character = 4
+session.sid_length = 32
+session.trans_sid_hosts = 
+session.trans_sid_tags = a=href,area=href,frame=src,form=
+session.upload_progress.cleanup = 1
+session.upload_progress.enabled = 1
+session.upload_progress.freq = 1%
+session.upload_progress.min_freq = 1
+session.upload_progress.name = PHP_SESSION_UPLOAD_PROGRESS
+session.upload_progress.prefix = upload_progress_
+session.use_strict_mode = 0
  • Redis

No idea about this. How does HHVM behave? What is MW-as-deployed-at-WMF benefiting from currently of and possibly depending on?

+redis.arrays.autorehash = 0
+redis.arrays.connecttimeout = 0
+redis.arrays.distributor = 
+redis.arrays.functions = 
+redis.arrays.hosts = 
+redis.arrays.index = 0
+redis.arrays.lazyconnect = 0
+redis.arrays.names = 
+redis.arrays.pconnect = 0
+redis.arrays.previous = 
+redis.arrays.readtimeout = 0
+redis.arrays.retryinterval = 0
+redis.clusters.persistent = 0
+redis.clusters.read_timeout = 0
+redis.clusters.seeds = 
+redis.clusters.timeout = 0
+redis.session.lock_expire = 0
+redis.session.lock_retries = 10
+redis.session.lock_wait_time = 2000
+redis.session.locking_enabled = 0
  • Mysql & PDO

TODO

-mysqli.cache_size = 

+mysqli.rollback_on_cached_plink = 0

+mysqlnd.collect_memory_statistics = 0
+mysqlnd.collect_statistics = 1
+mysqlnd.debug = 
+mysqlnd.fetch_data_copy = 0
+mysqlnd.log_mask = 0
+mysqlnd.mempool_default_size = 16000
+mysqlnd.net_cmd_buffer_size = 4096
+mysqlnd.net_read_buffer_size = 32768
+mysqlnd.net_read_timeout = 86400
+mysqlnd.sha256_server_public_key = 
+mysqlnd.trace_alloc = 

-pdo_mysql.default_socket = 
+pdo_mysql.default_socket = /var/run/mysqld/mysqld.sock
  • Memcached

TODO.


-memcache.hash_function = crc32
-memcache.hash_strategy = standard
-memcached.sess_prefix = 
+memcached.compression_factor = 1.3
+memcached.compression_threshold = 2000
+memcached.compression_type = fastlz
+memcached.default_binary_protocol = 0
+memcached.default_connect_timeout = 0
+memcached.default_consistent_hash = 0
+memcached.serializer = php
+memcached.sess_binary_protocol = 1
+memcached.sess_connect_timeout = 0
+memcached.sess_consistent_hash = 1
+memcached.sess_lock_expire = 0
+memcached.sess_lock_max_wait = not set
+memcached.sess_lock_retries = 5
+memcached.sess_lock_wait = not set
+memcached.sess_lock_wait_max = 2000
+memcached.sess_lock_wait_min = 1000
+memcached.sess_locking = 1
+memcached.sess_number_of_replicas = 0
+memcached.sess_persistent = 0
+memcached.sess_prefix = memc.sess.
+memcached.sess_randomize_replica_read = 0
+memcached.sess_remove_failed_servers = 0
+memcached.sess_sasl_password = 
+memcached.sess_sasl_username = 
+memcached.sess_server_failure_limit = 0
+memcached.store_retry_count = 2
  • Mail

TODO.

-sendmail_path = /usr/lib/sendmail -t -i
+sendmail_path = /usr/sbin/sendmail -t -i 

  • HHVM specific features
-hhvm.xenon. …

-hhvm. …

-hphp.build_id = 
-hphp.compiler_id = 3.18.5+dfsg-1+wmf8+deb9u2
-hphp.compiler_version = 3.18.6-dev

-notice_frequency = 1 # hhvm.error_handling.notice_frequency
-warning_frequency = 1 # hhvm.error_handling.warning_frequency

-pid = 
  • Brotli

(Not installed on php72. Doesn't matter. )

-brotli.chunked_compression = 
-brotli.compression = 
-brotli.compression_lgwin = 20
-brotli.compression_quality = 6
  • curl.namedPools

This was HHVM-only. A replacement for this on PHP 7 is tracked at T210717.

-curl.namedPools = cirrus-eqiad,cirrus-codfw
-curl.namedPools.cirrus-codfw.connGetTimeout = 5000
-curl.namedPools.cirrus-codfw.reuseLimit = 100
-curl.namedPools.cirrus-codfw.size = 20
-curl.namedPools.cirrus-eqiad.connGetTimeout = 5000
-curl.namedPools.cirrus-eqiad.reuseLimit = 100
-curl.namedPools.cirrus-eqiad.size = 20
  • auto_prepend_file

(Matches HHVM already, wasn't picked up through ini_get for some reason.)

+auto_prepend_file = /srv/mediawiki/wmf-config/PhpAutoPrepend.php
  • Zend

All defaults. Only difference is that HHVM pretended zend.enable_gc was off. Seems fine to have on. It's there to be able to detect and garbage-collect circular references.

-zend.enable_gc = 
+zend.enable_gc = 1
+zend.detect_unicode = 1
+zend.multibyte = 0
+zend.script_encoding = 
+zend.signal_check = 0
  • Hard limit

This is a new thing, introduced in PHP 7.1.

+hard_timeout = 2 # in seconds

See also stackoverflow / php news.

In a nut shell, when max_execution_time is reached, it seems maybe in the past things got killed without hesitation which left things in a potentially bad state.

In PHP 7.1, this changed to allow things to clean up properly. Maybe things like destructors and shutdown callbacks? I don't know what exactly. But whatever it is, those are then still killed after 2 seconds (hard_timeout). Seems fine.

  • Stat cache

HHVM had its own stat cache which worked different (and wasn't configurable). I don't know if PHP 7 has one. But maybe that's fine, given the OS is meant to take care of that already?

PHP 7 does have tweaks for these, which are currently at their conservative defaults. I don't know off-hand where or how much MW makes use of these. From a quick grep, it seems core uses it very rarely only. Marking this as "OK" for now.

+realpath_cache_size = 4096K
+realpath_cache_ttl = 120
  • Everything else.

These are all the ones I checked already against HHVM, the PHP website docs, defaults and made a judgement call on that they seem fine. Most of these were php72-only with values set to defaults that match HHVM's implicit behaviour, or were similar but in different notation (e.g. "0" vs empty string).

Core:
+auto_append_file = 
+auto_globals_jit = 1
+disable_classes = 
+disable_functions = 
+register_argc_argv = 1
+short_open_tag = 1
+enable_post_data_reading = 1

-extension_dir = /usr/lib/x86_64-linux-gnu/hhvm/extensions
+extension_dir = /usr/lib/php/20170718

-default_charset = 
+default_charset = UTF-8
# the below are deprecated in php 5.6, and all use 'default_charset' now
+input_encoding = 
+internal_encoding = 
+output_encoding = 
+iconv.input_encoding = 
+iconv.internal_encoding = 
+iconv.output_encoding = 
-mbstring.http_input = pass
+mbstring.http_input = 
-mbstring.http_output = pass
+mbstring.http_output = 

+user_dir = # public_html/~name feature, disabled

+user_ini.filename = .user.ini # unused
+user_ini.cache_ttl = 300

CGI:
+cgi.discard_path = 0
+cgi.fix_pathinfo = 1
+cgi.force_redirect = 1
+cgi.nph = 0
+cgi.redirect_status_env =
+cgi.rfc2616_headers = 0

Exif:
+exif.encode_unicode = ISO-8859-15
+exif.decode_unicode_motorola = UCS-2BE
+exif.decode_unicode_intel = UCS-2LE
+exif.encode_jis = 
+exif.decode_jis_motorola = JIS
+exif.decode_jis_intel = JIS

Filesystem:
+allow_url_include = 0
+auto_detect_line_endings = 0
+from = # related to ftp connections

FPM:
+fpm.config = 

Misc:
+browscap = 

Error handling:
+docref_ext = 
+docref_root = 

-track_errors = 
+track_errors = 0

+display_startup_errors = 0
+error_append_string = 
+error_prepend_string = 
+ignore_repeated_errors = 0
+ignore_repeated_source = 0
+report_memleaks = 1
+report_zend_debug = 1

+xmlrpc_error_number = 0
+xmlrpc_errors = 0

Misc:
+ignore_user_abort = 0
+max_input_time = -1 # means to use max_execution_time 
+sys_temp_dir = 

Filter:
+filter.default = unsafe_raw
+filter.default_flags = 

MongoDB:
+mongodb.debug = 

Readline:
+cli.pager = 
+cli.prompt = \b \> 

cURL:
+curl.cainfo = 

DBA:
+dba.default_handler = flatfile

FastCGI:
+fastcgi.error_header = 
+fastcgi.logging = 1

GeoIP:
+geoip.custom_directory = 

Output Control:
+output_buffering = 0
+output_handler = 
+implicit_flush = 0
+url_rewriter.hosts = 
+url_rewriter.tags = form=

Image Processing:
-imagick.locale_fix = 
-imagick.progress_monitor = 
+gd.jpeg_ignore_warning = 1

PHP Info Behaviour:
+assert.callback = 
+assert.quiet_eval = 0

Phar:
+phar.cache_list = 
+phar.readonly = 1
+phar.require_hash = 1

Variable Handling:
+unserialize_callback_func = 

zlib:
-zlib.output_compression = 
+zlib.output_compression = 0
+zlib.output_handler = 

xdebug:
-xdebug.enable = 

session:
-session.auto_start = 
+session.auto_start = 0
-session.cookie_httponly = 
+session.cookie_httponly = 0
-session.cookie_secure = 
+session.cookie_secure = 0
-session.use_trans_sid = 
+session.use_trans_sid = 0

mbstring: (defaults)
+mbstring.language = neutral
+mbstring.detect_order = 
+mbstring.internal_encoding = 
+mbstring.substitute_character = 
+mbstring.func_overload = 0
+mbstring.encoding_translation = 0
+mbstring.http_output_conv_mimetypes = ^(text/|application/xhtml\+xml)
+mbstring.strict_detection = 0

opcache: (defaults)
+opcache.enable = 1
+opcache.enable_cli = 0
+opcache.use_cwd = 1
+opcache.revalidate_freq = 2
+opcache.revalidate_path = 0
+opcache.save_comments = 1
+opcache.enable_file_override = 0
+opcache.optimization_level = 0x7FFFBFFF
+opcache.inherited_hack = 1
+opcache.dups_fix = 0
+opcache.blacklist_filename = 
+opcache.max_file_size = 0
+opcache.consistency_checks = 0
+opcache.error_log = 
+opcache.log_verbosity_level = 1
+opcache.lockfile_path = /tmp
+opcache.preferred_memory_model = 
+opcache.protect_memory = 0
+opcache.file_cache_only = 0
+opcache.file_update_protection = 2
+opcache.restrict_api = 
+opcache.opt_debug_level = 0
+opcache.validate_permission = 0
+opcache.validate_root = 0

openssl:
+openssl.cafile = 
+openssl.capath = 

msgpack:
+msgpack.error_display = 1
+msgpack.illegal_key_insert = 0
+msgpack.php_only = 1
+msgpack.use_str8_serialization = 1

mysqli:
-mysqli.allow_local_infile = 
+mysqli.allow_local_infile = 0
-mysqli.reconnect = 
+mysqli.reconnect = 0

Mail:
+sendmail_from = 
+mail.add_x_header = 0
+mail.force_extra_parameters = 
+mail.log = 
+SMTP = localhost
+smtp_port = 25

Tideways:
+tideways_xhprof.clock_use_rdtsc = 0
Krinkle updated the task description. (Show Details)Apr 10 2019, 11:44 PM
Krinkle removed a project: Patch-For-Review.
Krinkle updated the task description. (Show Details)Apr 11 2019, 1:17 AM
Krinkle added a subscriber: aaron.

Per @aaron, the Redis settings don't affect us. Most of phpredis isn't INI-configurable, it's passed at run-time. The only parts of phpredis that are INI-configurable are parts we don't use (RedisArrays, RedisCluster, and a redis-based php-session handler).

Joe added a comment.Apr 11 2019, 5:37 AM

Exported from mwdebug1001 in plain text and sorted. Full dumps at P8387 and P8386.

    1. Differences
  • APC

This seems worth looking at. Especially because this is effectively a very major change to how APC works. On HHVM it has essentially unlimited space and no expiry or garbage collection. It's strictly TTL based. This is now going to be very different, and I would expect the defaults to be far too conservative for us.

  • Why is apc.enable_cli different. What do we want? Does it matter?
  • What did apc.stat do for us on HHVM?
  • Figure out good settings for segments, size, etc.
-apc.enable_cli = 1
+apc.enable_cli = 0
-apc.stat = 1
+apc.coredump_unmap = 0
+apc.entries_hint = 4096
+apc.gc_ttl = 3600
+apc.mmap_file_mask = 
+apc.preload_path = 
+apc.serializer = php
+apc.shm_segments = 1
+apc.shm_size = 32M
+apc.slam_defense = 0
+apc.smart = 0
+apc.ttl = 0
+apc.use_request_time = 1

Apc works in fundamentally different ways between HHVM and PHP. Also, we properly use APCu, not the old APC. So the only settings that actually do something are listed here
https://www.php.net/manual/en/apcu.configuration.php

The shm settings are in theory irrelevant as we use opcache for code. Ditto for apc.stat.

My approach at tuning apc is "look at the metrics". We're collecting apc metrics from PHP7 (you can see some of them in the https://grafana.wikimedia.org/d/000000550/mediawiki-application-servers dashboard and you can see all of them running php7adm on an appserver); as soon as we have more traffic we will surely be able to tune them.

As for cli, I assumed APC would be unneeded there as we don't run multi-process cli applications, and the manual strongly suggests it's a bad idea.

I would consider this checked.

Joe updated the task description. (Show Details)Apr 11 2019, 5:37 AM
Joe added a comment.Apr 11 2019, 6:20 AM
  • Filesytem

Much lower. Don't know if it matters?

-default_socket_timeout = 60 # seconds
+default_socket_timeout = 1 # seconds

It does for places where we just open raw sockets. The number in HHVM was quite high (equal to the request timeout for GETs), but I think it's ok anyways. I'll amend the php config.

  • Intl

Decide whether to emit errors and/or throw exceptions when ICU functions encounter issues in PHP.
By default this is disabled, which seems typical for PHP functions. It means stuff returns null/false instead.
But, I couldn't find right away what HHVM does. Worth double-checking to see what we expect in MediaWiki and confirm that it is fine. Otherwise might need to update settings on PHP 7, or fix MW.

+intl.error_level = 0
+intl.use_exceptions = 0

I'm pretty sure this was introduced in the 7.x series, and that HHVM doesn't report ICU errors. I would assume we need to go read the HHVM source code to know more.

Most are obvious defaults that HHVM simply did differently but in a way we don't have much choice over and seemed fine. (See bottom for details).
But, there were a few I think are worth thinking about:

  • opcache.validate_timestamps = 0 – Enabled by default, but disabled for us. Unsure why. The fact that this is currently disable for us, might explain why I'm editing files locally on mwdebug1001 usually does not result in it taking affect on PHP 7.2.
  • +opcache.force_restart_timeout = 180 – Skimmed the docs and if this controls discarding of opcache after a certain time, might be worth increasing. Especially for mwdebug.
  • +opcache.huge_code_pages = 0 – Allegedly a way to improve performance. Disabled by default because it is hard to detect whether the Kernel supports it. I imagine ours do. Might be useful.
  • +opcache.max_accelerated_files = 24,000 – No idea how many files we have in total on a multi-version deployment. Worth checking. What would happen if we were to set this to 100,000, or the max – 1,000,000?
  • +opcache.memory_consumption = 256 (megabytes) – Is this enough? I don't know.

There is a long ticket about opcache optimizations that includes benchmarking. We do not invalidate opcache to ensure consistency of the code and atomic releases. We do clear the opcache from scap, and you can do it locally on a machine by running php7adm /opcache-free.

A few more interesting ones:

+opcache.interned_strings_buffer = 8
+opcache.max_wasted_percentage = 10
+opcache.file_cache = 
+opcache.file_cache_consistency_checks = 1

And here again, we're collecting metrics and we will tune all opcache parameters accordingly when we get a bit more traffic. One to keep an eye on, as I didn't use it for my benchmarks, is opcache.huge_code_pages. I learned the hard way not to trust general wisdom (or Zend's reccomendations) on tuning.

Joe updated the task description. (Show Details)Apr 11 2019, 6:21 AM
Joe added a comment.Apr 11 2019, 6:34 AM

Error handling was discussed in https://phabricator.wikimedia.org/T211488#4908305 and the followups. The most notable difference is HHVM has display_errors on but it doesn't really do anything - I did set it to on in php7 after looking at what we had in HHVM and we had to roll it back.

Joe updated the task description. (Show Details)Apr 11 2019, 6:34 AM
Joe added a comment.Apr 11 2019, 7:42 AM

Also:

  • PCRE JIT is enabled by default in HHVM and we definitely want it in php7 as well.
  • include_path in php's ini is still set to the value it had in the old times before we migrated to HHVM. I think it's irrelevant nowadays for all applications and can be removed. I think it's actually potentially harmful.
  • doc_root is only relevant under CGI or if you activate safe_mode, which we don't use, according to https://www.php.net/manual/en/ini.core.php#ini.doc-root
Joe updated the task description. (Show Details)Apr 11 2019, 8:33 AM
Joe added a comment.Apr 11 2019, 8:43 AM

Also, the value of doc_root for HHVM doesn't seem to be set from ini settings, so I'll have to dig up where that happens.

Joe added a comment.Apr 11 2019, 8:50 AM

Regarding enable_dl: i just verified dl() does work under HHVM - although I suspect it does nothing. So I would be careful in disabling it but we should definitely do it at some point.

I'm opening a subtask to do it.

Joe updated the task description. (Show Details)Apr 11 2019, 8:53 AM
Joe added a comment.EditedApr 11 2019, 9:06 AM
  • File Uploads & Data Input
-upload_tmp_dir = /tmp
+upload_tmp_dir = 
+max_input_nesting_level = 64
+max_input_vars = 1000
+upload_max_filesize = 100M
+post_max_size = 100M

upload_tmp_dir uses the system's default tmp dir if not specified. That would be /tmp under linux.
The value of upload_max_filesize / post_max_size match the value of hhvm.server.max_post_size which is 100MB

  • Session

We don't use the default session storage in PHP. But, we do use some of its functions and legacy variables like $_SESSION. I don't know whether some of those might vary their behaviour through these.

I'll be honest, I had no idea either, at least for some of these settings. But from a read of the docs, I don't see how the security ones might have an impact.

What does have an impact is

session.upload_progress.enabled = 1

which will indeed interact with $_SESSION by adding a variable named

session.upload_progress.name = PHP_SESSION_UPLOAD_PROGRESS

in the session to track file uploads progress.

Do we want this to be disabled? I'm not sure, maybe it's potentially useful?

Joe updated the task description. (Show Details)Apr 11 2019, 9:06 AM
Joe updated the task description. (Show Details)Apr 11 2019, 9:16 AM
Joe added a comment.Apr 11 2019, 9:35 AM

Regarding mail: both paths actually link to /usr/sbin/exim4 on our systems.

Joe added a comment.Apr 11 2019, 9:53 AM

As far as serialisation goes:

  • Igbinary is installed because the memcached extension and (IIRC) the apcu extension have it as a requirement. I'm not sure if we use it anywhere
  • the compact_strings setting is wrong in our puppet code, thanks for catching that, I'm fixing it.
  • memcached serializer is set to php as that's the value that makes memcached interoperable between HHVM and php7.2 (after we patched HHVM already)
  • serialize_precision is AIUI 14 in our version of HHVM as well, see https://github.com/facebook/hhvm/blob/HHVM-3.18/hphp/runtime/base/variable-serializer.cpp#L264
  • precision matches as well but has nothing to do with serialization, and we don't work much with high-precision math, right?
Joe updated the task description. (Show Details)Apr 11 2019, 9:54 AM
Joe added a subscriber: elukey.Apr 11 2019, 11:13 AM

Looking at memcached settings with @elukey the only one that really seems potentially problematic is

memcached.store_retry_count = 2

which we want to set to 1 instead. I'm not sure it matters for us as we don't use the multiserver capabilities of the memcached extension anyways.

Re mysqli, we should care about:

+mysqlnd.net_cmd_buffer_size = 4096
+mysqlnd.net_read_buffer_size = 32768
+mysqlnd.net_read_timeout = 86400

I don't think we care about the others (although I remember in the past changing some long-running query timeouts?)

We should also introduce mysqli.allow_local_infile = 0, but I've been asked to do that on a separate task.

Note I am not sure if mysqli uses mysqlnd for this installation, so maybe those do nothing (needs checking on how it is being compiled).

Joe added a comment.Apr 11 2019, 12:21 PM

Re mysqli, we should care about:

+mysqlnd.net_cmd_buffer_size = 4096
+mysqlnd.net_read_buffer_size = 32768
+mysqlnd.net_read_timeout = 86400

I don't think we care about the others (although I remember in the past changing some long-running query timeouts?)
We should also introduce mysqli.allow_local_infile = 0, but I've been asked to do that on a separate task.

We already have that as the default setting, so I can just explicitly add it without the need of a separate task I guess.

Note I am not sure if mysqli uses mysqlnd for this installation, so maybe those do nothing (needs checking on how it is being compiled).

It does indeed. For those settings, I'd frankly use the defaults for now, and we can iteratively improve doing some benchmarking later, unless you see something insane in the default settings.

Joe updated the task description. (Show Details)Apr 11 2019, 12:26 PM

For those settings, I'd frankly use the defaults for now, and we can iteratively improve doing some benchmarking later, unless you see something insane in the default settings.

If the intention was to do a sweep to see grave issues first, those are indeed not problematic- normally you wouldn't change them. But I have been here enough time to see huge (not sane) query sizes, so I cannot discard they may need changes on edge cases. :-)

Change 502986 had a related patch set uploaded (by Giuseppe Lavagetto; owner: Giuseppe Lavagetto):
[operations/puppet@production] profile::mediawiki::php: tweak ini settings

https://gerrit.wikimedia.org/r/502986

aaron added a comment.Apr 11 2019, 5:38 PM
  • Session

We don't use the default session storage in PHP. But, we do use some of its functions and legacy variables like $_SESSION. I don't know whether some of those might vary their behaviour through these.

I'll be honest, I had no idea either, at least for some of these settings. But from a read of the docs, I don't see how the security ones might have an impact.
What does have an impact is

session.upload_progress.enabled = 1

which will indeed interact with $_SESSION by adding a variable named

session.upload_progress.name = PHP_SESSION_UPLOAD_PROGRESS

in the session to track file uploads progress.
Do we want this to be disabled? I'm not sure, maybe it's potentially useful?

Upload progress should just be disabled since we don't use it.

The other session INI changes look harmless to me.

Tgr added a comment.Apr 12 2019, 12:03 AM
  • include_path in php's ini is still set to the value it had in the old times before we migrated to HHVM. I think it's irrelevant nowadays for all applications and can be removed. I think it's actually potentially harmful.

Yeah, PEAR relies on include_path and a non-empty value can result in surprises. See T215224: PEAR PHP classes are loaded from system packages instead of Composer packages in WMF production.

@Joe Regarding opcache, I'm not sure why this change for manual reloading is applied now. Could we do that after the migration? The way it is with HHVM is that it checks at run-time via stat/mtimes. I assume that's the default on PHP 7 as well, when validate_timestamps is enabled (as it is by default).

The reason I'm concerned is because it will be confusing for a while for people's debugging workflows (and things are already confusing due to the migration). Also, I'm not entirely sure it will be an adequate substitute for T104352 because disabling validation does not mean we disable parsing of new files (it's not like HHVM RepoAuth). It just means we won't eagerly re-parse files we still have in cache. As such, we can still get Frankenstein-states during a deployment.

In any event, I think it's cool, worth considering, and you're probably right :) - but perhaps we can do that at a later time?

Joe added a comment.Apr 15 2019, 10:49 AM

@Joe Regarding opcache, I'm not sure why this change for manual reloading is applied now. Could we do that after the migration? The way it is with HHVM is that it checks at run-time via stat/mtimes. I assume that's the default on PHP 7 as well, when validate_timestamps is enabled (as it is by default).
The reason I'm concerned is because it will be confusing for a while for people's debugging workflows (and things are already confusing due to the migration). Also, I'm not entirely sure it will be an adequate substitute for T104352 because disabling validation does not mean we disable parsing of new files (it's not like HHVM RepoAuth). It just means we won't eagerly re-parse files we still have in cache. As such, we can still get Frankenstein-states during a deployment.
In any event, I think it's cool, worth considering, and you're probably right :) - but perhaps we can do that at a later time?

While I commented on this on IRC, it's worth reporting it here:

  • If we leave a revalidate_interval different than zero, we could end up with unpredictable results during a code deploy as scap deploys are not atomic.
  • scap thus now invalidates opcache once a release is done, and no other revalidation is done.
  • always revalidating doesn't really work and wouldn't scale. It's also a sizeable performance hit.

The best alternative I can think of is adding a relatively long revalidation period (like 5 minutes), and/or controlling revalidation externally running opcache_free() from a periodic job that could check if scap is running before doing anything.

Joe added a comment.Apr 15 2019, 1:17 PM

regarding enable_dl, I did some investigating:

  • The documentation on php.net says it's removed since php 7.0
  • dl() always returns false under hhvm as far as I can say.
  • It works under php 7.2, with no warning whatsoever. At least it does if one uses our debian package.
  • It returns an error under php7.3 in buster

So I'll definitely disable enable_dl now.

Krinkle moved this task from Backlog to Wikimedia production on the PHP 7.2 support board.

Change 502986 merged by Giuseppe Lavagetto:
[operations/puppet@production] profile::mediawiki::php: tweak ini settings

https://gerrit.wikimedia.org/r/502986

Joe added a comment.Apr 20 2019, 8:49 AM

FWIW, the doc_root being set was causing severe issues under php7.2. I removed it from the list of ini settings we want to use, and from the actual config as well.

Mentioned in SAL (#wikimedia-operations) [2019-04-24T10:23:08Z] <jijiki> Restarting php-fpm on mw1238 for 505383 and T211488

Mentioned in SAL (#wikimedia-operations) [2019-04-24T11:25:12Z] <jijiki> Restarting php7.2-fpm on mw-canary for 505383 and T211488

Mentioned in SAL (#wikimedia-operations) [2019-04-24T11:59:36Z] <jijiki> Restarting php7.2-fpm on mw12* for 505383 and T211488

Mentioned in SAL (#wikimedia-operations) [2019-04-24T13:01:21Z] <jijiki> Restarting php7.2-fpm on mw13* for 505383 and T211488

Mentioned in SAL (#wikimedia-operations) [2019-04-24T15:32:03Z] <jijiki> Restarting php7.2-fpm on mw2* in codfw for 505383 and T211488

jijiki updated the task description. (Show Details)

We have pushed https://gerrit.wikimedia.org/r/502986 and (its update) https://gerrit.wikimedia.org/r/505383/ to production. Are we going to proceed with session and mysql settings or should we mark this task as resolved?

Krinkle added a comment.EditedApr 25 2019, 9:23 PM

For sessions.

-session.save_path = 
+session.save_path = /tmp
-session.entropy_file = 
-session.entropy_length = 0
-session.hash_bits_per_character = 4
-session.hash_function = 0
+session.sid_bits_per_character = 4
+session.sid_length = 32
+session.trans_sid_hosts = 
+session.trans_sid_tags = a=href,area=href,frame=src,form=

These relate to the creation of new session IDs as create by session_start or session_regenerate_id, and returned by session_id.

The session.entropy_* settings that HHVM and PHP 5 had were empty which means using the default of /dev/urandom. These configs have been removed in PHP 7.1 and use whatever random-entropy source is appropriate on a given platform.

session.hash_bits_per_character effectively became session.sid_bits_per_character. And the others are all new options in PHP 7 that were previously not configurable but behave the same at least as far as PHP code can observe. (Possibly more secure or scalable defaults now).

In any event, I realized that MediaWiki's SessionManager has its own algorithm for generating session IDs, which we then inform PHP of, and use instead. This code lives in MediaWiki\SessionManager::generateSessionId().

+session.lazy_write = 1

Doesn't affect us afaik. Our custom SessionHandler seems to write all data from what I can see. Adding support for the lazy-write method as this PHP 7.0 feature does seems like a good idea, but setting or not setting this doesn't affect us.

See also https://wiki.php.net/rfc/session-read_only-lazy_write

+session.upload_progress.cleanup = 1
+session.upload_progress.enabled = 1
+session.upload_progress.freq = 1%
+session.upload_progress.min_freq = 1
+session.upload_progress.name = PHP_SESSION_UPLOAD_PROGRESS
+session.upload_progress.prefix = upload_progress_

The patch @Joe wrote did the one thing we cared about, which is disabling session.upload_progress.enabled. The rest seems fine.

+session.use_strict_mode = 0

Interesting (ini docs), and something to consider for later. But doesn't matter for right now. This is a new option in PHP 7 that allows changing a behaviour that previously was not configurable. The default is effectively what PHP5/HHVM did.

Krinkle closed this task as Resolved.Apr 25 2019, 9:27 PM
Krinkle claimed this task.
Krinkle updated the task description. (Show Details)
Krinkle updated the task description. (Show Details)

The mysql settings have been checked by aaron, jcrespo and joe.