public inbox for [email protected]
help / color / mirror / Atom feed[PATCH v2] Add fileval-bootval consistency check of GUC parameters
14+ messages / 7 participants
[nested] [flat]
* [PATCH v2] Add fileval-bootval consistency check of GUC parameters
@ 2022-05-30 07:11 Kyotaro Horiguchi <[email protected]>
0 siblings, 0 replies; 14+ messages in thread
From: Kyotaro Horiguchi @ 2022-05-30 07:11 UTC (permalink / raw)
We should keep GUC variables consistent between the default values
written in postgresql.conf.sample and defined in guc.c. Add an
automated way to check for the consistency to the TAP test suite. Some
variables are still excluded since they cannot be checked simple way.
---
src/backend/utils/misc/guc.c | 70 +++++++++++++++++++
src/backend/utils/misc/postgresql.conf.sample | 6 +-
src/include/catalog/pg_proc.dat | 5 ++
src/test/modules/test_misc/t/003_check_guc.pl | 55 +++++++++++++--
4 files changed, 127 insertions(+), 9 deletions(-)
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 55d41ae7d6..cd47dede1a 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -7514,6 +7514,76 @@ parse_and_validate_value(struct config_generic *record,
return true;
}
+/*
+ * Helper function for pg_normalize_config_value().
+ * Makes a palloced copy of src then link val to it.
+ * DO NOT destroy val while dst is in use.
+ */
+static struct config_generic *
+copy_config_and_set_value(struct config_generic *src, union config_var_val *val)
+{
+ struct config_generic *dst;
+
+#define CREATE_CONFIG_COPY(dst, src, t) \
+ dst = palloc(sizeof(struct t)); \
+ *(struct t *) dst = *(struct t *) src; \
+
+ switch (src->vartype)
+ {
+ case PGC_BOOL:
+ CREATE_CONFIG_COPY(dst, src, config_bool);
+ ((struct config_bool *)dst)->variable = &val->boolval;
+ break;
+ case PGC_INT:
+ CREATE_CONFIG_COPY(dst, src, config_int);
+ ((struct config_int *)dst)->variable = &val->intval;
+ break;
+ case PGC_REAL:
+ CREATE_CONFIG_COPY(dst, src, config_real);
+ ((struct config_real *)dst)->variable = &val->realval;
+ break;
+ case PGC_STRING:
+ CREATE_CONFIG_COPY(dst, src, config_string);
+ ((struct config_string *)dst)->variable = &val->stringval;
+ break;
+ case PGC_ENUM:
+ CREATE_CONFIG_COPY(dst, src, config_enum);
+ ((struct config_enum *)dst)->variable = &val->enumval;
+ break;
+ }
+
+ return dst;
+}
+
+
+/*
+ * Normalize given value according to the specified GUC variable
+ */
+Datum
+pg_normalize_config_value(PG_FUNCTION_ARGS)
+{
+ char *name = "";
+ char *value = "";
+ struct config_generic *record;
+ char *result;
+ void *extra;
+ union config_var_val altval;
+
+ if (!PG_ARGISNULL(0))
+ name = text_to_cstring(PG_GETARG_TEXT_PP(0));
+ if (!PG_ARGISNULL(1))
+ value = text_to_cstring(PG_GETARG_TEXT_PP(1));
+
+ record = find_option(name, true, false, ERROR);
+
+ parse_and_validate_value(record, name, value, PGC_S_TEST, WARNING,
+ &altval, &extra);
+ record = copy_config_and_set_value(record, &altval);
+
+ result = _ShowOption(record, true);
+
+ PG_RETURN_TEXT_P(cstring_to_text(result));
+}
/*
* Sets option `name' to given value.
diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample
index 48ad80cf2e..a6d5e401a9 100644
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -476,7 +476,7 @@
# in all cases.
# These are relevant when logging to syslog:
-#syslog_facility = 'LOCAL0'
+#syslog_facility = 'local0'
#syslog_ident = 'postgres'
#syslog_sequence_numbers = on
#syslog_split_messages = on
@@ -709,7 +709,7 @@
# - Locale and Formatting -
-#datestyle = 'iso, mdy'
+#datestyle = 'ISO, MDY'
#intervalstyle = 'postgres'
#timezone = 'GMT'
#timezone_abbreviations = 'Default' # Select the set of available time zone
@@ -721,7 +721,7 @@
# share/timezonesets/.
#extra_float_digits = 1 # min -15, max 3; any value >0 actually
# selects precise output mode
-#client_encoding = sql_ascii # actually, defaults to database
+#client_encoding = SQL_ASCII # actually, defaults to database
# encoding
# These settings are initialized by initdb, but they can be changed.
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 87aa571a33..c089b351e9 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -6118,6 +6118,11 @@
proname => 'pg_settings_get_flags', provolatile => 's', prorettype => '_text',
proargtypes => 'text', prosrc => 'pg_settings_get_flags' },
+{ oid => '9956', descr => 'normalize value to the unit of specified GUC',
+ proname => 'pg_normalize_config_value', proisstrict => 'f',
+ provolatile => 's', prorettype => 'text', proargtypes => 'text text',
+ proargnames => '{varname,value}', prosrc => 'pg_normalize_config_value' },
+
{ oid => '3329', descr => 'show config file settings',
proname => 'pg_show_all_file_settings', prorows => '1000', proretset => 't',
provolatile => 'v', prorettype => 'record', proargtypes => '',
diff --git a/src/test/modules/test_misc/t/003_check_guc.pl b/src/test/modules/test_misc/t/003_check_guc.pl
index 60459ef759..a92206942f 100644
--- a/src/test/modules/test_misc/t/003_check_guc.pl
+++ b/src/test/modules/test_misc/t/003_check_guc.pl
@@ -11,18 +11,40 @@ my $node = PostgreSQL::Test::Cluster->new('main');
$node->init;
$node->start;
+# parameter names that cannot get consistency check performed
+my @ignored_parameters = (
+ 'data_directory',
+ 'hba_file',
+ 'ident_file',
+ 'krb_server_keyfile',
+ 'max_stack_depth',
+ 'bgwriter_flush_after',
+ 'wal_sync_method',
+ 'checkpoint_flush_after',
+ 'timezone_abbreviations',
+ 'lc_messages'
+ );
+
# Grab the names of all the parameters that can be listed in the
# configuration sample file. config_file is an exception, it is not
# in postgresql.conf.sample but is part of the lists from guc.c.
my $all_params = $node->safe_psql(
'postgres',
- "SELECT name
+ "SELECT lower(name), vartype, unit, boot_val, '!'
FROM pg_settings
WHERE NOT 'NOT_IN_SAMPLE' = ANY (pg_settings_get_flags(name)) AND
name <> 'config_file'
ORDER BY 1");
# Note the lower-case conversion, for consistency.
-my @all_params_array = split("\n", lc($all_params));
+my %all_params_hash;
+foreach my $line (split("\n", $all_params))
+{
+ my @f = split('\|', $line);
+ fail("query returned wrong number of columns: $#f : $line") if ($#f != 4);
+ $all_params_hash{$f[0]}->{type} = $f[1];
+ $all_params_hash{$f[0]}->{unit} = $f[2];
+ $all_params_hash{$f[0]}->{bootval} = $f[3];
+}
# Grab the names of all parameters marked as NOT_IN_SAMPLE.
my $not_in_sample = $node->safe_psql(
@@ -43,7 +65,7 @@ my @gucs_in_file;
# Read the sample file line-by-line, checking its contents to build a list
# of everything known as a GUC.
-my $num_tests = 0;
+my @check_elems = ();
open(my $contents, '<', $sample_file)
|| die "Could not open $sample_file: $!";
while (my $line = <$contents>)
@@ -53,11 +75,16 @@ while (my $line = <$contents>)
# file.
# - Valid configuration options are followed immediately by " = ",
# with one space before and after the equal sign.
- if ($line =~ m/^#?([_[:alpha:]]+) = .*/)
+ if ($line =~ m/^#?([_[:alpha:]]+) = (.*)$/)
{
# Lower-case conversion matters for some of the GUCs.
my $param_name = lc($1);
+ # extract value
+ my $file_value = $2;
+ $file_value =~ s/\s*#.*$//; # strip trailing comment
+ $file_value =~ s/^'(.*)'$/$1/; # strip quotes
+
# Ignore some exceptions.
next if $param_name eq "include";
next if $param_name eq "include_dir";
@@ -66,19 +93,35 @@ while (my $line = <$contents>)
# Update the list of GUCs found in the sample file, for the
# follow-up tests.
push @gucs_in_file, $param_name;
+
+ # Check for consistency between bootval and file value.
+ if (!grep { $_ eq $param_name } @ignored_parameters)
+ {
+ push (@check_elems, "('$param_name','$file_value')");
+ }
}
}
close $contents;
+# run consistency check between config-file's default value and boot values.
+my $check_query =
+ 'SELECT f.n, f.v, s.boot_val FROM (VALUES '.
+ join(',', @check_elems).
+ ') f(n,v) JOIN pg_settings s ON s.name = f.n '.
+ 'WHERE pg_normalize_config_value(f.n, f.v) <> '.
+ 'pg_normalize_config_value(f.n, s.boot_val)';
+
+is ($node->safe_psql('postgres', $check_query), '',
+ 'check if fileval-bootval consistency is fine');
+
# Cross-check that all the GUCs found in the sample file match the ones
# fetched above. This maps the arrays to a hash, making the creation of
# each exclude and intersection list easier.
my %gucs_in_file_hash = map { $_ => 1 } @gucs_in_file;
-my %all_params_hash = map { $_ => 1 } @all_params_array;
my %not_in_sample_hash = map { $_ => 1 } @not_in_sample_array;
-my @missing_from_file = grep(!$gucs_in_file_hash{$_}, @all_params_array);
+my @missing_from_file = grep(!$gucs_in_file_hash{$_}, keys %all_params_hash);
is(scalar(@missing_from_file),
0, "no parameters missing from postgresql.conf.sample");
--
2.31.1
----Next_Part(Mon_May_30_17_27_19_2022_009)----
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: Correct the documentation for work_mem
@ 2023-04-21 17:15 Tom Lane <[email protected]>
2023-04-21 17:39 ` Re: Correct the documentation for work_mem Gurjeet Singh <[email protected]>
2023-09-08 00:16 ` Re: Correct the documentation for work_mem Bruce Momjian <[email protected]>
0 siblings, 2 replies; 14+ messages in thread
From: Tom Lane @ 2023-04-21 17:15 UTC (permalink / raw)
To: Peter Eisentraut <[email protected]>; +Cc: Imseih (AWS), Sami <[email protected]>; pgsql-hackers
Peter Eisentraut <[email protected]> writes:
> On 21.04.23 16:28, Imseih (AWS), Sami wrote:
>> I suggest a small doc fix:
>> “Note that for a complex query, several sort or hash operations might be
>> running simultaneously;”
> Here is a discussion of these terms:
> https://takuti.me/note/parallel-vs-concurrent/
> I think "concurrently" is the correct word here.
Probably, but it'd do little to remove the confusion Sami is on about,
especially since the next sentence uses "concurrently" to describe the
other case. I think we need a more thorough rewording, perhaps like
- Note that for a complex query, several sort or hash operations might be
- running in parallel; each operation will generally be allowed
+ Note that a complex query may include several sort or hash
+ operations; each such operation will generally be allowed
to use as much memory as this value specifies before it starts
to write data into temporary files. Also, several running
sessions could be doing such operations concurrently.
I also find this wording a bit further down to be poor:
Hash-based operations are generally more sensitive to memory
availability than equivalent sort-based operations. The
memory available for hash tables is computed by multiplying
<varname>work_mem</varname> by
<varname>hash_mem_multiplier</varname>. This makes it
I think "available" is not le mot juste, and it's also unclear from
this whether we're speaking of the per-hash-table limit or some
(nonexistent) overall limit. How about
- memory available for hash tables is computed by multiplying
+ memory limit for a hash table is computed by multiplying
regards, tom lane
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: Correct the documentation for work_mem
2023-04-21 17:15 Re: Correct the documentation for work_mem Tom Lane <[email protected]>
@ 2023-04-21 17:39 ` Gurjeet Singh <[email protected]>
2023-04-22 03:36 ` Re: Correct the documentation for work_mem Imseih (AWS), Sami <[email protected]>
1 sibling, 1 reply; 14+ messages in thread
From: Gurjeet Singh @ 2023-04-21 17:39 UTC (permalink / raw)
To: Tom Lane <[email protected]>; +Cc: Peter Eisentraut <[email protected]>; Imseih (AWS), Sami <[email protected]>; pgsql-hackers
On Fri, Apr 21, 2023 at 10:15 AM Tom Lane <[email protected]> wrote:
>
> Peter Eisentraut <[email protected]> writes:
> > On 21.04.23 16:28, Imseih (AWS), Sami wrote:
> >> I suggest a small doc fix:
> >> “Note that for a complex query, several sort or hash operations might be
> >> running simultaneously;”
>
> > Here is a discussion of these terms:
> > https://takuti.me/note/parallel-vs-concurrent/
>
> > I think "concurrently" is the correct word here.
>
> Probably, but it'd do little to remove the confusion Sami is on about,
+1.
When discussing this internally, Sami's proposal was in fact to use
the word 'concurrently'. But given that when it comes to computers and
programming, it's common for someone to not understand the intricate
difference between the two terms, we thought it's best to not use any
of those, and instead use a word not usually associated with
programming and algorithms.
Aside: Another pair of words I see regularly used interchangeably,
when in fact they mean different things: precise vs. accurate.
> especially since the next sentence uses "concurrently" to describe the
> other case. I think we need a more thorough rewording, perhaps like
>
> - Note that for a complex query, several sort or hash operations might be
> - running in parallel; each operation will generally be allowed
> + Note that a complex query may include several sort or hash
> + operations; each such operation will generally be allowed
This wording doesn't seem to bring out the fact that there could be
more than one work_mem consumer running (in-progress) at the same
time. The reader to could mistake it to mean hashes and sorts in a
complex query may happen one after the other.
+ Note that a complex query may include several sort and hash operations, and
+ more than one of these operations may be in progress simultaneously at any
+ given time; each such operation will generally be allowed
I believe the phrase "several sort _and_ hash" better describes the
possible composition of a complex query, than does "several sort _or_
hash".
> I also find this wording a bit further down to be poor:
>
> Hash-based operations are generally more sensitive to memory
> availability than equivalent sort-based operations. The
> memory available for hash tables is computed by multiplying
> <varname>work_mem</varname> by
> <varname>hash_mem_multiplier</varname>. This makes it
>
> I think "available" is not le mot juste, and it's also unclear from
> this whether we're speaking of the per-hash-table limit or some
> (nonexistent) overall limit. How about
>
> - memory available for hash tables is computed by multiplying
> + memory limit for a hash table is computed by multiplying
+1
Best regards,
Gurjeet https://Gurje.et
Postgres Contributors Team, http://aws.amazon.com
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: Correct the documentation for work_mem
2023-04-21 17:15 Re: Correct the documentation for work_mem Tom Lane <[email protected]>
2023-04-21 17:39 ` Re: Correct the documentation for work_mem Gurjeet Singh <[email protected]>
@ 2023-04-22 03:36 ` Imseih (AWS), Sami <[email protected]>
2023-04-24 16:20 ` Re: Correct the documentation for work_mem Imseih (AWS), Sami <[email protected]>
0 siblings, 1 reply; 14+ messages in thread
From: Imseih (AWS), Sami @ 2023-04-22 03:36 UTC (permalink / raw)
To: Gurjeet Singh <[email protected]>; Tom Lane <[email protected]>; +Cc: Peter Eisentraut <[email protected]>; pgsql-hackers
> > especially since the next sentence uses "concurrently" to describe the
> > other case. I think we need a more thorough rewording, perhaps like
> >
> > - Note that for a complex query, several sort or hash operations might be
> > - running in parallel; each operation will generally be allowed
> > + Note that a complex query may include several sort or hash
> > + operations; each such operation will generally be allowed
> This wording doesn't seem to bring out the fact that there could be
> more than one work_mem consumer running (in-progress) at the same
> time.
Do you mean, more than one work_mem consumer running at the same
time for a given query? If so, that is precisely the point we need to convey
in the docs.
i.e. if I have 2 sorts in a query that can use up to 4MB each, at some point
in the query execution, I can have 8MB of memory allocated.
Regards,
Sami Imseih
Amazon Web Services (AWS)
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: Correct the documentation for work_mem
2023-04-21 17:15 Re: Correct the documentation for work_mem Tom Lane <[email protected]>
2023-04-21 17:39 ` Re: Correct the documentation for work_mem Gurjeet Singh <[email protected]>
2023-04-22 03:36 ` Re: Correct the documentation for work_mem Imseih (AWS), Sami <[email protected]>
@ 2023-04-24 16:20 ` Imseih (AWS), Sami <[email protected]>
2023-07-12 23:11 ` Re: Correct the documentation for work_mem David Rowley <[email protected]>
0 siblings, 1 reply; 14+ messages in thread
From: Imseih (AWS), Sami @ 2023-04-24 16:20 UTC (permalink / raw)
To: Gurjeet Singh <[email protected]>; Tom Lane <[email protected]>; +Cc: Peter Eisentraut <[email protected]>; pgsql-hackers
Based on the feedback, here is a v1 of the suggested doc changes.
I modified Gurjeets suggestion slightly to make it clear that a specific
query execution could have operations simultaneously using up to
work_mem.
I also added the small hash table memory limit clarification.
Regards,
Sami Imseih
Amazon Web Services (AWS)
Attachments:
[application/octet-stream] v1-0001-Fix-documentation-for-work_mem.patch (2.2K, ../../[email protected]/2-v1-0001-Fix-documentation-for-work_mem.patch)
download | inline diff:
From 2fbbe428c25d7d12ad7d818ef5d00fe7c8085433 Mon Sep 17 00:00:00 2001
From: EC2 Default User <[email protected]>
Date: Mon, 24 Apr 2023 16:04:45 +0000
Subject: [PATCH 1/1] Fix documentation for work_mem
A couple of small documentation fixes to clear
up terminology used in the work_mem documentation.
The removal of the usage of "parallel"
as it does not refer to parallel query in the context
of work_mem. Also, a clarification on the memory used
by hash tables.
Discussion: https://www.postgresql.org/message-id/flat/66590882-F48C-4A25-83E3-73792CF8C51F%40amazon.com
---
doc/src/sgml/config.sgml | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 091a79d4f3..bafda1c53a 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -1897,8 +1897,9 @@ include_dir 'conf.d'
(such as a sort or hash table) before writing to temporary disk files.
If this value is specified without units, it is taken as kilobytes.
The default value is four megabytes (<literal>4MB</literal>).
- Note that for a complex query, several sort or hash operations might be
- running in parallel; each operation will generally be allowed
+ Note that a complex query may include several sort and hash operations,
+ and more than one of these operations may be in progress simultaneously
+ for a given query execution; each such operation will generally be allowed
to use as much memory as this value specifies before it starts
to write data into temporary files. Also, several running
sessions could be doing such operations concurrently.
@@ -1914,7 +1915,7 @@ include_dir 'conf.d'
<para>
Hash-based operations are generally more sensitive to memory
availability than equivalent sort-based operations. The
- memory available for hash tables is computed by multiplying
+ memory limit for hash tables is computed by multiplying
<varname>work_mem</varname> by
<varname>hash_mem_multiplier</varname>. This makes it
possible for hash-based operations to use an amount of memory
--
2.39.2
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: Correct the documentation for work_mem
2023-04-21 17:15 Re: Correct the documentation for work_mem Tom Lane <[email protected]>
2023-04-21 17:39 ` Re: Correct the documentation for work_mem Gurjeet Singh <[email protected]>
2023-04-22 03:36 ` Re: Correct the documentation for work_mem Imseih (AWS), Sami <[email protected]>
2023-04-24 16:20 ` Re: Correct the documentation for work_mem Imseih (AWS), Sami <[email protected]>
@ 2023-07-12 23:11 ` David Rowley <[email protected]>
2023-07-31 20:44 ` Re: Correct the documentation for work_mem Tristen Raab <[email protected]>
0 siblings, 1 reply; 14+ messages in thread
From: David Rowley @ 2023-07-12 23:11 UTC (permalink / raw)
To: Imseih (AWS), Sami <[email protected]>; +Cc: Gurjeet Singh <[email protected]>; Tom Lane <[email protected]>; Peter Eisentraut <[email protected]>; pgsql-hackers
On Tue, 25 Apr 2023 at 04:20, Imseih (AWS), Sami <[email protected]> wrote:
>
> Based on the feedback, here is a v1 of the suggested doc changes.
>
> I modified Gurjeets suggestion slightly to make it clear that a specific
> query execution could have operations simultaneously using up to
> work_mem.
> - Note that for a complex query, several sort or hash operations might be
> - running in parallel; each operation will generally be allowed
> + Note that a complex query may include several sort and hash operations,
> + and more than one of these operations may be in progress simultaneously
> + for a given query execution; each such operation will generally be allowed
> to use as much memory as this value specifies before it starts
> to write data into temporary files. Also, several running
> sessions could be doing such operations concurrently.
I'm wondering about adding "and more than one of these operations may
be in progress simultaneously". Are you talking about concurrent
sessions running other queries which are using work_mem too? If so,
isn't that already covered by the final sentence in the quoted text
above? if not, what is running simultaneously?
I think Tom's suggestion looks fine. I'd maybe change "sort or hash"
to "sort and hash" per the suggestion from Gurjeet above.
David
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: Correct the documentation for work_mem
2023-04-21 17:15 Re: Correct the documentation for work_mem Tom Lane <[email protected]>
2023-04-21 17:39 ` Re: Correct the documentation for work_mem Gurjeet Singh <[email protected]>
2023-04-22 03:36 ` Re: Correct the documentation for work_mem Imseih (AWS), Sami <[email protected]>
2023-04-24 16:20 ` Re: Correct the documentation for work_mem Imseih (AWS), Sami <[email protected]>
2023-07-12 23:11 ` Re: Correct the documentation for work_mem David Rowley <[email protected]>
@ 2023-07-31 20:44 ` Tristen Raab <[email protected]>
0 siblings, 0 replies; 14+ messages in thread
From: Tristen Raab @ 2023-07-31 20:44 UTC (permalink / raw)
To: [email protected]; +Cc: Sami Imseih <[email protected]>
The following review has been posted through the commitfest application:
make installcheck-world: tested, passed
Implements feature: tested, passed
Spec compliant: not tested
Documentation: tested, passed
Hello,
I've reviewed and built the documentation for the updated patch. As it stands right now I think the documentation for this section is quite clear.
> I'm wondering about adding "and more than one of these operations may
> be in progress simultaneously". Are you talking about concurrent
> sessions running other queries which are using work_mem too?
This appears to be referring to the "sort and hash" operations mentioned prior.
> If so,
> isn't that already covered by the final sentence in the quoted text
> above? if not, what is running simultaneously?
I believe the last sentence is referring to another session that is running its own sort and hash operations. So the first section you mention is describing how sort and hash operations can be in execution at the same time for a query, while the second refers to how sessions may overlap in their execution of sort and hash operations if I am understanding this correctly.
I also agree that changing "sort or hash" to "sort and hash" is a better description.
Tristen
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: Correct the documentation for work_mem
2023-04-21 17:15 Re: Correct the documentation for work_mem Tom Lane <[email protected]>
@ 2023-09-08 00:16 ` Bruce Momjian <[email protected]>
2023-09-08 05:23 ` Re: Correct the documentation for work_mem David Rowley <[email protected]>
1 sibling, 1 reply; 14+ messages in thread
From: Bruce Momjian @ 2023-09-08 00:16 UTC (permalink / raw)
To: Tom Lane <[email protected]>; +Cc: Peter Eisentraut <[email protected]>; Imseih (AWS), Sami <[email protected]>; pgsql-hackers
On Fri, Apr 21, 2023 at 01:15:01PM -0400, Tom Lane wrote:
> Peter Eisentraut <[email protected]> writes:
> > On 21.04.23 16:28, Imseih (AWS), Sami wrote:
> >> I suggest a small doc fix:
> >> “Note that for a complex query, several sort or hash operations might be
> >> running simultaneously;”
>
> > Here is a discussion of these terms:
> > https://takuti.me/note/parallel-vs-concurrent/
>
> > I think "concurrently" is the correct word here.
>
> Probably, but it'd do little to remove the confusion Sami is on about,
> especially since the next sentence uses "concurrently" to describe the
> other case. I think we need a more thorough rewording, perhaps like
>
> - Note that for a complex query, several sort or hash operations might be
> - running in parallel; each operation will generally be allowed
> + Note that a complex query may include several sort or hash
> + operations; each such operation will generally be allowed
> to use as much memory as this value specifies before it starts
> to write data into temporary files. Also, several running
> sessions could be doing such operations concurrently.
>
> I also find this wording a bit further down to be poor:
>
> Hash-based operations are generally more sensitive to memory
> availability than equivalent sort-based operations. The
> memory available for hash tables is computed by multiplying
> <varname>work_mem</varname> by
> <varname>hash_mem_multiplier</varname>. This makes it
>
> I think "available" is not le mot juste, and it's also unclear from
> this whether we're speaking of the per-hash-table limit or some
> (nonexistent) overall limit. How about
>
> - memory available for hash tables is computed by multiplying
> + memory limit for a hash table is computed by multiplying
Adjusted patch attached.
--
Bruce Momjian <[email protected]> https://momjian.us
EDB https://enterprisedb.com
Only you can decide what is important to you.
Attachments:
[text/x-diff] workmem.diff (1.6K, ../../[email protected]/2-workmem.diff)
download | inline diff:
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 6bc1b215db..45d1bb4b7b 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -1829,9 +1829,10 @@ include_dir 'conf.d'
(such as a sort or hash table) before writing to temporary disk files.
If this value is specified without units, it is taken as kilobytes.
The default value is four megabytes (<literal>4MB</literal>).
- Note that for a complex query, several sort or hash operations might be
- running in parallel; each operation will generally be allowed
- to use as much memory as this value specifies before it starts
+ Note that a complex query might perform several sort or hash
+ operations at the same time, with each operation generally being
+ allowed to use as much memory as this value specifies before
+ it starts
to write data into temporary files. Also, several running
sessions could be doing such operations concurrently.
Therefore, the total memory used could be many times the value
@@ -1845,7 +1846,7 @@ include_dir 'conf.d'
<para>
Hash-based operations are generally more sensitive to memory
availability than equivalent sort-based operations. The
- memory available for hash tables is computed by multiplying
+ memory limit for a hash table is computed by multiplying
<varname>work_mem</varname> by
<varname>hash_mem_multiplier</varname>. This makes it
possible for hash-based operations to use an amount of memory
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: Correct the documentation for work_mem
2023-04-21 17:15 Re: Correct the documentation for work_mem Tom Lane <[email protected]>
2023-09-08 00:16 ` Re: Correct the documentation for work_mem Bruce Momjian <[email protected]>
@ 2023-09-08 05:23 ` David Rowley <[email protected]>
2023-09-09 02:25 ` Re: Correct the documentation for work_mem Imseih (AWS), Sami <[email protected]>
0 siblings, 1 reply; 14+ messages in thread
From: David Rowley @ 2023-09-08 05:23 UTC (permalink / raw)
To: Bruce Momjian <[email protected]>; +Cc: Tom Lane <[email protected]>; Peter Eisentraut <[email protected]>; Imseih (AWS), Sami <[email protected]>; pgsql-hackers
On Fri, 8 Sept 2023 at 15:24, Bruce Momjian <[email protected]> wrote:
> Adjusted patch attached.
This looks mostly fine to me modulo "sort or hash". I do see many
instances of "and/or" in the docs. Maybe that would work better.
David
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: Correct the documentation for work_mem
2023-04-21 17:15 Re: Correct the documentation for work_mem Tom Lane <[email protected]>
2023-09-08 00:16 ` Re: Correct the documentation for work_mem Bruce Momjian <[email protected]>
2023-09-08 05:23 ` Re: Correct the documentation for work_mem David Rowley <[email protected]>
@ 2023-09-09 02:25 ` Imseih (AWS), Sami <[email protected]>
2023-09-11 10:02 ` Re: Correct the documentation for work_mem David Rowley <[email protected]>
0 siblings, 1 reply; 14+ messages in thread
From: Imseih (AWS), Sami @ 2023-09-09 02:25 UTC (permalink / raw)
To: David Rowley <[email protected]>; Bruce Momjian <[email protected]>; +Cc: Tom Lane <[email protected]>; Peter Eisentraut <[email protected]>; pgsql-hackers
> This looks mostly fine to me modulo "sort or hash". I do see many
> instances of "and/or" in the docs. Maybe that would work better.
"sort or hash operations at the same time" is clear explanation IMO.
This latest version of the patch looks good to me.
Regards,
Sami
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: Correct the documentation for work_mem
2023-04-21 17:15 Re: Correct the documentation for work_mem Tom Lane <[email protected]>
2023-09-08 00:16 ` Re: Correct the documentation for work_mem Bruce Momjian <[email protected]>
2023-09-08 05:23 ` Re: Correct the documentation for work_mem David Rowley <[email protected]>
2023-09-09 02:25 ` Re: Correct the documentation for work_mem Imseih (AWS), Sami <[email protected]>
@ 2023-09-11 10:02 ` David Rowley <[email protected]>
2023-09-11 15:03 ` Re: Correct the documentation for work_mem Bruce Momjian <[email protected]>
0 siblings, 1 reply; 14+ messages in thread
From: David Rowley @ 2023-09-11 10:02 UTC (permalink / raw)
To: Imseih (AWS), Sami <[email protected]>; +Cc: Bruce Momjian <[email protected]>; Tom Lane <[email protected]>; Peter Eisentraut <[email protected]>; pgsql-hackers
On Sat, 9 Sept 2023 at 14:25, Imseih (AWS), Sami <[email protected]> wrote:
>
> > This looks mostly fine to me modulo "sort or hash". I do see many
> > instances of "and/or" in the docs. Maybe that would work better.
>
> "sort or hash operations at the same time" is clear explanation IMO.
Just for anyone else following along that haven't seen the patch. The
full text in question is:
+ Note that a complex query might perform several sort or hash
+ operations at the same time, with each operation generally being
It's certainly not a show-stopper. I do believe the patch makes some
improvements. The reason I'd prefer to see either "and" or "and/or"
in place of "or" is because the text is trying to imply that many of
these operations can run at the same time. I'm struggling to
understand why, given that there could be many sorts and many hashes
going on at once that we'd claim it could only be one *or* the other.
If we have 12 sorts and 4 hashes then that's not "several sort or hash
operations", it's "several sort and hash operations". Of course, it
could just be sorts or just hashes, so "and/or" works fine for that.
David
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: Correct the documentation for work_mem
2023-04-21 17:15 Re: Correct the documentation for work_mem Tom Lane <[email protected]>
2023-09-08 00:16 ` Re: Correct the documentation for work_mem Bruce Momjian <[email protected]>
2023-09-08 05:23 ` Re: Correct the documentation for work_mem David Rowley <[email protected]>
2023-09-09 02:25 ` Re: Correct the documentation for work_mem Imseih (AWS), Sami <[email protected]>
2023-09-11 10:02 ` Re: Correct the documentation for work_mem David Rowley <[email protected]>
@ 2023-09-11 15:03 ` Bruce Momjian <[email protected]>
2023-09-26 13:05 ` Re: Correct the documentation for work_mem David Rowley <[email protected]>
0 siblings, 1 reply; 14+ messages in thread
From: Bruce Momjian @ 2023-09-11 15:03 UTC (permalink / raw)
To: David Rowley <[email protected]>; +Cc: Imseih (AWS), Sami <[email protected]>; Tom Lane <[email protected]>; Peter Eisentraut <[email protected]>; pgsql-hackers
On Mon, Sep 11, 2023 at 10:02:55PM +1200, David Rowley wrote:
> On Sat, 9 Sept 2023 at 14:25, Imseih (AWS), Sami <[email protected]> wrote:
> >
> > > This looks mostly fine to me modulo "sort or hash". I do see many
> > > instances of "and/or" in the docs. Maybe that would work better.
> >
> > "sort or hash operations at the same time" is clear explanation IMO.
>
> Just for anyone else following along that haven't seen the patch. The
> full text in question is:
>
> + Note that a complex query might perform several sort or hash
> + operations at the same time, with each operation generally being
>
> It's certainly not a show-stopper. I do believe the patch makes some
> improvements. The reason I'd prefer to see either "and" or "and/or"
> in place of "or" is because the text is trying to imply that many of
> these operations can run at the same time. I'm struggling to
> understand why, given that there could be many sorts and many hashes
> going on at once that we'd claim it could only be one *or* the other.
> If we have 12 sorts and 4 hashes then that's not "several sort or hash
> operations", it's "several sort and hash operations". Of course, it
> could just be sorts or just hashes, so "and/or" works fine for that.
Yes, I see your point and went with "and", updated patch attached.
--
Bruce Momjian <[email protected]> https://momjian.us
EDB https://enterprisedb.com
Only you can decide what is important to you.
Attachments:
[text/x-diff] workmem.diff (1.6K, ../../[email protected]/2-workmem.diff)
download | inline diff:
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 6bc1b215db..8ed7ae57c2 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -1829,9 +1829,10 @@ include_dir 'conf.d'
(such as a sort or hash table) before writing to temporary disk files.
If this value is specified without units, it is taken as kilobytes.
The default value is four megabytes (<literal>4MB</literal>).
- Note that for a complex query, several sort or hash operations might be
- running in parallel; each operation will generally be allowed
- to use as much memory as this value specifies before it starts
+ Note that a complex query might perform several sort and hash
+ operations at the same time, with each operation generally being
+ allowed to use as much memory as this value specifies before
+ it starts
to write data into temporary files. Also, several running
sessions could be doing such operations concurrently.
Therefore, the total memory used could be many times the value
@@ -1845,7 +1846,7 @@ include_dir 'conf.d'
<para>
Hash-based operations are generally more sensitive to memory
availability than equivalent sort-based operations. The
- memory available for hash tables is computed by multiplying
+ memory limit for a hash table is computed by multiplying
<varname>work_mem</varname> by
<varname>hash_mem_multiplier</varname>. This makes it
possible for hash-based operations to use an amount of memory
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: Correct the documentation for work_mem
2023-04-21 17:15 Re: Correct the documentation for work_mem Tom Lane <[email protected]>
2023-09-08 00:16 ` Re: Correct the documentation for work_mem Bruce Momjian <[email protected]>
2023-09-08 05:23 ` Re: Correct the documentation for work_mem David Rowley <[email protected]>
2023-09-09 02:25 ` Re: Correct the documentation for work_mem Imseih (AWS), Sami <[email protected]>
2023-09-11 10:02 ` Re: Correct the documentation for work_mem David Rowley <[email protected]>
2023-09-11 15:03 ` Re: Correct the documentation for work_mem Bruce Momjian <[email protected]>
@ 2023-09-26 13:05 ` David Rowley <[email protected]>
2023-09-26 23:44 ` Re: Correct the documentation for work_mem Bruce Momjian <[email protected]>
0 siblings, 1 reply; 14+ messages in thread
From: David Rowley @ 2023-09-26 13:05 UTC (permalink / raw)
To: Bruce Momjian <[email protected]>; +Cc: Imseih (AWS), Sami <[email protected]>; Tom Lane <[email protected]>; Peter Eisentraut <[email protected]>; pgsql-hackers
On Tue, 12 Sept 2023 at 03:03, Bruce Momjian <[email protected]> wrote:
>
> On Mon, Sep 11, 2023 at 10:02:55PM +1200, David Rowley wrote:
> > It's certainly not a show-stopper. I do believe the patch makes some
> > improvements. The reason I'd prefer to see either "and" or "and/or"
> > in place of "or" is because the text is trying to imply that many of
> > these operations can run at the same time. I'm struggling to
> > understand why, given that there could be many sorts and many hashes
> > going on at once that we'd claim it could only be one *or* the other.
> > If we have 12 sorts and 4 hashes then that's not "several sort or hash
> > operations", it's "several sort and hash operations". Of course, it
> > could just be sorts or just hashes, so "and/or" works fine for that.
>
> Yes, I see your point and went with "and", updated patch attached.
Looks good to me.
David
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: Correct the documentation for work_mem
2023-04-21 17:15 Re: Correct the documentation for work_mem Tom Lane <[email protected]>
2023-09-08 00:16 ` Re: Correct the documentation for work_mem Bruce Momjian <[email protected]>
2023-09-08 05:23 ` Re: Correct the documentation for work_mem David Rowley <[email protected]>
2023-09-09 02:25 ` Re: Correct the documentation for work_mem Imseih (AWS), Sami <[email protected]>
2023-09-11 10:02 ` Re: Correct the documentation for work_mem David Rowley <[email protected]>
2023-09-11 15:03 ` Re: Correct the documentation for work_mem Bruce Momjian <[email protected]>
2023-09-26 13:05 ` Re: Correct the documentation for work_mem David Rowley <[email protected]>
@ 2023-09-26 23:44 ` Bruce Momjian <[email protected]>
0 siblings, 0 replies; 14+ messages in thread
From: Bruce Momjian @ 2023-09-26 23:44 UTC (permalink / raw)
To: David Rowley <[email protected]>; +Cc: Imseih (AWS), Sami <[email protected]>; Tom Lane <[email protected]>; Peter Eisentraut <[email protected]>; pgsql-hackers
On Wed, Sep 27, 2023 at 02:05:44AM +1300, David Rowley wrote:
> On Tue, 12 Sept 2023 at 03:03, Bruce Momjian <[email protected]> wrote:
> >
> > On Mon, Sep 11, 2023 at 10:02:55PM +1200, David Rowley wrote:
> > > It's certainly not a show-stopper. I do believe the patch makes some
> > > improvements. The reason I'd prefer to see either "and" or "and/or"
> > > in place of "or" is because the text is trying to imply that many of
> > > these operations can run at the same time. I'm struggling to
> > > understand why, given that there could be many sorts and many hashes
> > > going on at once that we'd claim it could only be one *or* the other.
> > > If we have 12 sorts and 4 hashes then that's not "several sort or hash
> > > operations", it's "several sort and hash operations". Of course, it
> > > could just be sorts or just hashes, so "and/or" works fine for that.
> >
> > Yes, I see your point and went with "and", updated patch attached.
>
> Looks good to me.
Patch applied back to Postgres 11.
--
Bruce Momjian <[email protected]> https://momjian.us
EDB https://enterprisedb.com
Only you can decide what is important to you.
^ permalink raw reply [nested|flat] 14+ messages in thread
end of thread, other threads:[~2023-09-26 23:44 UTC | newest]
Thread overview: 14+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2022-05-30 07:11 [PATCH v2] Add fileval-bootval consistency check of GUC parameters Kyotaro Horiguchi <[email protected]>
2023-04-21 17:15 Re: Correct the documentation for work_mem Tom Lane <[email protected]>
2023-04-21 17:39 ` Re: Correct the documentation for work_mem Gurjeet Singh <[email protected]>
2023-04-22 03:36 ` Re: Correct the documentation for work_mem Imseih (AWS), Sami <[email protected]>
2023-04-24 16:20 ` Re: Correct the documentation for work_mem Imseih (AWS), Sami <[email protected]>
2023-07-12 23:11 ` Re: Correct the documentation for work_mem David Rowley <[email protected]>
2023-07-31 20:44 ` Re: Correct the documentation for work_mem Tristen Raab <[email protected]>
2023-09-08 00:16 ` Re: Correct the documentation for work_mem Bruce Momjian <[email protected]>
2023-09-08 05:23 ` Re: Correct the documentation for work_mem David Rowley <[email protected]>
2023-09-09 02:25 ` Re: Correct the documentation for work_mem Imseih (AWS), Sami <[email protected]>
2023-09-11 10:02 ` Re: Correct the documentation for work_mem David Rowley <[email protected]>
2023-09-11 15:03 ` Re: Correct the documentation for work_mem Bruce Momjian <[email protected]>
2023-09-26 13:05 ` Re: Correct the documentation for work_mem David Rowley <[email protected]>
2023-09-26 23:44 ` Re: Correct the documentation for work_mem Bruce Momjian <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox