Re: Logging parallel worker draught

[Benoit Lobréau]

On 4/8/24 10:05, Andrey M. Borodin wrote:

Hi Benoit!

This is kind reminder that this thread is waiting for your response.
CF entry [0] is in "Waiting on Author", I'll move it to July CF.


Hi thanks for the reminder,

The past month as been hectic for me.
It should calm down by next week at wich point I'll have time to go back 
to this. sorry for the delay.


--
Benoit Lobréau
Consultant
http://dalibo.com

Re: Logging parallel worker draught

[Benoit Lobréau]

On 2/27/24 15:09, Tomas Vondra wrote> That is certainly true, but it's 
not a new thing, I believe. IIRC we may

> not report statistics until the end of the transaction, so no progress
> updates, I'm not sure what happens if the doesn't end correctly (e.g.
> backend dies, ...). Similarly for the temporary files, we don't report
> those until the temporary file gets closed, so I'm not sure you'd get a
> lot of info about the progress either.
>
> I admit I haven't tried and maybe I don't remember the details wrong.
> Might be useful to experiment with this first a little bit ...

Ah, yes, Tempfile usage is reported on tempfile closure or deletion
for shared file sets.

> I certainly understand that, and I realize it may feel daunting and even
> discouraging. What I can promise is that I'm willing to help, either by
> suggesting ways to approach the problems, doing reviews, and so on.
> Would that be sufficient for you to continue working on this patch?

Yes, thanks for the proposal, I'll work on it on report here. I am otherwise
booked on company projects so I cannot promise a quick progress.

> Some random thoughts/ideas about how to approach this:
>
> - For parallel workers I think this might be as simple as adding some
> counters into QueryDesc, and update those during query exec (instead of
> just logging stuff directly). I'm not sure if it should be added to
> "Instrumentation" or separately.

I will start here to see how it works.

--
Benoit Lobréau
Consultant
http://dalibo.com

Re: Logging parallel worker draught

[Benoit Lobréau]

On 2/25/24 23:32, Peter Smith wrote:

Also, I don't understand how the word "draught" (aka "draft") makes
sense here -- I assume the intended word was "drought" (???).


yes, that was the intent, sorry about that. English is not my native 
langage and I was convinced the spelling was correct.


--
Benoit Lobréau
Consultant
http://dalibo.com

Re: Logging parallel worker draught

[Benoit Lobréau]

On 2/25/24 20:13, Tomas Vondra wrote:
> 1) name of the GUC
...
> 2) logging just the failures provides an incomplete view
> log_parallel_workers = {none | failures | all}>
> where "failures" only logs when at least one worker fails to start, and
> "all" logs everything.
>
> AFAIK Sami made the same observation/argument in his last message [1].

I like the name and different levels you propose. I was initially 
thinking that the overall picture would be better summarized (an easier 
to implement) with pg_stat_statements. But with the granularity you 
propose, the choice is in the hands of the DBA, which is great and 
provides more options when we don't have full control of the installation.


> 3) node-level or query-level?
...
> There's no value in having information about every individual temp file,
> because we don't even know which node produced it. It'd be perfectly
> sufficient to log some summary statistics (number of files, total space,
> ...), either for the query as a whole, or perhaps per node (because
> that's what matters for work_mem). So I don't think we should mimic this
> just because log_temp_files does this.

I must admit that, given my (poor) technical level, I went for the 
easiest way.


If we go for the three levels you proposed, "node-level" makes even less 
sense (and has great "potential" for spam).


I can see one downside to the "query-level" approach: it might be more 
difficult to to give information in cases where the query doesn't end 
normally. It's sometimes useful to have hints about what was going wrong 
before a query was cancelled or crashed, which log_temp_files kinda does.


> I don't know how difficult would it be to track/collect this information
> for the whole query.

I am a worried this will be a little too much for me, given the time and 
the knowledge gap I have (both in C and PostgreSQL internals). I'll try 
to look anyway.


--
Benoit Lobréau
Consultant
http://dalibo.com

Re: Questions about the new subscription parameter: password_required

[Benoit Lobréau]

On 9/23/23 03:57, Jeff Davis wrote:

IIUC there is really one use case here, which is for superuser to
define a subscription including the connection, and then change the
owner to a non-superuser to actually run it (without being able to
touch the connection string itself). I'd just document that in its own
section, and mention a few caveats / mistakes to avoid. For instance,
when the superuser is defining the connection, don't forget to set
password_required=false, so that when you reassign to a non-superuser
then the connection doesn't break.


Hi,

I tried adding a section in "Logical Replication > Subscription" with 
the text you suggested and links in the CREATE / ALTER SUBSRIPTION commands.


Is it better ?

--
Benoit Lobréau
Consultant
http://dalibo.comFrom f3f1b0ce8617971b173ea901c9735d8357955aa2 Mon Sep 17 00:00:00 2001
From: benoit 
Date: Thu, 12 Oct 2023 16:45:11 +0200
Subject: [PATCH] Doc patch for password_required

Add documentation regarding non-superuser subscriptions with
password_required=true.
---
 doc/src/sgml/logical-replication.sgml | 32 +++
 doc/src/sgml/ref/alter_subscription.sgml  |  3 ++-
 doc/src/sgml/ref/create_subscription.sgml |  3 ++-
 3 files changed, 36 insertions(+), 2 deletions(-)

diff --git a/doc/src/sgml/logical-replication.sgml b/doc/src/sgml/logical-replication.sgml
index 3b2fa1129e..c3faaf88cd 100644
--- a/doc/src/sgml/logical-replication.sgml
+++ b/doc/src/sgml/logical-replication.sgml
@@ -329,6 +329,38 @@

   
 
+  
+   Password required
+
+   
+password_required is a subscription parameter which specifies whether
+connections to the publisher made as a result of this subscription must
+use password authentication. This setting is ignored when the subscription
+is owned by a superuser and set to true by default.
+   
+
+   
+If you want to have a subscription managed by a non-superuser with a connection string without
+a password, you have to set password_required = false before transferring it's
+ownership. In that case, only superusers can modify the subscription.
+   
+test_pub=# CREATE SUBSCRIPTION test_sub CONNECTION 'host=somehost port=5432 user=repli dbname=tests_pub' PUBLICATION test_pub WITH (password_required=false);
+CREATE SUBSCRIPTION
+test_pub=# ALTER SUBSCRIPTION test_sub OWNER TO new_sub_owner;
+ALTER SUBSCRIPTION
+   
+   
+
+   
+   
+   If the connection string doesn't contain a password or the publication
+   side doesn't require a password during authentication and you have set
+   password_required = truebefore transferring ownership,
+   the subscription will start failing.
+   
+   
+  
+
   
 Examples: Set Up Logical Replication
 
diff --git a/doc/src/sgml/ref/alter_subscription.sgml b/doc/src/sgml/ref/alter_subscription.sgml
index a85e04e4d6..e061c96937 100644
--- a/doc/src/sgml/ref/alter_subscription.sgml
+++ b/doc/src/sgml/ref/alter_subscription.sgml
@@ -50,7 +50,8 @@ ALTER SUBSCRIPTION name RENAME TO <
CREATE permission on the database. In addition,
to alter the owner, you must be able to SET ROLE to the
new owning role. If the subscription has
-   password_required=false, only superusers can modify it.
+   password_required=false, only superusers can modify it
+   (See ).
   
 
   
diff --git a/doc/src/sgml/ref/create_subscription.sgml b/doc/src/sgml/ref/create_subscription.sgml
index c1bafbfa06..33ad3d12c7 100644
--- a/doc/src/sgml/ref/create_subscription.sgml
+++ b/doc/src/sgml/ref/create_subscription.sgml
@@ -361,7 +361,8 @@ CREATE SUBSCRIPTION subscription_nametrue. Only superusers can set
-  this value to false.
+  this value to false
+  (See ).
  
 

-- 
2.41.0

Re: Logging parallel worker draught

[Benoit Lobréau]

On 10/11/23 17:26, Imseih (AWS), Sami wrote:

Thank you for resurrecting this thread.


Well, if you read Benoit's earlier proposal at [1] you'll see that he
does propose to have some cumulative stats; this LOG line he proposes
here is not a substitute for stats, but rather a complement.  I don't
see any reason to reject this patch even if we do get stats.


I believe both cumulative statistics and logs are needed. Logs excel in 
pinpointing specific queries at precise times, while statistics provide 
a broader overview of the situation. Additionally, I often encounter 
situations where clients lack pg_stat_statements and can't restart their 
production promptly.



Regarding the current patch, the latest version removes the separate GUC,
but the user should be able to control this behavior.


I created this patch in response to Amit Kapila's proposal to keep the 
discussion ongoing. However, I still favor the initial version with the 
GUCs.



Query text is logged when  log_min_error_statement > default level of "error".

This could be especially problematic when there is a query running more than 1 
Parallel
Gather node that is in draught. In those cases each node will end up
generating a log with the statement text. So, a single query execution could 
end up
having multiple log lines with the statement text.
...
I wonder if it will be better to accumulate the total # of workers planned and 
# of workers launched and
logging this information at the end of execution?


log_temp_files exhibits similar behavior when a query involves multiple 
on-disk sorts. I'm uncertain whether this is something we should or need 
to address. I'll explore whether the error message can be made more 
informative.


[local]:5437 postgres@postgres=# SET work_mem to '125kB';
[local]:5437 postgres@postgres=# SET log_temp_files TO 0;
[local]:5437 postgres@postgres=# SET client_min_messages TO log;
[local]:5437 postgres@postgres=# WITH a AS ( SELECT x FROM 
generate_series(1,1) AS F(x) ORDER BY 1 ) , b AS (SELECT x FROM 
generate_series(1,1) AS F(x) ORDER BY 1 ) SELECT * FROM a,b;
LOG:  temporary file: path "base/pgsql_tmp/pgsql_tmp138850.20", size 
122880 => First sort

LOG:  temporary file: path "base/pgsql_tmp/pgsql_tmp138850.19", size 14
LOG:  temporary file: path "base/pgsql_tmp/pgsql_tmp138850.23", size 14
LOG:  temporary file: path "base/pgsql_tmp/pgsql_tmp138850.22", size 
122880 => Second sort

LOG:  temporary file: path "base/pgsql_tmp/pgsql_tmp138850.21", size 14

--
Benoit Lobréau
Consultant
http://dalibo.com

Re: Questions about the new subscription parameter: password_required

[Benoit Lobréau]

On 9/26/23 19:00, Jeff Davis wrote:

   +   If the ownership of a subscription with
password_required=true
   +   is transferred to a non-superuser, they will gain full control
over the subscription
   +   but will not be able to modify it's connection string.

I think you mean false, right?


No, but I was wrong. At the beginning of the thread, I was surprised 
was even possible to change the ownership to a non-superuser because It 
shouldn't work and commands like ENABLE didn't complain in the terminal.
Then Robert Haas explained to me that it's ok because the superuser can 
do whatever he wants. I came back to it later and somehow convinced 
myself it was working. Sorry.



   +   If the ownership of a subscription with
password_required=true
   +   has been transferred to a non-superuser, it must be reverted to a
superuser for
   +   the DROP operation to succeed.

That's only needed if the superuser transfers a subscription with
password_required=true to a non-superuser and the connection string
does not contain a password. In that case, the subscription is already
in a failing state, not just for DROP. Ideally we'd have some other
warning in the docs not to do that -- maybe in CREATE and ALTER.


Yes, I forgot the connection string bit.


Also, if the subscription is in that kind of failing state, there are
other ways to get out of it as well, like disabling it and setting
connection=none, then dropping i

The code in for DropSubscription in
src/backend/commands/subscriptioncmds.c tries to connect before testing
if the slot is NONE / NULL. So it doesn't work to DISABLE the
subscription and set the slot to NONE.

Robert Haas proposed something in the following message but I am a 
little out of my depth here ...


https://www.postgresql.org/message-id/af9435ae-18df-6a9e-2374-2de23009518c%40dalibo.com


The whole thing is fairly delicate. As soon as you work slightly
outside of the intended use, password_required starts causing
unexpected things to happen.

As I said earlier, I think the best thing to do is to just have a
section that describes when to use password_required, what specific
things you should do to satisfy that case, and what caveats you should
avoid. Something like:

   "If you want to have a subscription using a connection string without
a password managed by a non-superuser, then: [ insert SQL steps here ].
Warning: if the connection string doesn't contain a password, make sure
to set password_required=false before transferring ownership, otherwise
it will start failing."


Ok, I will do it that way. Would you prefer this section to be in the 
ALTER SUBSCRIPTION on the CREATE SUBSCIPTION doc ?


--
Benoit Lobréau
Consultant
http://dalibo.com

Re: Questions about the new subscription parameter: password_required

[Benoit Lobréau]

On 9/26/23 16:27, Benoit Lobréau wrote:

I will try to come up with a documentation patch.


This is my attempt at a documentation patch.

--
Benoit Lobréau
Consultant
http://dalibo.comFrom a73baa91032fff37ef039168c276508553830f86 Mon Sep 17 00:00:00 2001
From: benoit 
Date: Tue, 26 Sep 2023 18:07:47 +0200
Subject: [PATCH] Doc patch for require_password

Add documentation to ALTER / DROP SUBSCRIPTION regarding non-superuser
subscriptions with require_password=true.
---
 doc/src/sgml/ref/alter_subscription.sgml | 3 +++
 doc/src/sgml/ref/drop_subscription.sgml  | 6 ++
 2 files changed, 9 insertions(+)

diff --git a/doc/src/sgml/ref/alter_subscription.sgml b/doc/src/sgml/ref/alter_subscription.sgml
index a85e04e4d6..0bbe7e2335 100644
--- a/doc/src/sgml/ref/alter_subscription.sgml
+++ b/doc/src/sgml/ref/alter_subscription.sgml
@@ -51,6 +51,9 @@ ALTER SUBSCRIPTION name RENAME TO <
to alter the owner, you must be able to SET ROLE to the
new owning role. If the subscription has
password_required=false, only superusers can modify it.
+   If the ownership of a subscription with password_required=true
+   is transferred to a non-superuser, they will gain full control over the subscription
+   but will not be able to modify it's connection string.
   
 
   
diff --git a/doc/src/sgml/ref/drop_subscription.sgml b/doc/src/sgml/ref/drop_subscription.sgml
index 2a67bdea91..8ec743abd0 100644
--- a/doc/src/sgml/ref/drop_subscription.sgml
+++ b/doc/src/sgml/ref/drop_subscription.sgml
@@ -102,6 +102,12 @@ DROP SUBSCRIPTION [ IF EXISTS ] nameDROP
SUBSCRIPTION cannot be executed inside a transaction block.
   
+
+  
+   If the ownership of a subscription with password_required=true
+   has been transferred to a non-superuser, it must be reverted to a superuser for
+   the DROP operation to succeed.
+  
  
 
  
-- 
2.41.0

Re: Questions about the new subscription parameter: password_required

[Benoit Lobréau]

On 9/22/23 21:58, Robert Haas wrote

I think that there normally shouldn't be any problem here, because if
form->subpasswordrequired is true, we expect that the connection
string should contain a password which the remote side actually uses,
or we expect the subscription to be owned by the superuser. If neither
of those things is the case, then either the superuser made a
subscription that doesn't use a password owned by a non-superuser
without fixing subpasswordrequired, or else the configuration on the
remote side has changed so that it now doesn't use the password when
formerly it did. In the first case, perhaps it would be fine to go
ahead and drop the slot, but in the second case I don't think it's OK
from a security point view, because the command is going to behave the
same way no matter who executes the drop command, and a non-superuser
who drops the slot shouldn't be permitted to rely on the postgres
processes's identity to do anything on a remote node -- including
dropping a relation slot. So I tentatively think that this behavior is
correct.


I must admin I hadn't considered the implication when the configuration 
on the remote side has changed and we use a non-superuser. I see how it 
could be problematic.


I will try to come up with a documentation patch.


Maybe you're altering it in a way that doesn't involve any connections
or any changes to the connection string? There's no security issue if,
say, you rename it.


I looked at the code again. Indeed, of the ALTER SUBSCRIPTION commands, 
only ALTER SUBSCRIPTION .. CONNECTION uses walrcv_check_conninfo().


I checked the other thread (Re: [16+] subscription can end up in 
inconsistent state [1]) and will try the patch. Is it the thread you 
where refering to earlier ?


[1] 
https://www.postgresql.org/message-id/flat/5dff4caf26f45ce224a33a5e18e110b93a351b2f.camel%40j-davis.com#ff4a06505de317b1ad436b8102a69446


--
Benoit Lobréau
Consultant
http://dalibo.com

Re: Questions about the new subscription parameter: password_required

[Benoit Lobréau]

On 9/22/23 14:36, Robert Haas wrote:

I haven't checked this, but I think what's happening here is that DROP
SUBSCRIPTION tries to drop the remote slot, which requires making a
connection, which can trigger the error. You might get different
results if you did ALTER SUBSCRIPTION ... SET (slot_name = none)
first.


You're right, it comes from the connection to drop the slot.

But the code in for DropSubscription in 
src/backend/commands/subscriptioncmds.c tries to connect before testing 
if the slot is NONE / NULL. So it doesn't work to DISABLE the 
subscription and set the slot to NONE.



  1522 >~~~must_use_password = !superuser_arg(subowner) && 
form->subpasswordrequired;

  ...
  1685 >~~~wrconn = walrcv_connect(conninfo, true, must_use_password,
 1 >~~~>~~~>~~~>~~~>~~~>~~~>~~~subname, );
 2 >~~~if (wrconn == NULL)
 3 >~~~{
 4 >~~~>~~~if (!slotname)
 5 >~~~>~~~{
 6 >~~~>~~~>~~~/* be tidy */
 7 >~~~>~~~>~~~list_free(rstates);
 8 >~~~>~~~>~~~table_close(rel, NoLock);
 9 >~~~>~~~>~~~return;
10 >~~~>~~~}
11 >~~~>~~~else
12 >~~~>~~~{
13 >~~~>~~~>~~~ReportSlotConnectionError(rstates, subid, slotname, 
err);

14 >~~~>~~~}
15 >~~~}


Reading the code, I think I understand why the postgres user cannot drop 
the slot:


* the owner is sub_owner (not a superuser)
* and form->subpasswordrequired is true

Should there be a test to check if the user executing the query is 
superuser? maybe it's handled differently? (I am not very familiar with 
the code).


I dont understand (yet?) why I can do ALTER SUBSCRIPTIONs after changing 
the ownership to an unpriviledged user (must_use_password should be true 
also in that case).


--
Benoit Lobréau
Consultant
http://dalibo.com

Re: Questions about the new subscription parameter: password_required

[Benoit Lobréau]

On 9/21/23 20:29, Robert Haas wrote:

Which one? I see 2 ALTER SUBSCRIPTION ... OWNER commands in
password_required.log and 1 more in password_required2.log, but
they're all performed by the superuser, who is entitled to do anything
they want.


Thank you for taking the time to respond!

I expected the ALTER SUBSCRIPTION ... OWNER command in 
password_required.log to fail because the end result of the command is a 
non-superuser owning a subscription with password_required=true, but the 
connection string has no password keyword, and the authentication scheme 
used doesn't require one anyway.


The description of the password_required parameter doesn't clearly state 
what will fail or when the configuration is enforced (during CREATE 
SUBSCRIPTION and ALTER SUBSCRIPTION .. CONNECTION):


""" https://www.postgresql.org/docs/16/sql-createsubscription.html
Specifies whether connections to the publisher made as a result of this 
subscription must use password authentication. This setting is ignored 
when the subscription is owned by a superuser. The default is true. Only 
superusers can set this value to false.

"""

The description of pg_subscription.subpasswordrequired doesn't either:

""" https://www.postgresql.org/docs/16/catalog-pg-subscription.html
If true, the subscription will be required to specify a password for 
authentication

"""

Can we consider adding something like this to clarify?

"""
This parameter is enforced when the CREATE SUBSCRIPTION or ALTER 
SUBSCRIPTION .. CONNECTION commands are executed. Therefore, it's 
possible to alter the ownership of a subscription with 
password_required=true to a non-superuser.

"""

Is the DROP SUBSCRIPTION failure in password_required.log expected for 
both superuser and non-superuser?


Is the DROP SUBSCRIPTION success in password_required2.log expected?
(i.e., with password_require=false, the only action a non-superuser can 
perform is dropping the subscription. Since they own it, it is 
understandable).


--
Benoit Lobréau
Consultant
http://dalibo.com

Questions about the new subscription parameter: password_required

[Benoit Lobréau]

Hi,

I am confused about the new subscription parameter: password_required.

I have two instances. The publisher's pg_hba is configured too allow 
connections without authentication. On the subscriber, I have an 
unprivileged user with pg_create_subscription and CREATE on the database.


I tried using a superuser to create a subsciption without setting the 
password_required parameter (the default is true). Then I changed the 
owner to the unprivileged user.


This user can use the subscription without limitation (including ALTER 
SUBSCRIPTION ENABLE / DISABLE). The \dRs+ metacommand shows that a 
password is requiered, which is not the case (or it is but it's not 
enforced).


Is this normal? I was expecting the ALTER SUBSCRIPTION .. OWNER to fail.

When I try to drop the subscription with the unprivileged user or a 
superuser, I get an error:


ERROR:  password is required
DETAIL:  Non-superuser cannot connect if the server does not request a 
password.
HINT:  Target server's authentication method must be changed, or set 
password_required=false in the subscription parameters.


I have to re-change the subscription owner to the superuser, to be able 
to drop it.


(See password_required.sql and password_required.log)

I tried the same setup and changed the connexion string to add an 
application_name with the unprivileged user. In this case, I am reminded 
that I need a password. I tried modifying password_required to false 
with the superuser and modify the connexion string with the unprivilege 
user again. It fails with:


HINT:  Subscriptions with the password_required option set to false may 
only be created or modified by the superuser.


I think that this part works as intended.

I tried dropping the subscription with the unprivilege user: it works. 
Is it normal (given the previous message)?


(see password_required2.sql and password_required2.log)

--
Benoit Lobréau
Consultant
http://dalibo.com

--
\c tests_pg16 postgres
You are now connected to database "tests_pg16" as user "postgres".
--
SELECT version();
 version  
--
 PostgreSQL 16.0 on x86_64-pc-linux-gnu, compiled by gcc (GCC) 12.3.1 20230508 (Red Hat 12.3.1-1), 64-bit
(1 row)

--
CREATE SUBSCRIPTION sub_pg16   
   CONNECTION 'host=/var/run/postgresql port=5437 user=user_pub_pg16 dbname=tests_pg16'
   PUBLICATION pub_pg16;
psql:/home/benoit/tmp/password_required.sql:8: NOTICE:  created replication slot "sub_pg16" on publisher
CREATE SUBSCRIPTION
--
ALTER SUBSCRIPTION sub_pg16 OWNER TO sub_owner ;
ALTER SUBSCRIPTION
--
\x
Expanded display is on.
\dRs+
List of subscriptions
-[ RECORD 1 ]--+
Name   | sub_pg16
Owner  | sub_owner
Enabled| t
Publication| {pub_pg16}
Binary | f
Streaming  | off
Two-phase commit   | d
Disable on error   | f
Origin | any
Password required  | t
Run as owner?  | f
Synchronous commit | off
Conninfo   | host=/var/run/postgresql port=5437 user=user_pub_pg16 dbname=tests_pg16
Skip LSN   | 0/0

\du+
List of roles
-[ RECORD 1 ]---
Role name   | postgres
Attributes  | Superuser, Create role, Create DB, Replication, Bypass RLS
Description | 
-[ RECORD 2 ]---
Role name   | sub_owner
Attributes  | 
Description | 

\l tests_pg16
List of databases
-[ RECORD 1 ]-+--
Name  | tests_pg16
Owner | postgres
Encoding  | UTF8
Locale Provider   | libc
Collate   | C
Ctype | C
ICU Locale| 
ICU Rules | 
Access privileges | =Tc/postgres +
  | postgres=CTc/postgres+
  | sub_owner=C/postgres

\x
Expanded display is off.
--
\c - sub_owner 
You are now connected to database "tests_pg16" as user "sub_owner".
--
ALTER SUBSCRIPTION sub_pg16 DISABLE;
ALTER SUBSCRIPTION
--
ALTER SUBSCRIPTION sub_pg16 ENABLE;
ALTER SUBSCRIPTION
--
ALTER SUBSCRIPTION sub_pg16 RENAME TO sub_pg16_renamed;
ALTER SUBSCRIPTION
--
DROP SUBSCRIPTION sub_pg16_renamed ;
psql:/home/benoit/tmp/password_required.sql:26: ERROR:  password is required
DETAIL:  Non-superuser cannot connect if the server does not request a password.
HINT:  Target server's authentication method must be changed, or set password_required=false in the subscription parameters.
--
\c - postgres
You are now connected to database "tests_pg16" as user "postgres".
--
DROP SUBSCRIPTION sub_pg16_renamed;
psql:/home/benoit/tmp/password_required.sql:30: ERROR:  password is required
DETAIL:  Non-superuser cannot connect if the server doe

Re: Logging parallel worker draught

[Benoit Lobréau]

On 5/1/23 18:33, Robert Haas wrote:
> Why not? It seems like something some people might want to log and
> others not. Running the whole server at DEBUG1 to get this information
> doesn't seem like a suitable answer.

Since the statement is also logged, it could spam the log with huge 
queries, which might also be a reason to stop logging this kind of 
problems until the issue is fixed.


I attached the patch without the guc anyway just in case.


What I was wondering was whether we would be better off putting this
into the statistics collector, vs. doing it via logging. Both
approaches seem to have pros and cons.


We tried to explore different options with my collegues in another 
thread [1]. I feel like both solutions are worthwhile, and would be 
helpful. I plan to take a look at the pg_stat_statement patch [2] next.


Since it's my first patch, I elected to choose the easiest solution to 
implement first. I also proposed this because I think logging can help 
pinpoint a lot of problems at a minimal cost, since it is easy to setup 
and exploit for everyone, everywhere


[1] 
https://www.postgresql.org/message-id/d657df20-c4bf-63f6-e74c-cb85a81d0...@dalibo.com


[2] 
https://www.postgresql.org/message-id/flat/6acbe570-068e-bd8e-95d5-00c737b865e8%40gmail.com


--
Benoit Lobréau
Consultant
http://dalibo.comFrom fc71ef40f33f94b0506a092fb5b3dcde6de6d60a Mon Sep 17 00:00:00 2001
From: benoit 
Date: Tue, 2 May 2023 10:08:00 +0200
Subject: [PATCH] Add logging for exceeded parallel worker slot limits

Procude a log message when a backend attempts to spawn a parallel worker
but fails due to insufficient worker slots. The shortage can stem from
max_worker_processes, max_parallel_worker, or
max_parallel_maintenance_workers. The log message can help database
administrators and developers diagnose performance issues related to
parallelism and optimize the configuration of the system accordingly.
---
 src/backend/access/transam/parallel.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/src/backend/access/transam/parallel.c 
b/src/backend/access/transam/parallel.c
index b26f2a64fb..c60d1bd739 100644
--- a/src/backend/access/transam/parallel.c
+++ b/src/backend/access/transam/parallel.c
@@ -630,6 +630,11 @@ LaunchParallelWorkers(ParallelContext *pcxt)
pcxt->nknown_attached_workers = 0;
}
 
+   if (pcxt->nworkers_launched < pcxt->nworkers_to_launch)
+   ereport(LOG,
+   (errmsg("Parallel Worker draught during 
statement execution: workers spawned %d, requested %d",
+   pcxt->nworkers_launched, 
pcxt->nworkers_to_launch)));
+
/* Restore previous memory context. */
MemoryContextSwitchTo(oldcontext);
 }
-- 
2.39.2

Re: Logging parallel worker draught

[Benoit Lobréau]

On 4/22/23 13:06, Amit Kapila wrote:

I don't think introducing a GUC for this is a good idea. We can
directly output this message in the server log either at LOG or DEBUG1
level.


Hi,

Sorry for the delayed answer, I was away from my computer for a few 
days. I don't mind removing the guc, but I would like to keep it at the 
LOG level. When I do audits most client are at that level and setting it 
to DEBUG1 whould add too much log for them on the long run.


I'll post the corresponding patch asap.

--
Benoit Lobréau
Consultant
http://dalibo.com

Logging parallel worker draught

[Benoit Lobréau]

Hi hackers,

Following my previous mail about adding stats on parallelism[1], this 
patch introduces the log_parallel_worker_draught parameter, which 
controls whether a log message is produced when a backend attempts to 
spawn a parallel worker but fails due to insufficient worker slots. The 
shortage can stem from insufficent settings for max_worker_processes, 
max_parallel_worker or max_parallel_maintenance_workers. It could also 
be caused by another pool (max_logical_replication_workers) or an 
extention using bg worker slots. This new parameter can help database 
administrators and developers diagnose performance issues related to 
parallelism and optimize the configuration of the system accordingly.


Here is a test script:

```
psql << _EOF_

SET log_parallel_worker_draught TO on;

-- Index creation
DROP TABLE IF EXISTS test_pql;
CREATE TABLE test_pql(i int, j int);
INSERT INTO test_pql SELECT x,x FROM generate_series(1,100) as F(x);

SET max_parallel_workers TO 0;

CREATE INDEX ON test_pql(i);
REINDEX TABLE test_pql;

RESET max_parallel_workers;

-- VACUUM
CREATE INDEX ON test_pql(j);
CREATE INDEX ON test_pql(i,j);
ALTER TABLE test_pql SET (autovacuum_enabled = off);
DELETE FROM test_pql WHERE i BETWEEN 1000 AND 2000;

SET max_parallel_workers TO 1;

VACUUM (PARALLEL 2, VERBOSE) test_pql;

RESET max_parallel_workers;

-- SELECT
SET min_parallel_table_scan_size TO 0;
SET parallel_setup_cost TO 0;
SET max_parallel_workers TO 1;

EXPLAIN (ANALYZE) SELECT i, avg(j) FROM test_pql GROUP BY i;

_EOF_
```

Which produces the following logs:

```
LOG:  Parallel Worker draught during statement execution: workers 
spawned 0, requested 1

STATEMENT:  CREATE INDEX ON test_pql(i);

LOG:  Parallel Worker draught during statement execution: workers 
spawned 0, requested 1

STATEMENT:  REINDEX TABLE test_pql;

LOG:  Parallel Worker draught during statement execution: workers 
spawned 1, requested 2

CONTEXT:  while scanning relation "public.test_pql"
STATEMENT:  VACUUM (PARALLEL 2, VERBOSE) test_pql;

LOG:  Parallel Worker draught during statement execution: workers 
spawned 1, requested 2

STATEMENT:  EXPLAIN (ANALYZE) SELECT i, avg(j) FROM test_pql GROUP BY i;
```

[1] 
https://www.postgresql.org/message-id/d657df20-c4bf-63f6-e74c-cb85a81d0...@dalibo.com


--
Benoit Lobréau
Consultant
http://dalibo.comFrom f7d3dae918df2dbbe7fd18cf48f7ca39d5716c73 Mon Sep 17 00:00:00 2001
From: benoit 
Date: Fri, 3 Mar 2023 10:47:50 +0100
Subject: [PATCH] Add logging for exceeded parallel worker slot limits

Introduce the log_parallel_worker_draught parameter, which controls
whether a log message is produced when a backend attempts to spawn a
parallel worker but fails due to insufficient worker slots. The shortage
can stem from max_worker_processes, max_parallel_worker, or
max_parallel_maintenance_workers. This new parameter can help database
administrators and developers diagnose performance issues related to
parallelism and optimize the configuration of the system accordingly.
---
 doc/src/sgml/config.sgml  | 16 
 src/backend/access/transam/parallel.c |  6 ++
 src/backend/utils/misc/guc_tables.c   | 10 ++
 src/backend/utils/misc/postgresql.conf.sample |  2 ++
 src/include/access/parallel.h |  1 +
 5 files changed, 35 insertions(+)

diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index bcc49aec45..0fe0c7796e 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -7499,6 +7499,22 @@ log_line_prefix = '%m [%p] %q%u@%d/%a '
   
  
 
+ 
+  log_parallel_worker_draught 
(boolean)
+  
+   log_parallel_worker_draught configuration 
parameter
+  
+  
+  
+   
+Controls whether a log message is produced when a backend tries to
+spawn a parallel worker but is not successful because either
+max_worker_processes, 
max_parallel_worker
+or max_parallel_maintenance_workers is reached.
+   
+  
+ 
+
  
   log_parameter_max_length (integer)
   
diff --git a/src/backend/access/transam/parallel.c 
b/src/backend/access/transam/parallel.c
index b26f2a64fb..d86ba5a838 100644
--- a/src/backend/access/transam/parallel.c
+++ b/src/backend/access/transam/parallel.c
@@ -630,6 +630,12 @@ LaunchParallelWorkers(ParallelContext *pcxt)
pcxt->nknown_attached_workers = 0;
}
 
+   if (log_parallel_worker_draught &&
+   pcxt->nworkers_launched < pcxt->nworkers_to_launch)
+   ereport(LOG,
+   (errmsg("Parallel Worker draught during 
statement execution: workers spawned %d, requested %d",
+   pcxt->nworkers_launched, 
pcxt->nworkers_to_launch)));
+
/* Restore previous memory context. */
MemoryContextSwitchTo(oldcontext);
 }
diff --git a/src/backend/utils/misc/guc_t

Parallel Query Stats

[Benoit Lobréau]

stat_all_indexes

We could add a parallel_seq_scan counter to pg_stat_all_tables. The column
would be incremented for each worker participating in a scan. The leader
would also increment the counter if it is participating.

The same thing could be done to pg_stat_all_indexes with a 
parallel_index_scan

column.

These metrics could be used in relation to system stats and other PostgreSQL
metrics such as pg_statio_* in tools like POWA.

# Workflow

An overview of the backgroud worker usage could be viewed via the
pg_stat_bgworker view. It could help detect, and in some cases explain, 
parallel

workers draughts. It would also help adapt the size of the worker pools and
prompt us to look into the logs or pg_stat_statements.

The statistics gathered in pg_stat_statements can be used the usual way:
* have an idea of the parallel query usage on the server;
* detect queries that starve from lack of parallel workers;
* compare snapshots to see the impact of parameter modifications;
* combine the statistics with other sources to know:
  * if the decrease in parallel workers had on impact on the average 
execution duration
  * if the increase in parallel workers allocation had an impact on the 
system

time;

The logs can be used to pin point specific queries with their parameters or
to get global statistics when pg_stat_statements is not available or 
can't be

used.

Once a query is singled out, it can be analysed as usual with EXPLAIN to
determine:
* if the lack of workers is a problem;
* how parallelism helps in this particular case.

Finally, the per relation statitics could be combined with system and other
PostgreSQL metrics to identify why the storage is stressed.


If you reach this point, thank you for reading me!

Many thanks to Melanie Plageman for the pointers she shared with us 
around the

pgsessions in Paris and her time in general.

--
Benoit Lobréau
Consultant
http://dalibo.com

Re: archive modules

[Benoit Lobréau]

On Tue, Aug 30, 2022 at 12:46 AM Nathan Bossart 
wrote:

> I would advise that you create a commitfest entry for your patch so that it
> isn't forgotten.
>

Ok done, https://commitfest.postgresql.org/39/3856/ (is that fine if I
added you as a reviewer ?)

and thanks for the added links in the patch.

Re: Probable memory leak with ECPG and AIX

[Benoit Lobréau]

Our client confirmed that the more he fetches the more memory is consumed.
The segfault was indeed caused by the absence of LDR_CNTRL.

The tests show that:

* without LDR_CNTRL, we reach 256Mb and segfault ;
* with LDR_CNTRL=MAXDATA=0x1000, we reach 256Mo but there is no
segfault, the program just continues running ;
* with LDR_CNTRL=MAXDATA=0x8000, we reach 2Go and there is no segfault
either, the program just continues running.

Re: archive_command / pg_stat_archiver & documentation

[Benoit Lobréau]

Thanks !

Le mar. 2 mars 2021 à 04:10, Julien Rouhaud  a écrit :

> On Tue, Mar 2, 2021 at 9:29 AM Michael Paquier 
> wrote:
> >
> > On Mon, Mar 01, 2021 at 05:17:06PM +0800, Julien Rouhaud wrote:
> > > Maybe this can be better addressed than with a link in the
> > > documentation.  The final outcome is that it can be difficult to
> > > monitor the archiver state in such case.  That's orthogonal to this
> > > patch but maybe we can add a new "archiver_start" timestamptz column
> > > in pg_stat_archiver, so monitoring tools can detect a problem if it's
> > > too far away from pg_postmaster_start_time() for instance?
> >
> > There may be other solutions as well.  I have applied the doc patch
> > for now.
>
> Thanks!
>

Re: archive_command / pg_stat_archiver & documentation

[Benoit Lobréau]

I like the idea !

If it's not too complicated, I'd like to take a stab at it.

Le lun. 1 mars 2021 à 10:16, Julien Rouhaud  a écrit :

> On Mon, Mar 1, 2021 at 4:33 PM Benoit Lobréau 
> wrote:
> >
> > Le lun. 1 mars 2021 à 08:36, Michael Paquier  a
> écrit :
> >>
> >> On Fri, Feb 26, 2021 at 10:03:05AM +0100, Benoit Lobréau wrote:
> >> > Done here : https://commitfest.postgresql.org/32/3012/
> >>
> >> Documenting that properly for the archive command, as already done for
> >> restore_command, sounds good to me.  I am not sure that there is much
> >> point in doing a cross-reference to the archiving section for one
> >> specific field of pg_stat_archiver.
> >
> >
> > I wanted to add a warning that using pg_stat_archiver to monitor the
> good health of the
> > archiver comes with a caveat in the view documentation itself. But
> couldn't find a concise
> > way to do it. So I added a link.
> >
> > If you think it's unnecessary, that's ok.
>
> Maybe this can be better addressed than with a link in the
> documentation.  The final outcome is that it can be difficult to
> monitor the archiver state in such case.  That's orthogonal to this
> patch but maybe we can add a new "archiver_start" timestamptz column
> in pg_stat_archiver, so monitoring tools can detect a problem if it's
> too far away from pg_postmaster_start_time() for instance?
>

Re: archive_command / pg_stat_archiver & documentation

[Benoit Lobréau]

Le lun. 1 mars 2021 à 08:36, Michael Paquier  a écrit :

> On Fri, Feb 26, 2021 at 10:03:05AM +0100, Benoit Lobréau wrote:
> > Done here : https://commitfest.postgresql.org/32/3012/
>
> Documenting that properly for the archive command, as already done for
> restore_command, sounds good to me.  I am not sure that there is much
> point in doing a cross-reference to the archiving section for one
> specific field of pg_stat_archiver.
>

I wanted to add a warning that using pg_stat_archiver to monitor the good
health of the
archiver comes with a caveat in the view documentation itself. But couldn't
find a concise
way to do it. So I added a link.

If you think it's unnecessary, that's ok.


> For the second paragraph, I would recommend to move that to a
> different  to outline this special case, leading to the
> attached.
>

Good.

Re: archive_command / pg_stat_archiver & documentation

[Benoit Lobréau]

Done here : https://commitfest.postgresql.org/32/3012/

Le jeu. 25 févr. 2021 à 15:34, Julien Rouhaud  a écrit :

> On Thu, Feb 25, 2021 at 7:25 PM Benoit Lobréau 
> wrote:
> >
> > Le mer. 24 févr. 2021 à 14:52, Julien Rouhaud  a
> écrit :
> >>
> >> I thought that this behavior was documented, especially for the lack
> >> of update of pg_stat_archiver.  If it's not the case then we should
> >> definitely fix that!
> >
> > I tried to do it in the attached patch.
> > Building the doc worked fine on my computer.
>
> Great, thanks!  Can you register it in the next commitfest to make
> sure it won't be forgotten?
>

Re: archive_command / pg_stat_archiver & documentation

[Benoit Lobréau]

Le mer. 24 févr. 2021 à 14:52, Julien Rouhaud  a écrit :

> Hi,
>
> On Wed, Feb 24, 2021 at 8:21 PM talk to ben  wrote:
> >
> > The documentation describes how a return code > 125 on the
> restore_command would prevent the server from starting [1] :
> >
> > "
> > It is important that the command return nonzero exit status on failure.
> The command will be called requesting files that are not present in the
> archive; it must return nonzero when so asked. This is not an error
> condition. An exception is that if the command was terminated by a signal
> (other than SIGTERM, which is used as part of a database server shutdown)
> or an error by the shell (such as command not found), then recovery will
> abort and the server will not start up.
> > "
> >
> > But, I dont see such a note on the archive_command side of thing. [2]
> >
> > It could happend in case the archive command is not checked beforehand
> or if the archive command becomes unavailable while PostgreSQL is running.
> rsync can also return 255 in some cases (bad ssh configuration or typos).
> In this case a fatal error is emitted, the archiver stops and is restarted
> by the postmaster.
> >
> > The view pg_stat_archiver is also not updated in this case. Is it on
> purpose ? It could be problematic if someone uses it to check the archiver
> process health.
>
> That's on purpose, see for instance that discussion:
> https://www.postgresql.org/message-id/flat/55731BB8.1050605%40dalibo.com
>

Thanks for pointing that out, I should have checked.


> > Should we document this ? (I can make a patch)
>
> I thought that this behavior was documented, especially for the lack
> of update of pg_stat_archiver.  If it's not the case then we should
> definitely fix that!
>

I tried to do it in the attached patch.
Building the doc worked fine on my computer.
From 350cd7c47d09754ae21f30f260a86e187054257f Mon Sep 17 00:00:00 2001
From: benoit 
Date: Thu, 25 Feb 2021 12:08:03 +0100
Subject: [PATCH] Document archive_command failures in more details

Document that, if the command was terminated by a signal (other than SIGTERM, which
is used as part of a database server shutdown) or an error by the shell with an exit
status greater than 125 (such as command not found), then the archiver process will
abort and the postmaster will restart it. In such cases, the failure will not be
reported in pg_stat_archiver.
---
 doc/src/sgml/backup.sgml | 8 +++-
 doc/src/sgml/monitoring.sgml | 3 ++-
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/doc/src/sgml/backup.sgml b/doc/src/sgml/backup.sgml
index 3c8aaed0b6..94d5dcbdf0 100644
--- a/doc/src/sgml/backup.sgml
+++ b/doc/src/sgml/backup.sgml
@@ -636,7 +636,13 @@ test ! -f /mnt/server/archivedir/000100A90065  cp pg_wal/0
 PostgreSQL will assume that the file has been
 successfully archived, and will remove or recycle it.  However, a nonzero
 status tells PostgreSQL that the file was not archived;
-it will try again periodically until it succeeds.
+it will try again periodically until it succeeds. 
+An exception is that if the command was terminated by
+a signal (other than SIGTERM, which is used as
+part of a database server shutdown) or an error by the shell with an exit
+status greater than 125 (such as command not found), then the archiver
+process will abort and the postmaster will restart it. In such cases,
+the failure will not be reported in .

 

diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml
index 3513e127b7..391df3055b 100644
--- a/doc/src/sgml/monitoring.sgml
+++ b/doc/src/sgml/monitoring.sgml
@@ -3251,7 +3251,8 @@ SELECT pid, wait_event_type, wait_event FROM pg_stat_activity WHERE wait_event i
failed_count bigint
   
   
-   Number of failed attempts for archiving WAL files
+  Number of failed attempts for archiving WAL files (See )
   
  
 
-- 
2.25.4

Re: recovery_target_timeline & documentation

[Benoit Lobréau]

> Pushed. Thanks!
>

Thank you !

recovery_target_timeline & documentation

[Benoit Lobréau]

Hi,

It seems like this part of the documentation was not updated after changing
the default value of recovery_target_timeline to latest in  v12.

"The default behavior of recovery is to recover along the same timeline
that was current when the base backup was taken. If you wish to recover
into some child timeline (that is, you want to return to some state that
was itself generated after a recovery attempt), you need to specify the
target timeline ID in recovery_target_timeline
.
You cannot recover into timelines that branched off earlier than the base
backup." [1][2]

Here is an attempt to fix that.

regards,
Benoit

[1]
https://www.postgresql.org/docs/13/continuous-archiving.html#BACKUP-TIMELINES
[2]
https://www.postgresql.org/docs/12/continuous-archiving.html#BACKUP-TIMELINES
From 21bbc9badfd5bd2eeb2c702295ef9e8233ed9552 Mon Sep 17 00:00:00 2001
From: benoit 
Date: Tue, 5 Jan 2021 11:00:07 +0100
Subject: [PATCH] Fix recovery_target_timeline default in backup.sgml

---
 doc/src/sgml/backup.sgml | 13 +++--
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/doc/src/sgml/backup.sgml b/doc/src/sgml/backup.sgml
index 42a8ed328d..3c8aaed0b6 100644
--- a/doc/src/sgml/backup.sgml
+++ b/doc/src/sgml/backup.sgml
@@ -1437,12 +1437,13 @@ restore_command = 'cp /mnt/server/archivedir/%f %p'

 

-The default behavior of recovery is to recover along the same timeline
-that was current when the base backup was taken.  If you wish to recover
-into some child timeline (that is, you want to return to some state that
-was itself generated after a recovery attempt), you need to specify the
-target timeline ID in . You cannot recover into
-timelines that branched off earlier than the base backup.
+The default behavior of recovery is to recover to the latest timeline found
+in the archive. If you wish to recover to the timeline that was current
+when the base backup was taken or into a specific child timeline (that
+is, you want to return to some state that was itself generated after a
+recovery attempt), you need to specify current or the
+target timeline ID in . You
+cannot recover into timelines that branched off earlier than the base backup.

   
 
-- 
2.25.4

Re: pg_shmem_allocations & documentation

[Benoit Lobréau]

Here's a proposal patch.

Le ven. 11 déc. 2020 à 09:58, Benoit Lobréau  a
écrit :

> Would "NULL for anonymous allocations, since details related to them are
> not known." be ok ?
>
>
> Le ven. 11 déc. 2020 à 09:29, Kyotaro Horiguchi 
> a écrit :
>
>> At Fri, 11 Dec 2020 14:42:45 +0900, Michael Paquier 
>> wrote in
>> > On Fri, Dec 11, 2020 at 11:00:58AM +0900, Kyotaro Horiguchi wrote:
>> > > Although we could just rip some words off, I'd like to propose instead
>> > > to add an explanation why it is not exposed for anonymous allocations,
>> > > like the column allocated_size.
>> >
>> > Indeed, there is a hiccup between what the code does and what the docs
>> > tell: the offset is not NULL for unused memory.
>> >
>> > > -   The offset at which the allocation starts. NULL for anonymous
>> > > -   allocations and unused memory.
>> > > +   The offset at which the allocation starts. For anonymous
>> allocations,
>> > > +   no information about individual allocations is available, so
>> the column
>> > > +   will be NULL in that case.
>> >
>> > I'd say: let's be simple and just remove "and unused memory" because
>> > anonymous allocations are...  Anonymous so you cannot know details
>> > related to them.  That's something easy to reason about, and the docs
>> > were written originally to remain simple.
>>
>> Hmm. I don't object to that.  Howerver, isn't the description for
>> allocated_size too verbose in that sense?
>>
>> regards.
>>
>> --
>> Kyotaro Horiguchi
>> NTT Open Source Software Center
>>
>
From 711f6b60098e67c23a97458ce56893f0ac1afcb6 Mon Sep 17 00:00:00 2001
From: benoit 
Date: Fri, 11 Dec 2020 09:44:51 +0100
Subject: [PATCH] Fix pg_shmem_allocation

---
 doc/src/sgml/catalogs.sgml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml
index 62711ee83f..89ca59b92b 100644
--- a/doc/src/sgml/catalogs.sgml
+++ b/doc/src/sgml/catalogs.sgml
@@ -12493,7 +12493,7 @@ SELECT * FROM pg_locks pl LEFT JOIN pg_prepared_xacts ppx
   
   
The offset at which the allocation starts. NULL for anonymous
-   allocations and unused memory.
+   allocations, since details related to them are not known.
   
  
 
-- 
2.25.4

Re: pg_shmem_allocations & documentation

[Benoit Lobréau]

Would "NULL for anonymous allocations, since details related to them are
not known." be ok ?


Le ven. 11 déc. 2020 à 09:29, Kyotaro Horiguchi  a
écrit :

> At Fri, 11 Dec 2020 14:42:45 +0900, Michael Paquier 
> wrote in
> > On Fri, Dec 11, 2020 at 11:00:58AM +0900, Kyotaro Horiguchi wrote:
> > > Although we could just rip some words off, I'd like to propose instead
> > > to add an explanation why it is not exposed for anonymous allocations,
> > > like the column allocated_size.
> >
> > Indeed, there is a hiccup between what the code does and what the docs
> > tell: the offset is not NULL for unused memory.
> >
> > > -   The offset at which the allocation starts. NULL for anonymous
> > > -   allocations and unused memory.
> > > +   The offset at which the allocation starts. For anonymous
> allocations,
> > > +   no information about individual allocations is available, so
> the column
> > > +   will be NULL in that case.
> >
> > I'd say: let's be simple and just remove "and unused memory" because
> > anonymous allocations are...  Anonymous so you cannot know details
> > related to them.  That's something easy to reason about, and the docs
> > were written originally to remain simple.
>
> Hmm. I don't object to that.  Howerver, isn't the description for
> allocated_size too verbose in that sense?
>
> regards.
>
> --
> Kyotaro Horiguchi
> NTT Open Source Software Center
>

pg_shmem_allocations & documentation

[Benoit Lobréau]

Hi,

While reading the documentation of pg_shmem_allocations, I noticed that the
off column is described as such :

"The offset at which the allocation starts. NULL for anonymous allocations
and unused memory."

Whereas, the view returns a value for unused memory:

[local]:5433 postgres@postgres=# SELECT * FROM pg_shmem_allocations WHERE
name IS NULL;
 name |off|  size   | allocated_size
--+---+-+
 ¤| 178095232 | 1923968 |1923968
(1 row)

>From what I understand, the doc is wrong.
Am I right ?

Benoit

[1] https://www.postgresql.org/docs/13/view-pg-shmem-allocations.html
[2]
https://www.postgresql.org/message-id/flat/20140504114417.GM12715%40awork2.anarazel.de
(original thread)