From d71908ed55dc4346325c82ed921f7b10932ad3b0 Mon Sep 17 00:00:00 2001 From: Denis Vaksman Date: Mon, 11 Sep 2023 19:04:54 +0300 Subject: [PATCH 1/5] don't fetch net events right before script termination --- server/php-worker.cpp | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/server/php-worker.cpp b/server/php-worker.cpp index dc13dadb45..7df734af84 100644 --- a/server/php-worker.cpp +++ b/server/php-worker.cpp @@ -6,6 +6,7 @@ #include #include +#include "common/algorithms/find.h" #include "common/precise-time.h" #include "common/rpc-error-codes.h" #include "common/wrappers/overloaded.h" @@ -233,7 +234,11 @@ void PhpWorker::state_run() noexcept { tvkprintf(php_runner, 3, "PHP-worker before swap context [req_id = %016llx]\n", req_id); php_script->iterate(); tvkprintf(php_runner, 3, "PHP-worker after swap context [req_id = %016llx]\n", req_id);; - wait(0); // check for net events + if (!vk::any_of_equal(php_script->state, run_state_t::finished, run_state_t::error)) { + // We don't need to check net events when the script is going to finish. + // Otherwise we can fetch some net events related to this script that will be processed after the script termination. + wait(0); + } break; } case run_state_t::query: { From 80dfd8ad7382d70a4cb1c3e34ad603c1be6315e9 Mon Sep 17 00:00:00 2001 From: Denis Vaksman Date: Thu, 2 Nov 2023 13:14:01 +0300 Subject: [PATCH 2/5] refactored script memory stats collecting --- server/php-runner.cpp | 3 +-- server/server-stats.cpp | 13 ++++++------- server/server-stats.h | 5 +++-- server/statshouse/statshouse-manager.cpp | 8 ++++---- server/statshouse/statshouse-manager.h | 2 +- 5 files changed, 15 insertions(+), 16 deletions(-) diff --git a/server/php-runner.cpp b/server/php-runner.cpp index e1065953ee..bf6d73f304 100644 --- a/server/php-runner.cpp +++ b/server/php-runner.cpp @@ -319,8 +319,7 @@ void PhpScript::finish() noexcept { process_rusage_t script_rusage = get_script_rusage(); vk::singleton::get().add_request_stats(script_time, net_time, script_init_time_sec, connection_process_time_sec, - queries_cnt, long_queries_cnt, script_mem_stats.max_memory_used, - script_mem_stats.max_real_memory_used, vk::singleton::get().total_allocated, script_rusage, error_type); + queries_cnt, long_queries_cnt, script_mem_stats, vk::singleton::get().total_allocated, script_rusage, error_type); if (save_state == run_state_t::error) { assert (error_message != nullptr); kprintf("Critical error during script execution: %s\n", error_message); diff --git a/server/server-stats.cpp b/server/server-stats.cpp index df960686dc..97d8465bb8 100644 --- a/server/server-stats.cpp +++ b/server/server-stats.cpp @@ -302,7 +302,7 @@ struct WorkerSharedStats : private vk::not_copyable { } void add_request_stats(const EnumTable &queries, script_error_t error, - uint64_t memory_used, uint64_t real_memory_used, uint64_t curl_total_allocated) noexcept { + const memory_resource::MemoryStats &script_memory_stats, uint64_t curl_total_allocated) noexcept { errors[static_cast(error)].fetch_add(1, std::memory_order_relaxed); for (size_t i = 0; i != queries.size(); ++i) { @@ -310,8 +310,8 @@ struct WorkerSharedStats : private vk::not_copyable { } EnumTable sample; - sample[ScriptSamples::Key::memory_used] = memory_used; - sample[ScriptSamples::Key::real_memory_used] = real_memory_used; + sample[ScriptSamples::Key::memory_used] = script_memory_stats.memory_used; + sample[ScriptSamples::Key::real_memory_used] = script_memory_stats.real_memory_used; sample[ScriptSamples::Key::total_allocated_by_curl] = curl_total_allocated; sample[ScriptSamples::Key::outgoing_queries] = queries[QueriesStat::Key::outgoing_queries]; sample[ScriptSamples::Key::outgoing_long_queries] = queries[QueriesStat::Key::outgoing_long_queries]; @@ -635,8 +635,7 @@ void ServerStats::after_fork(pid_t worker_pid, uint64_t active_connections, uint } void ServerStats::add_request_stats(double script_time_sec, double net_time_sec, double script_init_time_sec, double connection_process_time_sec, - int64_t script_queries, int64_t long_script_queries, int64_t memory_used, - int64_t real_memory_used, int64_t curl_total_allocated, process_rusage_t script_rusage, script_error_t error) noexcept { + int64_t script_queries, int64_t long_script_queries, const memory_resource::MemoryStats &script_memory_stats, int64_t curl_total_allocated, process_rusage_t script_rusage, script_error_t error) noexcept { auto &stats = worker_type_ == WorkerType::job_worker ? shared_stats_->job_workers : shared_stats_->general_workers; const auto script_time = std::chrono::duration_cast(std::chrono::duration(script_time_sec)); const auto net_time = std::chrono::duration_cast(std::chrono::duration(net_time_sec)); @@ -650,10 +649,10 @@ void ServerStats::add_request_stats(double script_time_sec, double net_time_sec, script_rusage.voluntary_context_switches, script_rusage.involuntary_context_switches); - stats.add_request_stats(queries_stat, error, memory_used, real_memory_used, curl_total_allocated); + stats.add_request_stats(queries_stat, error, script_memory_stats, curl_total_allocated); shared_stats_->workers.add_worker_stats(queries_stat, worker_process_id_); - StatsHouseManager::get().add_request_stats(script_time.count(), net_time.count(), error, memory_used, real_memory_used, script_queries, long_script_queries, + StatsHouseManager::get().add_request_stats(script_time.count(), net_time.count(), error, script_memory_stats, script_queries, long_script_queries, script_user_time.count(), script_system_time.count(), script_init_time.count(), http_connection_process_time.count(), script_rusage.voluntary_context_switches, script_rusage.involuntary_context_switches); diff --git a/server/server-stats.h b/server/server-stats.h index 9e0990526c..b637f6b5b5 100644 --- a/server/server-stats.h +++ b/server/server-stats.h @@ -12,6 +12,7 @@ #include "common/smart_ptrs/singleton.h" #include "common/stats/provider.h" +#include "runtime/memory_resource/memory_resource.h" #include "server/php-runner.h" #include "server/workers-control.h" @@ -20,8 +21,8 @@ class ServerStats : vk::not_copyable { void init() noexcept; void add_request_stats(double script_time_sec, double net_time_sec, double script_init_time_sec, double connection_process_time_sec, - int64_t script_queries, int64_t long_script_queries, int64_t memory_used, - int64_t real_memory_used, int64_t curl_total_allocated, + int64_t script_queries, int64_t long_script_queries, + const memory_resource::MemoryStats &script_memory_stats, int64_t curl_total_allocated, process_rusage_t script_rusage, script_error_t error) noexcept; void add_job_stats(double job_wait_time_sec, int64_t request_memory_used, int64_t request_real_memory_used, int64_t response_memory_used, int64_t response_real_memory_used) noexcept; diff --git a/server/statshouse/statshouse-manager.cpp b/server/statshouse/statshouse-manager.cpp index e16c30cd59..3b937b2001 100644 --- a/server/statshouse/statshouse-manager.cpp +++ b/server/statshouse/statshouse-manager.cpp @@ -95,8 +95,8 @@ void StatsHouseManager::generic_cron_check_if_tag_host_needed() { } } -void StatsHouseManager::add_request_stats(uint64_t script_time_ns, uint64_t net_time_ns, script_error_t error, uint64_t memory_used, - uint64_t real_memory_used, uint64_t script_queries, uint64_t long_script_queries, +void StatsHouseManager::add_request_stats(uint64_t script_time_ns, uint64_t net_time_ns, script_error_t error, + const memory_resource::MemoryStats &script_memory_stats, uint64_t script_queries, uint64_t long_script_queries, uint64_t script_user_time_ns, uint64_t script_system_time_ns, uint64_t script_init_time, uint64_t http_connection_process_time, uint64_t voluntary_context_switches, uint64_t involuntary_context_switches) { @@ -126,8 +126,8 @@ void StatsHouseManager::add_request_stats(uint64_t script_time_ns, uint64_t net_ client.metric("kphp_by_host_request_errors", true).tag(status).tag(worker_type).write_count(1); } - client.metric("kphp_memory_script_usage").tag("used").tag(worker_type).write_value(memory_used); - client.metric("kphp_memory_script_usage").tag("real_used").tag(worker_type).write_value(real_memory_used); + client.metric("kphp_memory_script_usage").tag("used").tag(worker_type).write_value(script_memory_stats.memory_used); + client.metric("kphp_memory_script_usage").tag("real_used").tag(worker_type).write_value(script_memory_stats.real_memory_used); client.metric("kphp_requests_outgoing_queries").tag(worker_type).write_value(script_queries); client.metric("kphp_requests_outgoing_long_queries").tag(worker_type).write_value(long_script_queries); diff --git a/server/statshouse/statshouse-manager.h b/server/statshouse/statshouse-manager.h index 2fbf78a5d4..7957a04681 100644 --- a/server/statshouse/statshouse-manager.h +++ b/server/statshouse/statshouse-manager.h @@ -42,7 +42,7 @@ class StatsHouseManager : vk::not_copyable { need_write_enable_tag_host = true; } - void add_request_stats(uint64_t script_time_ns, uint64_t net_time_ns, script_error_t error, uint64_t memory_used, uint64_t real_memory_used, + void add_request_stats(uint64_t script_time_ns, uint64_t net_time_ns, script_error_t error, const memory_resource::MemoryStats &script_memory_stats, uint64_t script_queries, uint64_t long_script_queries, uint64_t script_user_time_ns, uint64_t script_system_time_ns, uint64_t script_init_time, uint64_t http_connection_process_time, From ae44f7cf6141f786f775f4cfadece5a083835c9a Mon Sep 17 00:00:00 2001 From: Denis Vaksman Date: Thu, 2 Nov 2023 13:29:13 +0300 Subject: [PATCH 3/5] added total allocations metrics to StatsHouse and statsd --- server/server-stats.cpp | 6 ++++++ server/statshouse/statshouse-manager.cpp | 3 +++ 2 files changed, 9 insertions(+) diff --git a/server/server-stats.cpp b/server/server-stats.cpp index 97d8465bb8..e45cece4e5 100644 --- a/server/server-stats.cpp +++ b/server/server-stats.cpp @@ -33,6 +33,8 @@ struct ScriptSamples : WithStatType { enum class Key { memory_used = 0, real_memory_used, + memory_allocated_total, + memory_allocations_count, total_allocated_by_curl, outgoing_queries, outgoing_long_queries, @@ -312,6 +314,8 @@ struct WorkerSharedStats : private vk::not_copyable { EnumTable sample; sample[ScriptSamples::Key::memory_used] = script_memory_stats.memory_used; sample[ScriptSamples::Key::real_memory_used] = script_memory_stats.real_memory_used; + sample[ScriptSamples::Key::memory_allocated_total] = script_memory_stats.total_memory_allocated; + sample[ScriptSamples::Key::memory_allocations_count] = script_memory_stats.total_allocations; sample[ScriptSamples::Key::total_allocated_by_curl] = curl_total_allocated; sample[ScriptSamples::Key::outgoing_queries] = queries[QueriesStat::Key::outgoing_queries]; sample[ScriptSamples::Key::outgoing_long_queries] = queries[QueriesStat::Key::outgoing_long_queries]; @@ -788,6 +792,8 @@ void write_to(stats_t *stats, const char *prefix, const WorkerAggregatedStats &a write_to(stats, prefix, ".requests.working_time", agg.script_samples[ScriptSamples::Key::working_time], ns2double); write_to(stats, prefix, ".memory.script_usage", agg.script_samples[ScriptSamples::Key::memory_used]); write_to(stats, prefix, ".memory.script_real_usage", agg.script_samples[ScriptSamples::Key::real_memory_used]); + write_to(stats, prefix, ".memory.script_allocated_total", agg.script_samples[ScriptSamples::Key::memory_allocated_total]); + write_to(stats, prefix, ".memory.script_allocations_count", agg.script_samples[ScriptSamples::Key::memory_allocations_count]); write_to(stats, prefix, ".memory.script_total_allocated_by_curl", agg.script_samples[ScriptSamples::Key::total_allocated_by_curl]); write_to(stats, prefix, ".memory.currently_script_heap_usage_bytes", agg.heap_samples[HeapStat::Key::script_heap_memory_usage]); diff --git a/server/statshouse/statshouse-manager.cpp b/server/statshouse/statshouse-manager.cpp index 3b937b2001..3ad25aed29 100644 --- a/server/statshouse/statshouse-manager.cpp +++ b/server/statshouse/statshouse-manager.cpp @@ -129,6 +129,9 @@ void StatsHouseManager::add_request_stats(uint64_t script_time_ns, uint64_t net_ client.metric("kphp_memory_script_usage").tag("used").tag(worker_type).write_value(script_memory_stats.memory_used); client.metric("kphp_memory_script_usage").tag("real_used").tag(worker_type).write_value(script_memory_stats.real_memory_used); + client.metric("kphp_memory_script_allocated_total").tag(worker_type).write_value(script_memory_stats.total_memory_allocated); + client.metric("kphp_memory_script_allocations_count").tag(worker_type).write_value(script_memory_stats.total_allocations); + client.metric("kphp_requests_outgoing_queries").tag(worker_type).write_value(script_queries); client.metric("kphp_requests_outgoing_long_queries").tag(worker_type).write_value(long_script_queries); From a91434670c471b9168f67604fbcfc4db1bdec70f Mon Sep 17 00:00:00 2001 From: Denis Vaksman Date: Thu, 2 Nov 2023 19:45:38 +0300 Subject: [PATCH 4/5] removed maximum script timeout limit --- server/php-engine-vars.h | 1 - server/php-engine.cpp | 6 +----- server/php-runner.cpp | 4 ---- 3 files changed, 1 insertion(+), 10 deletions(-) diff --git a/server/php-engine-vars.h b/server/php-engine-vars.h index e191e794a8..e1cd9c47d8 100644 --- a/server/php-engine-vars.h +++ b/server/php-engine-vars.h @@ -19,7 +19,6 @@ ***/ #define DEFAULT_SCRIPT_TIMEOUT 30 -#define MAX_SCRIPT_TIMEOUT (60 * 7) enum class ProcessType { master, http_worker, rpc_worker, job_worker }; diff --git a/server/php-engine.cpp b/server/php-engine.cpp index 4610dd7f08..fcda82b415 100644 --- a/server/php-engine.cpp +++ b/server/php-engine.cpp @@ -872,13 +872,9 @@ static bool check_tasks_manager_pid(process_id_t tasks_manager_pid) { static double normalize_script_timeout(double timeout_sec) { if (timeout_sec < 1) { - kprintf("Too small script timeout: %f sec, should be [%d..%d] sec", timeout_sec, 1, MAX_SCRIPT_TIMEOUT); + kprintf("Too small script timeout: %f sec, should be at least 1 sec\n", timeout_sec); return 1; } - if (timeout_sec > MAX_SCRIPT_TIMEOUT) { - kprintf("Too big script timeout: %f sec, should be [%d..%d] sec", timeout_sec, 1, MAX_SCRIPT_TIMEOUT); - return MAX_SCRIPT_TIMEOUT; - } return timeout_sec; } diff --git a/server/php-runner.cpp b/server/php-runner.cpp index bf6d73f304..bee0c05432 100644 --- a/server/php-runner.cpp +++ b/server/php-runner.cpp @@ -513,10 +513,6 @@ void PhpScript::set_timeout(double t) noexcept { disable_timeout(); static itimerval timer; - if (t > MAX_SCRIPT_TIMEOUT) { - return; - } - int sec = (int)t, usec = (int)((t - sec) * 1000000); timer.it_value.tv_sec = sec; timer.it_value.tv_usec = usec; From 2d4cb8d8a2b6171506af62fddd774c00191c0f2c Mon Sep 17 00:00:00 2001 From: Denis Vaksman Date: Thu, 2 Nov 2023 20:11:21 +0300 Subject: [PATCH 5/5] make `disable-sql` option deprecated and turned on unless SQL port is given --- server/php-engine-vars.cpp | 2 -- server/php-engine-vars.h | 2 -- server/php-engine.cpp | 12 +++--------- tests/python/lib/kphp_run_once.py | 2 +- tests/python/lib/kphp_server.py | 1 - 5 files changed, 4 insertions(+), 15 deletions(-) diff --git a/server/php-engine-vars.cpp b/server/php-engine-vars.cpp index 4182097324..e94d0b39fc 100644 --- a/server/php-engine-vars.cpp +++ b/server/php-engine-vars.cpp @@ -23,8 +23,6 @@ double oom_handling_memory_ratio = 0.00; int worker_id = -1; int pid = -1; -int no_sql = 0; - ProcessType process_type = ProcessType::master; int master_flag = 0; // 1 -- master, 0 -- single process, -1 -- child diff --git a/server/php-engine-vars.h b/server/php-engine-vars.h index e1cd9c47d8..37f33ae9ec 100644 --- a/server/php-engine-vars.h +++ b/server/php-engine-vars.h @@ -33,8 +33,6 @@ extern double oom_handling_memory_ratio; extern int worker_id; extern int pid; -extern int no_sql; - extern ProcessType process_type; extern int master_flag; diff --git a/server/php-engine.cpp b/server/php-engine.cpp index fcda82b415..9355d1e786 100644 --- a/server/php-engine.cpp +++ b/server/php-engine.cpp @@ -133,7 +133,7 @@ tcp_rpc_client_functions tcp_rpc_client_outbound = [] { OUTBOUND CONNECTIONS ***/ -static int db_port = 3306; +static int db_port = -1; static const char *db_host = "localhost"; conn_type_t ct_tcp_rpc_client_read_all = [] { @@ -1518,9 +1518,7 @@ void generic_event_loop(WorkerType worker_type, bool init_and_listen_rpc_port) n vk::singleton::get().init(logname_id); } - if (no_sql) { - sql_target_id = -1; - } else { + if (db_port != -1) { vkprintf(1, "mysql host: %s; port: %d\n", db_host, db_port); sql_target_id = get_target(db_host, db_port, &db_ct); assert (sql_target_id != -1); @@ -1866,10 +1864,6 @@ int main_args_handler(int i, const char *long_option) { } return 0; } - case 'q': { - no_sql = 1; - return 0; - } case 'Q': { return read_option_to(long_option, 1000, 0xffff, db_port); } @@ -2227,6 +2221,7 @@ DEPRECATED_OPTION("use-unix", no_argument); DEPRECATED_OPTION_SHORT("json-log", "j", no_argument); DEPRECATED_OPTION_SHORT("crc32c", "C", no_argument); DEPRECATED_OPTION_SHORT("tl-schema", "T", required_argument); +DEPRECATED_OPTION_SHORT("disable-sql", "q", no_argument); void parse_main_args(int argc, char *argv[]) { usage_set_other_args_desc(""); @@ -2242,7 +2237,6 @@ void parse_main_args(int argc, char *argv[]) { parse_option("rpc-client", required_argument, 'w', "host and port for client mode (host:port)"); parse_option("hard-memory-limit", required_argument, 'm', "maximal size of memory used by script"); parse_option("force-clear-sql", no_argument, 'R', "force clear sql connection every script run"); - parse_option("disable-sql", no_argument, 'q', "disable using sql"); parse_option("sql-port", required_argument, 'Q', "sql port"); parse_option("static-buffers-size", required_argument, 'L', "limit for static buffers length (e.g. limits script output size)"); parse_option("error-tag", required_argument, 'E', "name of file with engine tag showed on every warning"); diff --git a/tests/python/lib/kphp_run_once.py b/tests/python/lib/kphp_run_once.py index ba078a8fe8..485b4925bd 100644 --- a/tests/python/lib/kphp_run_once.py +++ b/tests/python/lib/kphp_run_once.py @@ -99,7 +99,7 @@ def run_with_kphp(self, runs_cnt=1): sanitizer_log_name = "kphp_runtime_sanitizer_log" env, sanitizer_glob_mask = self._prepare_sanitizer_env(self._kphp_runtime_tmp_dir, sanitizer_log_name) - cmd = [self._kphp_runtime_bin, "--once={}".format(runs_cnt), "--disable-sql", "--profiler-log-prefix", "profiler.log", + cmd = [self._kphp_runtime_bin, "--once={}".format(runs_cnt), "--profiler-log-prefix", "profiler.log", "--worker-queries-to-reload", "1"] if not os.getuid(): cmd += ["-u", "root", "-g", "root"] diff --git a/tests/python/lib/kphp_server.py b/tests/python/lib/kphp_server.py index 219d996324..e2667f05cd 100644 --- a/tests/python/lib/kphp_server.py +++ b/tests/python/lib/kphp_server.py @@ -27,7 +27,6 @@ def __init__(self, engine_bin, working_dir, options=None, auto_start=False): self._options["--master-name"] = \ "kphp_server_{}".format("".join(chr(ord('A') + int(c)) for c in str(self._http_port))) self._options["--server-config"] = self.get_server_config_path() - self._options["--disable-sql"] = True self._options["--workers-num"] = 1 self._options["--allow-loopback"] = None self._options["--dump-next-queries"] = None