Re: warn if GUC set to an invalid shared library

From: Justin Pryzby <pryzby(at)telsasoft(dot)com>
To: Robert Haas <robertmhaas(at)gmail(dot)com>, Maciek Sakrejda <m(dot)sakrejda(at)gmail(dot)com>, "David G(dot) Johnston" <david(dot)g(dot)johnston(at)gmail(dot)com>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com>, pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: warn if GUC set to an invalid shared library
Date: 2022-02-10 01:58:54
Message-ID: 20220210015854.GV31460@telsasoft.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Fri, Jan 28, 2022 at 09:42:17AM -0500, Robert Haas wrote:
> -1 from me on using "guc" in any user-facing error message. And even
> guc -> setting isn't a big improvement. If we're going to structure
> the reporting this way there, we should try to use a meaningful phrase
> there, probably beginning with the word "while"; see "git grep
> errcontext.*while" for interesting precedents.

On Wed, Feb 02, 2022 at 06:06:01AM +0000, Maciek Sakrejda wrote:
> I do agree that GUC is awkward here, and I like the "while..." wording suggested both for consistency with other messages and how it could work here:
> CONTEXT: while loading "shared_preload_libraries"

FYI, it has said "while..." and hasn't said "guc" since the 2nd revision of the
patch.

> That said, that series of messages seems a bit suspect to me, because
> the WARNING seems to be stating the same problem as the subsequent
> FATAL and CONTEXT lines. Ideally we'd tighten that somehow.

I avoided the warning by checking IsUnderPostmaster, though I'm not sure if
that's the right condition..

On Wed, Feb 02, 2022 at 06:06:01AM +0000, Maciek Sakrejda wrote:
> I think this is great, but it would be really helpful to also indicate that at this point the server will fail to come back up after a restart.

> I don't really know a good wording here, but maybe a hint like "The server or session will not be able to start if it has been configured to use libraries that cannot be loaded."?

postgres=# ALTER SYSTEM SET shared_preload_libraries =a,b;
WARNING: could not access file "$libdir/plugins/a"
HINT: The server will fail to start with the existing configuration. If it is is shut down, it will be necessary to manually edit the postgresql.auto.conf file to allow it to start.
WARNING: could not access file "$libdir/plugins/b"
HINT: The server will fail to start with the existing configuration. If it is is shut down, it will be necessary to manually edit the postgresql.auto.conf file to allow it to start.
ALTER SYSTEM
postgres=# ALTER SYSTEM SET session_preload_libraries =c,d;
WARNING: could not access file "$libdir/plugins/c"
HINT: New sessions will fail with the existing configuration.
WARNING: could not access file "$libdir/plugins/d"
HINT: New sessions will fail with the existing configuration.
ALTER SYSTEM

$ ./tmp_install/usr/local/pgsql/bin/postgres -D src/test/regress/tmp_check/data -clogging_collector=on
2022-02-09 19:53:48.034 CST postmaster[30979] FATAL: could not access file "a": No such file or directory
2022-02-09 19:53:48.034 CST postmaster[30979] CONTEXT: while loading shared libraries for setting "shared_preload_libraries"
from /home/pryzbyj/src/postgres/src/test/regress/tmp_check/data/postgresql.auto.conf:3
2022-02-09 19:53:48.034 CST postmaster[30979] LOG: database system is shut down

Maybe it's enough to show the GucSource rather than file:line...

--
Justin

Attachment Content-Type Size
v4-0001-warn-when-setting-GUC-to-a-nonextant-library.patch text/x-diff 9.2 KB
v4-0002-errcontext-if-server-fails-to-start-due-to-librar.patch text/x-diff 4.2 KB
v4-0003-show-the-GUC-source-in-errcontext.patch text/x-diff 3.2 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Amit Kapila 2022-02-10 02:14:22 Re: [BUG]Update Toast data failure in logical replication
Previous Message Andres Freund 2022-02-10 01:47:52 Re: [RFC] building postgres with meson - perl embedding