Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1pS6fR-0007yD-H3 for pgsql-hackers@arkaria.postgresql.org; Wed, 15 Feb 2023 01:33:05 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.92) (envelope-from ) id 1pS6fP-0004gc-SX for pgsql-hackers@arkaria.postgresql.org; Wed, 15 Feb 2023 01:33:03 +0000 Received: from magus.postgresql.org ([2a02:c0:301:0:ffff::29]) by malur.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1pS6fP-0004gL-Gg for pgsql-hackers@lists.postgresql.org; Wed, 15 Feb 2023 01:33:03 +0000 Received: from mail-il1-x134.google.com ([2607:f8b0:4864:20::134]) by magus.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.92) (envelope-from ) id 1pS6fM-0005zN-Fg for pgsql-hackers@postgresql.org; Wed, 15 Feb 2023 01:33:03 +0000 Received: by mail-il1-x134.google.com with SMTP id h5so1163765ilq.6 for ; Tue, 14 Feb 2023 17:33:00 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=telsasoft-com.20210112.gappssmtp.com; s=20210112; h=user-agent:in-reply-to:content-disposition:mime-version:references :message-id:subject:cc:to:from:date:from:to:cc:subject:date :message-id:reply-to; bh=jyI0cRc57zlpQPY+dKttqGXibwfd6kNKcwHn8aOKYog=; b=ePwrBtnNVAsKuVZsuCDcvnhysVOa0KbiEoMcI918UCXhffLMao0F79+x71b4xXzATd tmLPRG0HONa8KZne3hFCyt73kynFAJvw0KsrtVHzACYNuK7LqUgnd/xOjp4JAekWEKEd o16eJl+eVQv/cgLvM0A9H4Rv2yLl0FLYrfpUrsZue1XAz/2YjmgrREsN8lnTWiiMOe2+ zeqTi9SNif7+66ufyePdkjeeFSPqDHE2m4vZ00fPYmsSJMQy1jZkdz8yyWhqfZ/ypSlt 8fzHXReW46hWWv9YwGspAqTB2CveETnURdO27otwNu0rF3+AecRxhuzoc+KFV+gGVeEl fUlw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=user-agent:in-reply-to:content-disposition:mime-version:references :message-id:subject:cc:to:from:date:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=jyI0cRc57zlpQPY+dKttqGXibwfd6kNKcwHn8aOKYog=; b=8J/GjjjV6PsWSD+55Rq1akFEmgG9zTUseuv1DcR+cbgBqrTBnwiErDgPj6ibEkUbGd taII06BD+YOyGXR6hagjjWIZo9yFQcxUF7pMF6OfwHKh33p6yFsRx2AYzmB1jNe+LC1M P9LqIK2HrKM1pAOGrhqKFUrXbi0OMvxsXtRB0nB4A8kGCGxB5mIh9UeLFKeVcPqcWpvU LMSyCsAh5KQS95DVcowKbEeS2O7Wok1nrvylz810DFOQaLCYOLpCqeGQSNJQW4jb1imN ic6+H9joMQDPfv08uZTQd1H4U513H2XltXhwy4BDVzZASeQ8h1nvf9IyZGv2euInGrj+ vNIA== X-Gm-Message-State: AO0yUKVmNzSjQ6yudAjYQqVm/RLmse/8d2n6t59WwDFO2GQUkMOFstQ8 /KeO6c7ZgYhBKuOd/tJZvo8ZSQ== X-Google-Smtp-Source: AK7set8l0yVx4PC/xLZK081lgjO9CuuIZF75sqWl8l7aLNVTHvp0A13kv15rUNGq1F0txMFbe5Ak0Q== X-Received: by 2002:a05:6e02:2144:b0:312:7a4e:e94d with SMTP id d4-20020a056e02214400b003127a4ee94dmr686923ilv.2.1676424778239; Tue, 14 Feb 2023 17:32:58 -0800 (PST) Received: from pryzbyj.telsasoft (charmander.telsasoft.com. [50.244.222.1]) by smtp.gmail.com with ESMTPSA id e7-20020a02a787000000b003c4e1bfbab8sm730439jaj.44.2023.02.14.17.32.57 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 14 Feb 2023 17:32:57 -0800 (PST) Received: by pryzbyj.telsasoft (Postfix, from userid 1000) id 90C82800957; Tue, 14 Feb 2023 19:32:56 -0600 (CST) Date: Tue, 14 Feb 2023 19:32:56 -0600 From: Justin Pryzby To: Nathan Bossart Cc: Alvaro Herrera , Andres Freund , Michael Paquier , Kyotaro Horiguchi , thomas.munro@gmail.com, tgl@sss.pgh.pa.us, john.naylor@enterprisedb.com, noriyoshi.shinoda@hpe.com, jchampion@timescale.com, sawada.mshk@gmail.com, masao.fujii@oss.nttdata.com, pgsql-hackers@postgresql.org, rjuju123@gmail.com Subject: Re: Improve logging when using Huge Pages Message-ID: <20230215013256.GP1653@telsasoft.com> References: <20230208231803.GP1653@telsasoft.com> <20230209094013.67varf63bihops33@alvherre.pgsql> <20230209192909.GA606606@nathanxps13> <20230213232245.GI1653@telsasoft.com> <20230214041852.GA1190218@nathanxps13> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="OrT4iOlIQZp3kw4S" Content-Disposition: inline In-Reply-To: <20230214041852.GA1190218@nathanxps13> User-Agent: Mutt/1.9.4 (2018-02-28) List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Archived-At: Precedence: bulk --OrT4iOlIQZp3kw4S Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Mon, Feb 13, 2023 at 08:18:52PM -0800, Nathan Bossart wrote: > On Mon, Feb 13, 2023 at 05:22:45PM -0600, Justin Pryzby wrote: > > + Reports whether huge pages are in use by the current process. > > + See for more information. > > nitpick: Should this say "server" instead of "current process"? It should probably say "instance" :) > > +static char *huge_pages_active = "unknown"; /* dynamically set */ > > nitpick: Does this need to be initialized here? None of the GUCs' C vars need to be initialized, since the guc machinery will do it. ...but the convention is that they *are* initialized - and that's now partially enforced. See: d9d873bac67047cfacc9f5ef96ee488f2cb0f1c3 7d25958453a60337bcb7bcc986e270792c007ea4 a73952b795632b2cf5acada8476e7cf75857e9be > > + { > > + {"huge_pages_active", PGC_INTERNAL, PRESET_OPTIONS, > > + gettext_noop("Indicates whether huge pages are in use."), > > + NULL, > > + GUC_NOT_IN_SAMPLE | GUC_DISALLOW_IN_FILE | GUC_RUNTIME_COMPUTED > > + }, > > + &huge_pages_active, > > + "unknown", > > + NULL, NULL, NULL > > + }, > > I'm curious why you chose to make this a string instead of an enum. There > might be little practical difference, but since there are only three > possible values, I wonder if it'd be better form to make it an enum. It takes more code to write as an enum - see 002.txt. I'm not convinced this is better. But your comment made me fix its , and reconsider the strings, which I changed to active={unknown/true/false} rather than {unk/on/off}. It could also be active={unknown/yes/no}... -- Justin --OrT4iOlIQZp3kw4S Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v4-0001-add-GUC-huge_pages_active.patch" From 7bf67d398902570ffc68a6b78265a0e346503392 Mon Sep 17 00:00:00 2001 From: Justin Pryzby Date: Mon, 23 Jan 2023 18:33:51 -0600 Subject: [PATCH v4 1/2] add GUC: huge_pages_active This is useful to show the current state of huge pages when huge_pages=try. The effective status is not otherwise visible without OS level tools like gdb or /proc/N/smaps. https://www.postgresql.org/message-id/flat/TU4PR8401MB1152EBB0D271F827E2E37A01EECC9@TU4PR8401MB1152.NAMPRD84.PROD.OUTLOOK.COM --- doc/src/sgml/config.sgml | 17 ++++++++++++++++- src/backend/port/sysv_shmem.c | 3 +++ src/backend/port/win32_shmem.c | 4 ++++ src/backend/utils/misc/guc_tables.c | 12 ++++++++++++ 4 files changed, 35 insertions(+), 1 deletion(-) diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml index 8c56b134a84..3ff301edf8a 100644 --- a/doc/src/sgml/config.sgml +++ b/doc/src/sgml/config.sgml @@ -1700,23 +1700,24 @@ include_dir 'conf.d' Controls whether huge pages are requested for the main shared memory area. Valid values are try (the default), on, and off. With huge_pages set to try, the server will try to request huge pages, but fall back to the default if that fails. With on, failure to request huge pages will prevent the server from starting up. With off, - huge pages will not be requested. + huge pages will not be requested. The actual state of huge pages is + indicated by the server variable . At present, this setting is supported only on Linux and Windows. The setting is ignored on other systems when set to try. On Linux, it is only supported when shared_memory_type is set to mmap (the default). @@ -10679,22 +10680,36 @@ dynamic_library_path = 'C:\tools\postgresql;H:\my_project\lib;$libdir' with assertions enabled. That is the case if the macro USE_ASSERT_CHECKING is defined when PostgreSQL is built (accomplished e.g., by the configure option ). By default PostgreSQL is built without assertions. + + huge_pages_active (string) + + huge_pages_active configuration parameter + + + + + Reports whether huge pages are in use by the current instance. + See for more information. + + + + integer_datetimes (boolean) integer_datetimes configuration parameter Reports whether PostgreSQL was built with support for 64-bit-integer dates and times. As of PostgreSQL 10, this is always on. diff --git a/src/backend/port/sysv_shmem.c b/src/backend/port/sysv_shmem.c index eaba244bc9c..89ec5c30364 100644 --- a/src/backend/port/sysv_shmem.c +++ b/src/backend/port/sysv_shmem.c @@ -619,22 +619,25 @@ CreateAnonymousSegment(Size *size) allocsize += hugepagesize - (allocsize % hugepagesize); ptr = mmap(NULL, allocsize, PROT_READ | PROT_WRITE, PG_MMAP_FLAGS | mmap_flags, -1, 0); mmap_errno = errno; if (huge_pages == HUGE_PAGES_TRY && ptr == MAP_FAILED) elog(DEBUG1, "mmap(%zu) with MAP_HUGETLB failed, huge pages disabled: %m", allocsize); } #endif + SetConfigOption("huge_pages_active", ptr == MAP_FAILED ? "false" : "true", + PGC_INTERNAL, PGC_S_DYNAMIC_DEFAULT); + if (ptr == MAP_FAILED && huge_pages != HUGE_PAGES_ON) { /* * Use the original size, not the rounded-up value, when falling back * to non-huge pages. */ allocsize = *size; ptr = mmap(NULL, allocsize, PROT_READ | PROT_WRITE, PG_MMAP_FLAGS, -1, 0); mmap_errno = errno; } diff --git a/src/backend/port/win32_shmem.c b/src/backend/port/win32_shmem.c index 62e08074770..fa72b3d1f8f 100644 --- a/src/backend/port/win32_shmem.c +++ b/src/backend/port/win32_shmem.c @@ -319,22 +319,26 @@ retry: * If the segment already existed, CreateFileMapping() will return a * handle to the existing one and set ERROR_ALREADY_EXISTS. */ if (GetLastError() == ERROR_ALREADY_EXISTS) { CloseHandle(hmap); /* Close the handle, since we got a valid one * to the previous segment. */ hmap = NULL; Sleep(1000); continue; } + + SetConfigOption("huge_pages_active", (flProtect & SEC_LARGE_PAGES) ? + "true" : "false", PGC_INTERNAL, PGC_S_DYNAMIC_DEFAULT); + break; } /* * If the last call in the loop still returned ERROR_ALREADY_EXISTS, this * shared memory segment exists and we assume it belongs to somebody else. */ if (!hmap) ereport(FATAL, (errmsg("pre-existing shared memory block is still in use"), errhint("Check if there are any old server processes still running, and terminate them."))); diff --git a/src/backend/utils/misc/guc_tables.c b/src/backend/utils/misc/guc_tables.c index b21698934c8..1a298f16c81 100644 --- a/src/backend/utils/misc/guc_tables.c +++ b/src/backend/utils/misc/guc_tables.c @@ -554,22 +554,23 @@ static int server_version_num; #define DEFAULT_SYSLOG_FACILITY LOG_LOCAL0 #else #define DEFAULT_SYSLOG_FACILITY 0 #endif static int syslog_facility = DEFAULT_SYSLOG_FACILITY; static char *timezone_string; static char *log_timezone_string; static char *timezone_abbreviations_string; static char *data_directory; static char *session_authorization_string; +static char *huge_pages_active = "unknown"; /* dynamically set */ static int max_function_args; static int max_index_keys; static int max_identifier_length; static int block_size; static int segment_size; static int shared_memory_size_mb; static int shared_memory_size_in_huge_pages; static int wal_block_size; static bool data_checksums; static bool integer_datetimes; @@ -4516,22 +4517,33 @@ struct config_string ConfigureNamesString[] = { {"backtrace_functions", PGC_SUSET, DEVELOPER_OPTIONS, gettext_noop("Log backtrace for errors in these functions."), NULL, GUC_NOT_IN_SAMPLE }, &backtrace_functions, "", check_backtrace_functions, assign_backtrace_functions, NULL }, + { + {"huge_pages_active", PGC_INTERNAL, PRESET_OPTIONS, + gettext_noop("Indicates whether huge pages are in use."), + NULL, + GUC_NOT_IN_SAMPLE | GUC_DISALLOW_IN_FILE | GUC_RUNTIME_COMPUTED + }, + &huge_pages_active, + "unknown", + NULL, NULL, NULL + }, + /* End-of-list marker */ { {NULL, 0, 0, NULL, NULL}, NULL, NULL, NULL, NULL, NULL } }; struct config_enum ConfigureNamesEnum[] = { { {"backslash_quote", PGC_USERSET, COMPAT_OPTIONS_PREVIOUS, -- 2.34.1 --OrT4iOlIQZp3kw4S Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v4-0002-f-convert-to-an-enum.txt" From 96ad5dcd77bb8d2e4c9c3ca06ae4dcf43017defe Mon Sep 17 00:00:00 2001 From: Justin Pryzby Date: Tue, 14 Feb 2023 15:29:41 -0600 Subject: [PATCH v4 2/2] f! convert to an enum --- doc/src/sgml/config.sgml | 2 +- src/backend/utils/misc/guc_tables.c | 32 ++++++++++++++++++----------- src/include/storage/pg_shmem.h | 8 ++++++++ 3 files changed, 29 insertions(+), 13 deletions(-) diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml index 3ff301edf8a..1f6f71a2826 100644 --- a/doc/src/sgml/config.sgml +++ b/doc/src/sgml/config.sgml @@ -10681,23 +10681,23 @@ dynamic_library_path = 'C:\tools\postgresql;H:\my_project\lib;$libdir' macro USE_ASSERT_CHECKING is defined when PostgreSQL is built (accomplished e.g., by the configure option ). By default PostgreSQL is built without assertions. - huge_pages_active (string) + huge_pages_active (enum) huge_pages_active configuration parameter Reports whether huge pages are in use by the current instance. See for more information. diff --git a/src/backend/utils/misc/guc_tables.c b/src/backend/utils/misc/guc_tables.c index 1a298f16c81..f9c98997492 100644 --- a/src/backend/utils/misc/guc_tables.c +++ b/src/backend/utils/misc/guc_tables.c @@ -339,22 +339,29 @@ static const struct config_enum_entry huge_pages_options[] = { {"on", HUGE_PAGES_ON, false}, {"try", HUGE_PAGES_TRY, false}, {"true", HUGE_PAGES_ON, true}, {"false", HUGE_PAGES_OFF, true}, {"yes", HUGE_PAGES_ON, true}, {"no", HUGE_PAGES_OFF, true}, {"1", HUGE_PAGES_ON, true}, {"0", HUGE_PAGES_OFF, true}, {NULL, 0, false} }; +static const struct config_enum_entry huge_pages_active_options[] = { + {"unknown", HUGE_PAGES_ACTIVE_UNKNOWN, false}, + {"false", HUGE_PAGES_ACTIVE_FALSE, false}, + {"true", HUGE_PAGES_ACTIVE_TRUE, false}, + {NULL, 0, false} +}; + static const struct config_enum_entry recovery_prefetch_options[] = { {"off", RECOVERY_PREFETCH_OFF, false}, {"on", RECOVERY_PREFETCH_ON, false}, {"try", RECOVERY_PREFETCH_TRY, false}, {"true", RECOVERY_PREFETCH_ON, true}, {"false", RECOVERY_PREFETCH_OFF, true}, {"yes", RECOVERY_PREFETCH_ON, true}, {"no", RECOVERY_PREFETCH_OFF, true}, {"1", RECOVERY_PREFETCH_ON, true}, {"0", RECOVERY_PREFETCH_OFF, true}, {NULL, 0, false} @@ -527,22 +534,24 @@ int tcp_user_timeout; * renegotiation and therefore always try to zero it. */ int ssl_renegotiation_limit; /* * This really belongs in pg_shmem.c, but is defined here so that it doesn't * need to be duplicated in all the different implementations of pg_shmem.c. */ int huge_pages = HUGE_PAGES_TRY; int huge_page_size; +int huge_pages_active = HUGE_PAGES_ACTIVE_UNKNOWN; + /* * These variables are all dummies that don't do anything, except in some * cases provide the value for SHOW to display. The real state is elsewhere * and is kept in sync by assign_hooks. */ static char *syslog_ident_str; static double phony_random_seed; static char *client_encoding_string; static char *datestyle_string; static char *locale_collate; static char *locale_ctype; @@ -554,23 +563,22 @@ static int server_version_num; #define DEFAULT_SYSLOG_FACILITY LOG_LOCAL0 #else #define DEFAULT_SYSLOG_FACILITY 0 #endif static int syslog_facility = DEFAULT_SYSLOG_FACILITY; static char *timezone_string; static char *log_timezone_string; static char *timezone_abbreviations_string; static char *data_directory; static char *session_authorization_string; -static char *huge_pages_active = "unknown"; /* dynamically set */ static int max_function_args; static int max_index_keys; static int max_identifier_length; static int block_size; static int segment_size; static int shared_memory_size_mb; static int shared_memory_size_in_huge_pages; static int wal_block_size; static bool data_checksums; static bool integer_datetimes; @@ -4517,33 +4525,22 @@ struct config_string ConfigureNamesString[] = { {"backtrace_functions", PGC_SUSET, DEVELOPER_OPTIONS, gettext_noop("Log backtrace for errors in these functions."), NULL, GUC_NOT_IN_SAMPLE }, &backtrace_functions, "", check_backtrace_functions, assign_backtrace_functions, NULL }, - { - {"huge_pages_active", PGC_INTERNAL, PRESET_OPTIONS, - gettext_noop("Indicates whether huge pages are in use."), - NULL, - GUC_NOT_IN_SAMPLE | GUC_DISALLOW_IN_FILE | GUC_RUNTIME_COMPUTED - }, - &huge_pages_active, - "unknown", - NULL, NULL, NULL - }, - /* End-of-list marker */ { {NULL, 0, 0, NULL, NULL}, NULL, NULL, NULL, NULL, NULL } }; struct config_enum ConfigureNamesEnum[] = { { {"backslash_quote", PGC_USERSET, COMPAT_OPTIONS_PREVIOUS, @@ -4845,22 +4842,33 @@ struct config_enum ConfigureNamesEnum[] = { {"huge_pages", PGC_POSTMASTER, RESOURCES_MEM, gettext_noop("Use of huge pages on Linux or Windows."), NULL }, &huge_pages, HUGE_PAGES_TRY, huge_pages_options, NULL, NULL, NULL }, + { + {"huge_pages_active", PGC_INTERNAL, PRESET_OPTIONS, + gettext_noop("Indicates whether huge pages are in use."), + NULL, + GUC_NOT_IN_SAMPLE | GUC_DISALLOW_IN_FILE | GUC_RUNTIME_COMPUTED + }, + &huge_pages_active, + HUGE_PAGES_ACTIVE_UNKNOWN, huge_pages_active_options, + NULL, NULL, NULL + }, + { {"recovery_prefetch", PGC_SIGHUP, WAL_RECOVERY, gettext_noop("Prefetch referenced blocks during recovery."), gettext_noop("Look ahead in the WAL to find references to uncached data.") }, &recovery_prefetch, RECOVERY_PREFETCH_TRY, recovery_prefetch_options, check_recovery_prefetch, assign_recovery_prefetch, NULL }, { diff --git a/src/include/storage/pg_shmem.h b/src/include/storage/pg_shmem.h index 4dd05f156d5..1699bf64238 100644 --- a/src/include/storage/pg_shmem.h +++ b/src/include/storage/pg_shmem.h @@ -46,22 +46,30 @@ extern PGDLLIMPORT int shared_memory_type; extern PGDLLIMPORT int huge_pages; extern PGDLLIMPORT int huge_page_size; /* Possible values for huge_pages */ typedef enum { HUGE_PAGES_OFF, HUGE_PAGES_ON, HUGE_PAGES_TRY } HugePagesType; +/* Possible values for huge_pages_active */ +typedef enum +{ + HUGE_PAGES_ACTIVE_UNKNOWN, + HUGE_PAGES_ACTIVE_FALSE, + HUGE_PAGES_ACTIVE_TRUE, +} HugePagesActiveType; + /* Possible values for shared_memory_type */ typedef enum { SHMEM_TYPE_WINDOWS, SHMEM_TYPE_SYSV, SHMEM_TYPE_MMAP } PGShmemType; #ifndef WIN32 extern PGDLLIMPORT unsigned long UsedShmemSegID; #else -- 2.34.1 --OrT4iOlIQZp3kw4S--