agora inbox for [email protected]
help / color / mirror / Atom feedPostgres with pthread
51+ messages / 23 participants
[nested] [flat]
* Postgres with pthread
@ 2017-12-06 16:40 Konstantin Knizhnik <[email protected]>
0 siblings, 3 replies; 51+ messages in thread
From: Konstantin Knizhnik @ 2017-12-06 16:40 UTC (permalink / raw)
To: pgsql-hackers
Hi hackers,
As far as I remember, several years ago when implementation of
intra-query parallelism was just started there was discussion whether to
use threads or leave traditional Postgres process architecture. The
decision was made to leave processes. So now we have bgworkers, shared
message queue, DSM, ...
The main argument for such decision was that switching to threads will
require rewriting of most of Postgres code.
It seems to be quit reasonable argument and and until now I agreed with it.
But recently I wanted to check it myself.
The first problem with porting Postgres to pthreads is static variables
widely used in Postgres code.
Most of modern compilers support thread local variables, for example GCC
provides __thread keyword.
Such variables are placed in separate segment which is address through
segment register (at Intel).
So access time to such variables is the same as to normal static variables.
Certainly may be not all compilers have builtin support of TLS and may
be not at all hardware platforms them are implemented ias efficiently as
at Intel.
So certainly such approach decreases portability of Postgres. But IMHO
it is not so critical.
What I have done:
1. Add session_local (defined as __thread) to definition of most of
static and global variables.
I leaved some variables pointed to shared memory as static. Also I have
to changed initialization of some static variables,
because address of TLS variable can not be used in static initializers.
2. Change implementation of GUCs to make them thread specific.
3. Replace fork() with pthread_create
4. Rewrite file descriptor cache to be global (shared by all threads).
I have not changed all Postgres synchronization primitives and shared
memory.
It took me about one week of work.
What is not done yet:
1. Handling of signals (I expect that Win32 code can be somehow reused
here).
2. Deallocation of memory and closing files on backend (thread) termination.
3. Interaction of postmaster and backends with PostgreSQL auxiliary
processes (threads), such as autovacuum, bgwriter, checkpointer, stat
collector,...
What are the advantages of using threads instead of processes?
1. No need to use shared memory. So there is no static limit for amount
of memory which can be used by Postgres. No need in distributed shared
memory and other stuff designed to share memory between backends and
bgworkers.
2. Threads significantly simplify implementation of parallel algorithms:
interaction and transferring data between threads can be done easily and
more efficiently.
3. It is possible to use more efficient/lightweight synchronization
primitives. Postgres now mostly relies on its own low level
sync.primitives which user-level implementation
is using spinlocks and atomics and then fallback to OS semaphores/poll.
I am not sure how much gain can we get by replacing this primitives with
one optimized for threads.
My colleague from Firebird community told me that just replacing
processes with threads can obtain 20% increase of performance, but it is
just first step and replacing sync. primitive
can give much greater advantage. But may be for Postgres with its low
level primitives it is not true.
4. Threads are more lightweight entities than processes. Context switch
between threads takes less time than between process. And them consume
less memory. It is usually possible to spawn more threads than processes.
5. More efficient access to virtual memory. As far as all threads are
sharing the same memory space, TLB is used much efficiently in this case.
6. Faster backend startup. Certainly starting backend at each user's
request is bad thing in any case. Some kind of connection pooling should
be used in any case to provide acceptable performance. But in any case,
start of new backend process in postgres causes a lot of page faults
which have dramatical impact on performance. And there is no such
problem with threads.
Certainly, processes are also having some advantages comparing with threads:
1. Better isolation and error protection
2. Easier error handling
3. Easier control of used resources
But it is a theory. The main idea of this prototype was to prove or
disprove this expectation at practice.
I didn't expect large differences in performance because synchronization
primitives are not changed and I performed my experiments at Linux where
threads/processes are implemented in similar way.
Below are some results (1000xTPS) of select-only (-S) pgbench with scale
100 at my desktop with quad-core i7-4770 3.40GHz and 16Gb of RAM:
Connections Vanilla/default Vanilla/prepared
pthreads/defaultpthreads/prepared
10 100 191
106 207
100 67 131
105 168
1000 41 65
55 102
As you can see, for small number of connection results are almost
similar. But for large number of connection pthreads provide less
degradation.
You can look at my prototype here:
https://github.com/postgrespro/postgresql.pthreads.git
But please notice that it is very raw prototype. A lot of stuff is not
working yet. And supporting all of exited Postgres functionality requires
much more efforts (and even more efforts are needed for optimizing
Postgres for this architecture).
I just want to receive some feedback and know if community is interested
in any further work in this direction.
--
Konstantin Knizhnik
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company
^ permalink raw reply [nested|flat] 51+ messages in thread
* Re: Postgres with pthread
@ 2017-12-06 16:53 Tom Lane <[email protected]>
parent: Konstantin Knizhnik <[email protected]>
2 siblings, 3 replies; 51+ messages in thread
From: Tom Lane @ 2017-12-06 16:53 UTC (permalink / raw)
To: Konstantin Knizhnik <[email protected]>; +Cc: pgsql-hackers
Konstantin Knizhnik <[email protected]> writes:
> Below are some results (1000xTPS) of select-only (-S) pgbench with scale
> 100 at my desktop with quad-core i7-4770 3.40GHz and 16Gb of RAM:
> Connections Vanilla/default Vanilla/prepared
> pthreads/defaultpthreads/prepared
> 10 100 191
> 106 207
> 100 67 131
> 105 168
> 1000 41 65
> 55 102
This table is so mangled that I'm not very sure what it's saying.
Maybe you should have made it an attachment?
However, if I guess at which numbers are supposed to be what,
it looks like even the best case is barely a 50% speedup.
That would be worth pursuing if it were reasonably low-hanging
fruit, but converting PG to threads seems very far from being that.
I think you've done us a very substantial service by pursuing
this far enough to get some quantifiable performance results.
But now that we have some results in hand, I think we're best
off sticking with the architecture we've got.
regards, tom lane
^ permalink raw reply [nested|flat] 51+ messages in thread
* Re: Postgres with pthread
@ 2017-12-06 17:02 Adam Brusselback <[email protected]>
parent: Tom Lane <[email protected]>
2 siblings, 0 replies; 51+ messages in thread
From: Adam Brusselback @ 2017-12-06 17:02 UTC (permalink / raw)
To: Tom Lane <[email protected]>; +Cc: Konstantin Knizhnik <[email protected]>; pgsql-hackers
Here it is formatted a little better.
So a little over 50% performance improvement for a couple of the test cases.
On Wed, Dec 6, 2017 at 11:53 AM, Tom Lane <[email protected]> wrote:
> Konstantin Knizhnik <[email protected]> writes:
> > Below are some results (1000xTPS) of select-only (-S) pgbench with scale
> > 100 at my desktop with quad-core i7-4770 3.40GHz and 16Gb of RAM:
>
> > Connections Vanilla/default Vanilla/prepared
> > pthreads/defaultpthreads/prepared
> > 10 100 191
> > 106 207
> > 100 67 131
> > 105 168
> > 1000 41 65
> > 55 102
>
> This table is so mangled that I'm not very sure what it's saying.
> Maybe you should have made it an attachment?
>
> However, if I guess at which numbers are supposed to be what,
> it looks like even the best case is barely a 50% speedup.
> That would be worth pursuing if it were reasonably low-hanging
> fruit, but converting PG to threads seems very far from being that.
>
> I think you've done us a very substantial service by pursuing
> this far enough to get some quantifiable performance results.
> But now that we have some results in hand, I think we're best
> off sticking with the architecture we've got.
>
> regards, tom lane
>
>
Attachments:
[image/png] pthreads.PNG (5.6K, ../../CAMjNa7cHrtXRxJM50KnRBrOUhp8JO8PfYF-1b3Qpi0HQmJv_mw@mail.gmail.com/3-pthreads.PNG)
download | view image
^ permalink raw reply [nested|flat] 51+ messages in thread
* Re: Postgres with pthread
@ 2017-12-06 17:08 Andres Freund <[email protected]>
parent: Konstantin Knizhnik <[email protected]>
2 siblings, 4 replies; 51+ messages in thread
From: Andres Freund @ 2017-12-06 17:08 UTC (permalink / raw)
To: Konstantin Knizhnik <[email protected]>; +Cc: pgsql-hackers
Hi!
On 2017-12-06 19:40:00 +0300, Konstantin Knizhnik wrote:
> As far as I remember, several years ago when implementation of intra-query
> parallelism was just started there was discussion whether to use threads or
> leave traditional Postgres process architecture. The decision was made to
> leave processes. So now we have bgworkers, shared message queue, DSM, ...
> The main argument for such decision was that switching to threads will
> require rewriting of most of Postgres code.
> It seems to be quit reasonable argument and and until now I agreed with it.
>
> But recently I wanted to check it myself.
I think that's something pretty important to play with. There've been
several discussions lately, both on and off list / in person, that we're
taking on more-and-more technical debt just because we're using
processes. Besides the above, we've grown:
- a shared memory allocator
- a shared memory hashtable
- weird looking thread aware pointers
- significant added complexity in various projects due to addresses not
being mapped to the same address etc.
> The first problem with porting Postgres to pthreads is static variables
> widely used in Postgres code.
> Most of modern compilers support thread local variables, for example GCC
> provides __thread keyword.
> Such variables are placed in separate segment which is address through
> segment register (at Intel).
> So access time to such variables is the same as to normal static variables.
I experimented similarly. Although I'm not 100% sure that if were to go
for it, we wouldn't instead want to abstract our session concept
further, or well, at all.
> Certainly may be not all compilers have builtin support of TLS and may be
> not at all hardware platforms them are implemented ias efficiently as at
> Intel.
> So certainly such approach decreases portability of Postgres. But IMHO it is
> not so critical.
I'd agree there, but I don't think the project necessarily does.
> What I have done:
> 1. Add session_local (defined as __thread) to definition of most of static
> and global variables.
> I leaved some variables pointed to shared memory as static. Also I have to
> changed initialization of some static variables,
> because address of TLS variable can not be used in static initializers.
> 2. Change implementation of GUCs to make them thread specific.
> 3. Replace fork() with pthread_create
> 4. Rewrite file descriptor cache to be global (shared by all threads).
That one I'm very unconvinced of, that's going to add a ton of new
contention.
> What are the advantages of using threads instead of processes?
>
> 1. No need to use shared memory. So there is no static limit for amount of
> memory which can be used by Postgres. No need in distributed shared memory
> and other stuff designed to share memory between backends and
> bgworkers.
This imo is the biggest part. We can stop duplicating OS and our own
implementations in a shmem aware way.
> 2. Threads significantly simplify implementation of parallel algorithms:
> interaction and transferring data between threads can be done easily and
> more efficiently.
That's imo the same as 1.
> 3. It is possible to use more efficient/lightweight synchronization
> primitives. Postgres now mostly relies on its own low level sync.primitives
> which user-level implementation
> is using spinlocks and atomics and then fallback to OS semaphores/poll. I am
> not sure how much gain can we get by replacing this primitives with one
> optimized for threads.
> My colleague from Firebird community told me that just replacing processes
> with threads can obtain 20% increase of performance, but it is just first
> step and replacing sync. primitive
> can give much greater advantage. But may be for Postgres with its low level
> primitives it is not true.
I don't believe that that's actually the case to any significant degree.
> 6. Faster backend startup. Certainly starting backend at each user's request
> is bad thing in any case. Some kind of connection pooling should be used in
> any case to provide acceptable performance. But in any case, start of new
> backend process in postgres causes a lot of page faults which have
> dramatical impact on performance. And there is no such problem with threads.
I don't buy this in itself. The connection establishment overhead isn't
largely the fork, it's all the work afterwards. I do think it makes
connection pooling etc easier.
> I just want to receive some feedback and know if community is interested in
> any further work in this direction.
I personally am. I think it's beyond high time that we move to take
advantage of threads.
That said, I don't think just replacing threads is the right thing. I'm
pretty sure we'd still want to have postmaster as a separate process,
for robustness. Possibly we even want to continue having various
processes around besides that, the most interesting cases involving
threads are around intra-query parallelism, and pooling, and for both a
hybrid model could be beneficial.
I think that we probably initially want some optional move to
threads. Most extensions won't initially be thread ready, and imo we
should continue to work with that for a while, just refusing to use
parallelism if any loaded shared library doesn't signal parallelism
support. We also don't necessarily want to require threads on all
platforms at the same time.
I think the biggest problem with doing this for real is that it's a huge
project, and that it'll take a long time.
Thanks for working on this!
Andres Freund
^ permalink raw reply [nested|flat] 51+ messages in thread
* Re: Postgres with pthread
@ 2017-12-06 17:13 Robert Haas <[email protected]>
parent: Tom Lane <[email protected]>
2 siblings, 0 replies; 51+ messages in thread
From: Robert Haas @ 2017-12-06 17:13 UTC (permalink / raw)
To: Tom Lane <[email protected]>; +Cc: Konstantin Knizhnik <[email protected]>; pgsql-hackers
On Wed, Dec 6, 2017 at 11:53 AM, Tom Lane <[email protected]> wrote:
> barely a 50% speedup.
I think that's an awfully strange choice of adverb. This is, by its
authors own admission, a rough cut at this, probably with very little
of the optimization that could ultimately done, and it's already
buying 50% on some test cases? That sounds phenomenally good to me.
A 50% speedup is huge, and chances are that it can be made quite a bit
better with more work, or that it already is quite a bit better with
the right test case.
TBH, based on previous discussion, I expected this to initially be
*slower* but still worthwhile in the long run because of optimizations
that it would let us do eventually with parallel query and other
things. If it's this much faster out of the gate, that's really
exciting.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
^ permalink raw reply [nested|flat] 51+ messages in thread
* Re: Postgres with pthread
@ 2017-12-06 17:17 Andres Freund <[email protected]>
parent: Tom Lane <[email protected]>
2 siblings, 2 replies; 51+ messages in thread
From: Andres Freund @ 2017-12-06 17:17 UTC (permalink / raw)
To: Tom Lane <[email protected]>; +Cc: Konstantin Knizhnik <[email protected]>; pgsql-hackers
Hi,
On 2017-12-06 11:53:21 -0500, Tom Lane wrote:
> Konstantin Knizhnik <[email protected]> writes:
> However, if I guess at which numbers are supposed to be what,
> it looks like even the best case is barely a 50% speedup.
"barely a 50% speedup" - Hah. I don't believe the numbers, but that'd be
huge.
> That would be worth pursuing if it were reasonably low-hanging
> fruit, but converting PG to threads seems very far from being that.
I don't think immediate performance gains are the interesting part about
using threads. It's rather what their absence adds a lot in existing /
submitted code complexity, and makes some very commonly requested
features a lot harder to implement:
- we've a lot of duplicated infrastructure around dynamic shared
memory. dsm.c dsa.c, dshash.c etc. A lot of these, especially dsa.c,
are going to become a lot more complicated over time, just look at how
complicated good multi threaded allocators are.
- we're adding a lot of slowness to parallelism, just because we have
different memory layouts in different processes. Instead of just
passing pointers through queues, we put entire tuples in there. We
deal with dsm aware pointers.
- a lot of features have been a lot harder (parallelism!), and a lot of
frequently requested ones are so hard due to processes that they never
got off ground (in-core pooling, process reuse, parallel worker reuse)
- due to the statically sized shared memory a lot of our configuration
is pretty fundamentally PGC_POSTMASTER, even though that present a lot
of administrative problems.
...
> I think you've done us a very substantial service by pursuing
> this far enough to get some quantifiable performance results.
> But now that we have some results in hand, I think we're best
> off sticking with the architecture we've got.
I don't agree.
I'd personally expect that an immediate conversion would result in very
little speedup, a bunch of code deleted, a bunch of complexity
added. And it'd still be massively worthwhile, to keep medium to long
term complexity and feature viability in control.
Greetings,
Andres Freund
^ permalink raw reply [nested|flat] 51+ messages in thread
* Re: Postgres with pthread
@ 2017-12-06 17:24 Adam Brusselback <[email protected]>
parent: Andres Freund <[email protected]>
1 sibling, 0 replies; 51+ messages in thread
From: Adam Brusselback @ 2017-12-06 17:24 UTC (permalink / raw)
To: Andres Freund <[email protected]>; +Cc: Tom Lane <[email protected]>; Konstantin Knizhnik <[email protected]>; pgsql-hackers
> "barely a 50% speedup" - Hah. I don't believe the numbers, but that'd be
> huge.
They are numbers derived from a benchmark that any sane person would
be using a connection pool for in a production environment, but
impressive if true none the less.
^ permalink raw reply [nested|flat] 51+ messages in thread
* Re: Postgres with pthread
@ 2017-12-06 17:26 Andreas Karlsson <[email protected]>
parent: Andres Freund <[email protected]>
3 siblings, 1 reply; 51+ messages in thread
From: Andreas Karlsson @ 2017-12-06 17:26 UTC (permalink / raw)
To: Andres Freund <[email protected]>; Konstantin Knizhnik <[email protected]>; +Cc: pgsql-hackers
On 12/06/2017 06:08 PM, Andres Freund wrote:
> I think the biggest problem with doing this for real is that it's a huge
> project, and that it'll take a long time.
An additional issue is that this could break a lot of extensions and in
a way that it is not apparent at compile time. This means we may need to
break all extensions to force extensions authors to check if they are
thread safe.
I do not like making life hard for out extension community, but if the
gains are big enough it might be worth it.
> Thanks for working on this!
Seconded.
Andreas
^ permalink raw reply [nested|flat] 51+ messages in thread
* Re: Postgres with pthread
@ 2017-12-06 17:28 Robert Haas <[email protected]>
parent: Andres Freund <[email protected]>
3 siblings, 1 reply; 51+ messages in thread
From: Robert Haas @ 2017-12-06 17:28 UTC (permalink / raw)
To: Andres Freund <[email protected]>; +Cc: Konstantin Knizhnik <[email protected]>; pgsql-hackers
On Wed, Dec 6, 2017 at 12:08 PM, Andres Freund <[email protected]> wrote:
>> 4. Rewrite file descriptor cache to be global (shared by all threads).
>
> That one I'm very unconvinced of, that's going to add a ton of new
> contention.
It might be OK on systems where we can use pread()/pwrite().
Otherwise it's going to be terrible.
> That said, I don't think just replacing threads is the right thing. I'm
> pretty sure we'd still want to have postmaster as a separate process,
> for robustness.
+1. The tendency of the postmaster to not die has been a huge boon to
the reliability of PostgreSQL - I would not like to give that up.
MySQL ends up needing safe_mysqld to cope with this issue; our idea of
having it built into the server is better.
> Possibly we even want to continue having various
> processes around besides that, the most interesting cases involving
> threads are around intra-query parallelism, and pooling, and for both a
> hybrid model could be beneficial.
I think if we only use threads for intra-query parallelism we're
leaving a lot of money on the table. For example, if all
shmem-connected backends are using the same process, then we can make
max_locks_per_transaction PGC_SIGHUP. That would be sweet, and there
are probably plenty of similar things. Moreover, if threads are this
thing that we only use now and then for parallel query, then our
support for them will probably have bugs. If we use them all the
time, we'll actually find the bugs and fix them. I hope.
> I think the biggest problem with doing this for real is that it's a huge
> project, and that it'll take a long time.
+1
> Thanks for working on this!
+1
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
^ permalink raw reply [nested|flat] 51+ messages in thread
* Re: Postgres with pthread
@ 2017-12-06 17:42 Andres Freund <[email protected]>
parent: Robert Haas <[email protected]>
0 siblings, 0 replies; 51+ messages in thread
From: Andres Freund @ 2017-12-06 17:42 UTC (permalink / raw)
To: Robert Haas <[email protected]>; +Cc: Konstantin Knizhnik <[email protected]>; pgsql-hackers
Hi,
On 2017-12-06 12:28:29 -0500, Robert Haas wrote:
> > Possibly we even want to continue having various
> > processes around besides that, the most interesting cases involving
> > threads are around intra-query parallelism, and pooling, and for both a
> > hybrid model could be beneficial.
>
> I think if we only use threads for intra-query parallelism we're
> leaving a lot of money on the table. For example, if all
> shmem-connected backends are using the same process, then we can make
> max_locks_per_transaction PGC_SIGHUP. That would be sweet, and there
> are probably plenty of similar things. Moreover, if threads are this
> thing that we only use now and then for parallel query, then our
> support for them will probably have bugs. If we use them all the
> time, we'll actually find the bugs and fix them. I hope.
I think it'd make a lot of sense to go there gradually. I agree that we
probably want to move to more and more use of threads, but we also want
our users not to kill us ;). Initially we'd surely continue to use
partitioned dynahash for locks, which'd make resizing infeasible
anyway. Similar for shared buffers (which I find a hell of a lot more
interesting to change at runtime than max_locks_per_transaction), etc...
- Andres
^ permalink raw reply [nested|flat] 51+ messages in thread
* Re: Postgres with pthread
@ 2017-12-06 21:58 Thomas Munro <[email protected]>
parent: Andres Freund <[email protected]>
3 siblings, 2 replies; 51+ messages in thread
From: Thomas Munro @ 2017-12-06 21:58 UTC (permalink / raw)
To: Andres Freund <[email protected]>; +Cc: Konstantin Knizhnik <[email protected]>; pgsql-hackers
On Thu, Dec 7, 2017 at 6:08 AM, Andres Freund <[email protected]> wrote:
> On 2017-12-06 19:40:00 +0300, Konstantin Knizhnik wrote:
>> As far as I remember, several years ago when implementation of intra-query
>> parallelism was just started there was discussion whether to use threads or
>> leave traditional Postgres process architecture. The decision was made to
>> leave processes. So now we have bgworkers, shared message queue, DSM, ...
>> The main argument for such decision was that switching to threads will
>> require rewriting of most of Postgres code.
>
>> It seems to be quit reasonable argument and and until now I agreed with it.
>>
>> But recently I wanted to check it myself.
>
> I think that's something pretty important to play with. There've been
> several discussions lately, both on and off list / in person, that we're
> taking on more-and-more technical debt just because we're using
> processes. Besides the above, we've grown:
> - a shared memory allocator
> - a shared memory hashtable
> - weird looking thread aware pointers
> - significant added complexity in various projects due to addresses not
> being mapped to the same address etc.
Yes, those are all workarounds for an ancient temporary design choice.
To quote from a 1989 paper[1] "Currently, POSTGRES runs as one process
for each active user. This was done as an expedient to get a system
operational as quickly as possible. We plan on converting POSTGRES to
use lightweight processes [...]". +1 for sticking to the plan.
While personally contributing to the technical debt items listed
above, I always imagined that all that machinery could become
compile-time options controlled with --with-threads and
dsa_get_address() would melt away leaving only a raw pointers, and
dsa_area would forward to the MemoryContext + ResourceOwner APIs, or
something like that. It's unfortunate that we lose type safety along
the way though. (If only there were some way we could write
dsa_pointer<my_type>. In fact it was also a goal of the original
project to adopt C++, based on a comment in 4.2's nodes.h: "Eventually
this code should be transmogrified into C++ classes, and this is more
or less compatible with those things.")
If there were a good way to reserve (but not map) a large address
range before forking, there could also be an intermediate build mode
that keeps the multi-process model but where DSA behaves as above,
which might an interesting way to decouple the
DSA-go-faster-and-reduce-tech-debt project from the threading project.
We could manage the reserved address space ourselves and map DSM
segments with MAP_FIXED, so dsa_get_address() address decoding could
be compiled away. One way would be to mmap a huge range backed with
/dev/zero, and then map-with-MAP_FIXED segments over the top of it and
then remap /dev/zero back into place when finished, but that sucks
because it gives you that whole mapping in your core files and relies
on overcommit which we don't like, hence my interest in a way to
reserve but not map.
>> The first problem with porting Postgres to pthreads is static variables
>> widely used in Postgres code.
>> Most of modern compilers support thread local variables, for example GCC
>> provides __thread keyword.
>> Such variables are placed in separate segment which is address through
>> segment register (at Intel).
>> So access time to such variables is the same as to normal static variables.
>
> I experimented similarly. Although I'm not 100% sure that if were to go
> for it, we wouldn't instead want to abstract our session concept
> further, or well, at all.
Using a ton of thread local variables may be a useful stepping stone,
but if we want to be able to separate threads/processes from sessions
eventually then I guess we'll want to model sessions as first class
objects and pass them around explicitly or using a single TLS variable
current_session.
> I think the biggest problem with doing this for real is that it's a huge
> project, and that it'll take a long time.
>
> Thanks for working on this!
+1
[1] http://db.cs.berkeley.edu/papers/ERL-M90-34.pdf
--
Thomas Munro
http://www.enterprisedb.com
^ permalink raw reply [nested|flat] 51+ messages in thread
* Re: Postgres with pthread
@ 2017-12-07 03:20 Craig Ringer <[email protected]>
parent: Andres Freund <[email protected]>
1 sibling, 2 replies; 51+ messages in thread
From: Craig Ringer @ 2017-12-07 03:20 UTC (permalink / raw)
To: Andres Freund <[email protected]>; +Cc: Tom Lane <[email protected]>; Konstantin Knizhnik <[email protected]>; pgsql-hackers
On 7 December 2017 at 01:17, Andres Freund <[email protected]> wrote:
>
> > I think you've done us a very substantial service by pursuing
> > this far enough to get some quantifiable performance results.
> > But now that we have some results in hand, I think we're best
> > off sticking with the architecture we've got.
>
> I don't agree.
>
> I'd personally expect that an immediate conversion would result in very
> little speedup, a bunch of code deleted, a bunch of complexity
> added. And it'd still be massively worthwhile, to keep medium to long
> term complexity and feature viability in control.
>
Personally I think it's a pity we didn't land up here before the
foundations for parallel query went in - DSM, shm_mq, DSA, etc. I know the
EDB folks at least looked into it though, and presumably there were good
reasons to go in this direction. Maybe that was just "community will never
accept threaded conversion" at the time, though.
Now we have quite a lot of homebrew infrastructure to consider if we do a
conversion.
That said, it might in some ways make it easier. shm_mq, for example, would
likely convert to a threaded backend with minimal changes to callers, and
probably only limited changes to shm_mq its self. So maybe these
abstractions will prove to have been a win in some ways. Except DSA, and
even then it could serve as a transitional API...
--
Craig Ringer http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
^ permalink raw reply [nested|flat] 51+ messages in thread
* Re: Postgres with pthread
@ 2017-12-07 03:26 Craig Ringer <[email protected]>
parent: Thomas Munro <[email protected]>
1 sibling, 1 reply; 51+ messages in thread
From: Craig Ringer @ 2017-12-07 03:26 UTC (permalink / raw)
To: Thomas Munro <[email protected]>; +Cc: Andres Freund <[email protected]>; Konstantin Knizhnik <[email protected]>; pgsql-hackers
On 7 December 2017 at 05:58, Thomas Munro <[email protected]>
wrote:
>
> Using a ton of thread local variables may be a useful stepping stone,
> but if we want to be able to separate threads/processes from sessions
> eventually then I guess we'll want to model sessions as first class
> objects and pass them around explicitly or using a single TLS variable
> current_session.
>
>
Yep.
This is the real reason I'm excited by the idea of a threading conversion.
PostgreSQL's architecture conflates "connection", "session" and "executor"
into one somewhat muddled mess. I'd love to be able to untangle that to the
point where we can pool executors amongst active queries, while retaining
idle sessions' state properly even while they're in a transaction.
Yeah, that's a long way off, but it'd be a whole lot more practical if we
didn't have to serialize and deserialize the entire session state to do it.
--
Craig Ringer http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
^ permalink raw reply [nested|flat] 51+ messages in thread
* RE: Postgres with pthread
@ 2017-12-07 03:44 Tsunakawa, Takayuki <[email protected]>
parent: Craig Ringer <[email protected]>
1 sibling, 1 reply; 51+ messages in thread
From: Tsunakawa, Takayuki @ 2017-12-07 03:44 UTC (permalink / raw)
To: 'Craig Ringer' <[email protected]>; Andres Freund <[email protected]>; +Cc: Tom Lane <[email protected]>; Konstantin Knizhnik <[email protected]>; pgsql-hackers
From: Craig Ringer [mailto:[email protected]]
> I'd personally expect that an immediate conversion would result
> in very
> little speedup, a bunch of code deleted, a bunch of complexity
> added. And it'd still be massively worthwhile, to keep medium to
> long
> term complexity and feature viability in control.
+1
I hope for things like:
* More performance statistics like system-wide LWLock waits, without the concern about fixed shared memory size
* Dynamic memory sizing, such as shared_buffers, work_mem, maintenance_work_mem
* Running multi-threaded components in postgres extension (is it really safe to run JVM for PL/Java in a single-threaded postgres?)
Regards
Takayuki Tsunakawa
^ permalink raw reply [nested|flat] 51+ messages in thread
* Re: Postgres with pthread
@ 2017-12-07 04:45 Craig Ringer <[email protected]>
parent: Tsunakawa, Takayuki <[email protected]>
0 siblings, 0 replies; 51+ messages in thread
From: Craig Ringer @ 2017-12-07 04:45 UTC (permalink / raw)
To: Tsunakawa, Takayuki <[email protected]>; +Cc: Andres Freund <[email protected]>; Tom Lane <[email protected]>; Konstantin Knizhnik <[email protected]>; pgsql-hackers
On 7 December 2017 at 11:44, Tsunakawa, Takayuki <
[email protected]> wrote:
> From: Craig Ringer [mailto:[email protected]]
> > I'd personally expect that an immediate conversion would result
> > in very
> > little speedup, a bunch of code deleted, a bunch of complexity
> > added. And it'd still be massively worthwhile, to keep medium to
> > long
> > term complexity and feature viability in control.
>
> +1
> I hope for things like:
>
> * More performance statistics like system-wide LWLock waits, without the
> concern about fixed shared memory size
>
* Dynamic memory sizing, such as shared_buffers, work_mem,
> maintenance_work_mem
>
I'm not sure how threaded operations would help us much there. If we could
split shared_buffers into extents we could do this with something like dsm
already. Without the ability to split it into extents, we can't do it with
locally malloc'd memory in a threaded system either.
Re performance diagnostics though, you can already get a lot of useful data
from PostgreSQL's SDT tracepoints, which are usable with perf and DTrace
amongst other tools. Dynamic userspace 'perf' probes can tell you a lot too.
I'm confident you could collect some seriously useful data with perf
tracepoints and 'perf script' these days. (BTW, I extended the
https://wiki.postgresql.org/wiki/Profiling_with_perf article a bit
yesterday with some tips on this).
Of course better built-in diagnostics would be nice. But I really don't see
how it'd have much to do with threaded vs forked model of execution; we can
allocate chunks of memory with dsm now, after all.
> * Running multi-threaded components in postgres extension (is it really
> safe to run JVM for PL/Java in a single-threaded postgres?)
>
PL/Java is a giant mess for so many more reasons than that. The JVM is a
heavyweight startup, lightweight thread model system. It doesn't play at
all well with postgres's lightweight process fork()-based CoW model. You
can't fork() the JVM because fork() doesn't play nice with threads, at all.
So you have to start it in each backend individually, which is just awful.
One of the nice things if Pg got a threaded model would be that you could
embed a JVM, Mono/.NET runtime, etc and have your sessions work together in
ways you cannot currently sensibly do. Folks using MS SQL, Oracle, etc are
pretty used to being able to do this, and while it should be done with
caution it can offer huge benefits for some complex workloads.
Right now if a PostgreSQL user wants to do anything involving IPC, shared
data, etc, we pretty much have to write quite complex C extensions to do it.
--
Craig Ringer http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
^ permalink raw reply [nested|flat] 51+ messages in thread
* Re: Postgres with pthread
@ 2017-12-07 07:41 Simon Riggs <[email protected]>
parent: Konstantin Knizhnik <[email protected]>
2 siblings, 2 replies; 51+ messages in thread
From: Simon Riggs @ 2017-12-07 07:41 UTC (permalink / raw)
To: Konstantin Knizhnik <[email protected]>; +Cc: pgsql-hackers
> But it is a theory. The main idea of this prototype was to prove or disprove
> this expectation at practice.
> But please notice that it is very raw prototype. A lot of stuff is not
> working yet.
> And supporting all of exited Postgres functionality requires
> much more efforts (and even more efforts are needed for optimizing Postgres
> for this architecture).
>
> I just want to receive some feedback and know if community is interested in
> any further work in this direction.
Looks good. You are right, it is a theory. If your prototype does
actually show what we think it does then it is a good and interesting
result.
I think we need careful analysis to show where these exact gains come
from. The actual benefit is likely not evenly distributed across the
list of possible benefits. Did they arise because you produced a
stripped down version of Postgres? Or did they arise from using
threads?
It would not be the first time a result shown in protoype did not show
real gains on a completed project.
I might also read your results to show that connection concentrators
would be a better area of work, since 100 connections perform better
than 1000 in both cases, so why bother optimising for 1000 connections
at all? In which case we should read the benefit at the 100
connections line, where it shows the lower 28% gain, closer to the
gain your colleague reported.
So I think we don't yet have enough to make a decision.
--
Simon Riggs http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
^ permalink raw reply [nested|flat] 51+ messages in thread
* Re: Postgres with pthread
@ 2017-12-07 11:55 Konstantin Knizhnik <[email protected]>
parent: Simon Riggs <[email protected]>
1 sibling, 2 replies; 51+ messages in thread
From: Konstantin Knizhnik @ 2017-12-07 11:55 UTC (permalink / raw)
To: [email protected]
I want to thank everybody for feedbacks and a lot of useful notices.
I am very pleased with interest of community to this topic and will
continue research in this direction.
Some more comments from my side:
My original intention was to implement some king of built-in connection
pooling for Postgres: be able to execute several transactions into one
backend.
It requires use of some kind lightweight multitasking (coroutines). The
obvious candidate for it is libcore.
In this case we also need to solve the problem with static variables.
And __thread will not help in this case. We have to collect all static
variables into some structure (context)
and replace any references to such variable with indirection through
pointer. It will be much harder to implement than annotating variable
definitions with __thread:
it will require change of all accesses to variables, so almost all
Postgres code has to be refactored.
Another problem with this approach is that we need asynchronous disk IO
for it. Unfortunately this is no good file AIO implementation for Linux.
Certainly we can spawn dedicated IO thread (or threads) and queue IO
requests to it. But such architecture seems to become quite complex.
Also cooperative multitasking itself is not able to load all CPU cores.
So we need to have several physical processes/threads which will execute
coroutines.
In theory such architecture should provide the best performance and
scalability (handle hundreds of thousands of client connections). But in
practice there are a lot of pitfals:
1. Right now each backend has its local relation, catalog and prepared
statement caches. For large database this caches can be large enough:
several megabytes.
So such coroutines becomes really not "lightweight". The obvious
solution is to have global caches or combine global and local caches.
But it once again requires significant
changes in postgres.
2. Large number of sessions makes current approach with procarray almost
unusable: we need to provide some alternative implementation of
snapshots, for example CSN based.
3. All locking mechanisms have to be rewritten.
So this approach almost exclude possibility of evolution of existed
postgres code base and requires "revolution": rewriting most of Postgres
components from scratch and refactoring almost all other postgres code.
This is why I have to abandon move in this direction.
Replacing processes with threads can be considered just as first step
and requires changes in many postgres components if we really want to
get significant advantages from it.
But at least such work can be splitted into several phases and it is
possible for some time to support both multithreaded and multiprocess
model in the same codebase.
Below I want to summarize the most important (from my point of view)
arguments pro/contra multithreaded I got from your feedbacks:
Pros:
1. Simplified memory model: no need in DSM, shm_mq, DSA, etc
2. Efficient integration of PLs supporting multithreaded execution,
first of all Java
3. Less memory footprint, faster context switching, more efficient use
of TLB
Contras:
1. Breaks compatibility with existed extensions and adds more
requirements for authors of new extension
2. Problems with integration of single-threaded PLs: Python, Lua,...
3. Worser protection from programming errors, included errors in extensions.
4. Lack of explicit separation of shared and privite memory leads to
more synchronization errors.
Right now in Postgres there is strict distinction between shared memory
and private memory, so it is clear for programmer
whether (s)he is working with shared data and so need some kind of
synchronization to avoid race condition.
In pthreads all memory is shared and more care is needed to work with it.
So pthreads can help to increase scalability, but still do not help much
in implementation of built-in connection pooling, autonomous
transactions,...
Current 50% improvement of select speed for large number of connections
certainly can not be considered as enough motivation for such radical
changes of Postgres architecture.
But it is just first step and much more benefits can be obtained by
adopting Postgres to this model.
It is hard to me to estimate now all complexity of switching to thread
model and all advantages we can get from it.
First of all I am going to repeat my benchmarks at SMP computers with
large number of cores (so that 100 or more active backends can be really
useful even in case of connection pooling).
--
Konstantin Knizhnik
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company
^ permalink raw reply [nested|flat] 51+ messages in thread
* Re: Postgres with pthread
@ 2017-12-07 12:06 Konstantin Knizhnik <[email protected]>
parent: Andres Freund <[email protected]>
3 siblings, 0 replies; 51+ messages in thread
From: Konstantin Knizhnik @ 2017-12-07 12:06 UTC (permalink / raw)
To: Andres Freund <[email protected]>; +Cc: pgsql-hackers
Hi
On 06.12.2017 20:08, Andres Freund wrote:
>
> 4. Rewrite file descriptor cache to be global (shared by all threads).
> That one I'm very unconvinced of, that's going to add a ton of new
> contention.
Do you mean lock contention because of mutex I used to synchronize
access to shared file descriptor cache
or contention for file descriptors?
Right now each thread has its own virtual file descriptors, so them are
not shared between threads.
But there is common LRU, restricting total number of opened descriptors
in the process.
Actually I have not other choice if I want to support thousands of
connection.
If each thread has its own private descriptor cache (as it is now for
processes) and its size is estimated base on open file quota,
then there will be millions of opened file descriptors.
Concerning contention for mutex, I do not think that it is a problem.
At least I have to say that performance (with 100 connections) is
significantly improved and shows almost the same speed as for 10
connections
after I have rewritten file descriptor can and made it global
(my original implementation just made all fd.c static variables as
thread local, so each thread has its separate pool).
It is possible to go further and shared file descriptors between threads
and use pwrite/pread instead of seek+read/write.
But we still need mutex to implement LRU l2list and free handler list.
^ permalink raw reply [nested|flat] 51+ messages in thread
* Re: Postgres with pthread
@ 2017-12-07 12:13 Konstantin Knizhnik <[email protected]>
parent: Thomas Munro <[email protected]>
1 sibling, 0 replies; 51+ messages in thread
From: Konstantin Knizhnik @ 2017-12-07 12:13 UTC (permalink / raw)
To: Thomas Munro <[email protected]>; Andres Freund <[email protected]>; +Cc: pgsql-hackers
On 07.12.2017 00:58, Thomas Munro wrote:
> Using a ton of thread local variables may be a useful stepping stone,
> but if we want to be able to separate threads/processes from sessions
> eventually then I guess we'll want to model sessions as first class
> objects and pass them around explicitly or using a single TLS variable
> current_session.
>
It was my primary intention.
Unfortunately separating all static variables into some kind of session
context requires much more efforts:
we have to change all accesses to such variables.
But please notice, that from performance point of view access to
__thread variables is not more expensive then access to static variable or
access to fields of session context structure through current_session.
And there is no extra space overhead for them.
--
Konstantin Knizhnik
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company
^ permalink raw reply [nested|flat] 51+ messages in thread
* Re: Postgres with pthread
@ 2017-12-07 14:56 Craig Ringer <[email protected]>
parent: Konstantin Knizhnik <[email protected]>
1 sibling, 0 replies; 51+ messages in thread
From: Craig Ringer @ 2017-12-07 14:56 UTC (permalink / raw)
To: Konstantin Knizhnik <[email protected]>; +Cc: [email protected]
On 7 December 2017 at 19:55, Konstantin Knizhnik <[email protected]>
wrote:
>
> Pros:
> 1. Simplified memory model: no need in DSM, shm_mq, DSA, etc
>
shm_mq would remain useful, and the others could only be dropped if you
also dropped process-model support entirely.
> 1. Breaks compatibility with existed extensions and adds more requirements
> for authors of new extension
>
Depends on how much frightening preprocessor magic you're willing to use,
doesn't it? ;)
Wouldn't be surprised if simple extensions (C functions etc) stayed fairly
happy, but it'd be hazardous enough in terms of library use etc that
deliberate breakage may be beter.
> 2. Problems with integration of single-threaded PLs: Python, Lua,...
>
Yeah, that's going to hurt. Especially since most non-plpgsql code out
there will be plperl and plpython. Breaking that's not going to be an
option, but nobody's going to be happy if all postgres backends must
contend for the same Python GIL. Plus it'd be deadlock-city.
That's nearly a showstopper right there. Especially since with a quick look
around it looks like the cPython GIL is per-DLL (at least on Windows) not
per-interpreter-state, so spawning separate interpreter states per-thread
may not be sufficient. That makes sense given that cPython its self is
thread-aware; otherwise it'd have a really hard time figuring out which GIL
and interpreter state to look at when in a cPython-spawned thread.
> 3. Worser protection from programming errors, included errors in
> extensions.
>
Mainly contaminating memory of unrelated procesess, or the postmaster.
I'm not worried about outright crashes. On any modern system it's not
significantly worse to take down the postmaster than it is to have it do
its own recovery. A modern init will restart it promptly. (If you're not
running postgres under an init daemon for production then... well, you
should be.)
> 4. Lack of explicit separation of shared and privite memory leads to more
> synchronization errors.
>
Accidentally clobbering postmaster memory/state would be my main worry
there.
Right now we gain a lot of protection from our copy-on-write
shared-nothing-by-default model, and we rely on it in quite a lot of places
where backends merrily stomp on inherited postmaster state.
The more I think about it, the less enthusiastic I am, really.
--
Craig Ringer http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
^ permalink raw reply [nested|flat] 51+ messages in thread
* Re: Postgres with pthread
@ 2017-12-07 17:52 Robert Haas <[email protected]>
parent: Craig Ringer <[email protected]>
1 sibling, 0 replies; 51+ messages in thread
From: Robert Haas @ 2017-12-07 17:52 UTC (permalink / raw)
To: Craig Ringer <[email protected]>; +Cc: Andres Freund <[email protected]>; Tom Lane <[email protected]>; Konstantin Knizhnik <[email protected]>; pgsql-hackers
On Wed, Dec 6, 2017 at 10:20 PM, Craig Ringer <[email protected]> wrote:
> Personally I think it's a pity we didn't land up here before the foundations
> for parallel query went in - DSM, shm_mq, DSA, etc. I know the EDB folks at
> least looked into it though, and presumably there were good reasons to go in
> this direction. Maybe that was just "community will never accept threaded
> conversion" at the time, though.
Yep. Never is a long time, but it took 3 release cycles to get a
user-visible feature as it was, and if I'd tried to insist on a
process->thread conversion first I suspect we'd still be stuck on that
point today. Perhaps we would have gotten as far as getting that much
done, but that wouldn't make parallel query be done on top of it.
> Now we have quite a lot of homebrew infrastructure to consider if we do a
> conversion.
>
> That said, it might in some ways make it easier. shm_mq, for example, would
> likely convert to a threaded backend with minimal changes to callers, and
> probably only limited changes to shm_mq its self. So maybe these
> abstractions will prove to have been a win in some ways. Except DSA, and
> even then it could serve as a transitional API...
Yeah, I don't feel too bad about what we've built. Even if it
ultimately goes away, it will have served the useful purpose of
proving that parallel query is a good idea and can work. Besides,
shm_mq is just a ring buffer for messages; that's not automatically
something that we don't want just because we move to threads. If it
goes away, which I think not unlikely, it'll be because something else
is faster.
Also, it's not as if only parallel query structures might have been
designed differently if we had been using threads all along.
dynahash, for example, is quite unlike most concurrent hash tables and
a big part of the reason is that it has to cope with being situated in
a fixed-size chunk of shared memory. More generally, the whole reason
there's no cheap, straightforward palloc_shared() is the result of the
current design, and it seems very unlikely we wouldn't have that quite
apart from parallel query. Install pg_stat_statements without a
server restart? Yes, please.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
^ permalink raw reply [nested|flat] 51+ messages in thread
* Re: Postgres with pthread
@ 2017-12-07 19:58 Andres Freund <[email protected]>
parent: Craig Ringer <[email protected]>
0 siblings, 2 replies; 51+ messages in thread
From: Andres Freund @ 2017-12-07 19:58 UTC (permalink / raw)
To: Craig Ringer <[email protected]>; +Cc: Thomas Munro <[email protected]>; Konstantin Knizhnik <[email protected]>; pgsql-hackers
On 2017-12-07 11:26:07 +0800, Craig Ringer wrote:
> PostgreSQL's architecture conflates "connection", "session" and "executor"
> into one somewhat muddled mess.
How is the executor entangled in the other two?
Greetings,
Andres Freund
^ permalink raw reply [nested|flat] 51+ messages in thread
* Re: Postgres with pthread
@ 2017-12-07 20:48 Greg Stark <[email protected]>
parent: Andres Freund <[email protected]>
1 sibling, 1 reply; 51+ messages in thread
From: Greg Stark @ 2017-12-07 20:48 UTC (permalink / raw)
To: Andres Freund <[email protected]>; +Cc: Craig Ringer <[email protected]>; Thomas Munro <[email protected]>; Konstantin Knizhnik <[email protected]>; pgsql-hackers
On 7 December 2017 at 19:58, Andres Freund <[email protected]> wrote:
> On 2017-12-07 11:26:07 +0800, Craig Ringer wrote:
>> PostgreSQL's architecture conflates "connection", "session" and "executor"
>> into one somewhat muddled mess.
>
> How is the executor entangled in the other two?
I was going to ask the same question. AFAICS it's the one part of
Postgres that isn't muddled at all -- it's crystal clear that
"connection" == "session" as far as the backend is concerned and
"executor context" is completely separate.
But then I thought about it a bit and I do wonder. I don't know how
well we test having multiple portals doing all kinds of different
query plans with their execution interleaved. And I definitely have
doubts whether you can start SPI sessions from arbitrary points in the
executor expression evaluation and don't know what state you can leave
and resume them from on subsequent evaluations...
--
greg
^ permalink raw reply [nested|flat] 51+ messages in thread
* Re: Postgres with pthread
@ 2017-12-07 20:52 Andres Freund <[email protected]>
parent: Greg Stark <[email protected]>
0 siblings, 0 replies; 51+ messages in thread
From: Andres Freund @ 2017-12-07 20:52 UTC (permalink / raw)
To: Greg Stark <[email protected]>; +Cc: Craig Ringer <[email protected]>; Thomas Munro <[email protected]>; Konstantin Knizhnik <[email protected]>; pgsql-hackers
Hi,
On 2017-12-07 20:48:06 +0000, Greg Stark wrote:
> But then I thought about it a bit and I do wonder. I don't know how
> well we test having multiple portals doing all kinds of different
> query plans with their execution interleaved.
Cursors test that pretty well.
> And I definitely have doubts whether you can start SPI sessions from
> arbitrary points in the executor expression evaluation and don't know
> what state you can leave and resume them from on subsequent
> evaluations...
SPI being weird doesn't really have that much bearing on the executor
structure imo. But I'm unclear what you'd use SPI for that really
necessitates that. We don't suspend execution it the middle of function
execution...
Greetings,
Andres Freund
^ permalink raw reply [nested|flat] 51+ messages in thread
* Re: Postgres with pthread
@ 2017-12-08 01:14 Craig Ringer <[email protected]>
parent: Andres Freund <[email protected]>
1 sibling, 0 replies; 51+ messages in thread
From: Craig Ringer @ 2017-12-08 01:14 UTC (permalink / raw)
To: Andres Freund <[email protected]>; +Cc: Thomas Munro <[email protected]>; Konstantin Knizhnik <[email protected]>; pgsql-hackers
On 8 December 2017 at 03:58, Andres Freund <[email protected]> wrote:
> On 2017-12-07 11:26:07 +0800, Craig Ringer wrote:
> > PostgreSQL's architecture conflates "connection", "session" and
> "executor"
> > into one somewhat muddled mess.
>
> How is the executor entangled in the other two?
>
>
Executor in the postgres sense isn't, so I chose the word poorly.
"Engine of execution" maybe. What I'm getting at is that we tie up more
resources than should ideally be necessary when a session is idle,
especially idle in transaction. But I guess a lot of that is really down to
memory allocated and not returned to the OS (because like other C programs
we can't do that), etc. The key resources like PGXACT entries aren't
something we can release while idle in a transaction after all.
--
Craig Ringer http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
^ permalink raw reply [nested|flat] 51+ messages in thread
* Re: Postgres with pthread
@ 2017-12-08 22:09 konstantin knizhnik <[email protected]>
parent: Simon Riggs <[email protected]>
1 sibling, 1 reply; 51+ messages in thread
From: konstantin knizhnik @ 2017-12-08 22:09 UTC (permalink / raw)
To: Simon Riggs <[email protected]>; +Cc: pgsql-hackers
On Dec 7, 2017, at 10:41 AM, Simon Riggs wrote:
>> But it is a theory. The main idea of this prototype was to prove or disprove
>> this expectation at practice.
>
>> But please notice that it is very raw prototype. A lot of stuff is not
>> working yet.
>
>> And supporting all of exited Postgres functionality requires
>> much more efforts (and even more efforts are needed for optimizing Postgres
>> for this architecture).
>>
>> I just want to receive some feedback and know if community is interested in
>> any further work in this direction.
>
> Looks good. You are right, it is a theory. If your prototype does
> actually show what we think it does then it is a good and interesting
> result.
>
> I think we need careful analysis to show where these exact gains come
> from. The actual benefit is likely not evenly distributed across the
> list of possible benefits. Did they arise because you produced a
> stripped down version of Postgres? Or did they arise from using
> threads?
>
> It would not be the first time a result shown in protoype did not show
> real gains on a completed project.
>
> I might also read your results to show that connection concentrators
> would be a better area of work, since 100 connections perform better
> than 1000 in both cases, so why bother optimising for 1000 connections
> at all? In which case we should read the benefit at the 100
> connections line, where it shows the lower 28% gain, closer to the
> gain your colleague reported.
>
> So I think we don't yet have enough to make a decision.
Concerning optimal number of connection: one of my intentions was to eliminate meed in external connection pool (pgbouncer&Co).
In this case applications can use prepared statements which itself provides two times increase of performance.
I believe that threads have smaller footprint than processes, to it is possible to spawn more threads and directly access them without intermediate layer with connection pooling.
I have performed experiments at more power server:
144 virtual cores Intel(R) Xeon(R) CPU E7-8890 v3 @ 2.50GHz.
Here results of read-only queries are different: both pthreads and vanilla version shows almost the same speed both for 100 and 1000 connections: about 1300k TPS
with prepared statement. So there is no performance degradation with increased number of connections and no larger difference between processes and threads.
But at read-write workload (pgbench -N) there is still significant advantage of pthreads version (kTPS):
Connections
Vanilla
pthreads
100
165
154
1000
85
118
For some reasons (which I do not know yet) multiprocess version of postgres is slightly faster for 100 connections,
but degrades almost twice for 1000 connections, while degradation of multithreads version is not so large.
By the way, pthreads version make it possible to much easily check whats going on using gdb (manual "profiling") :
thread apply all bt
Thread 997 (Thread 0x7f6e08810700 (LWP 61345)):
#0 0x00007f7e03263576 in do_futex_wait.constprop () from /lib64/libpthread.so.0
#1 0x00007f7e03263668 in __new_sem_wait_slow.constprop.0 () from /lib64/libpthread.so.0
#2 0x0000000000698552 in PGSemaphoreLock ()
#3 0x0000000000702804 in LWLockAcquire ()
#4 0x00000000004f9ac4 in XLogInsertRecord ()
#5 0x0000000000503b97 in XLogInsert ()
#6 0x00000000004bb0d1 in log_heap_clean ()
#7 0x00000000004bd7c8 in heap_page_prune ()
#8 0x00000000004bd9c1 in heap_page_prune_opt ()
---Type <return> to continue, or q <return> to quit---
#9 0x00000000004c43d4 in index_fetch_heap ()
#10 0x00000000004c4410 in index_getnext ()
#11 0x00000000006037d2 in IndexNext ()
#12 0x00000000005f3a80 in ExecScan ()
#13 0x0000000000609eba in ExecModifyTable ()
#14 0x00000000005ed6fa in standard_ExecutorRun ()
#15 0x0000000000713622 in ProcessQuery ()
#16 0x0000000000713885 in PortalRunMulti ()
#17 0x00000000007143a5 in PortalRun ()
#18 0x0000000000711cf1 in PostgresMain ()
#19 0x00000000006a708b in backend_main_proc ()
#20 0x00007f7e0325a36d in start_thread () from /lib64/libpthread.so.0
#21 0x00007f7e02870b8f in clone () from /lib64/libc.so.6
Thread 996 (Thread 0x7f6e08891700 (LWP 61344)):
#0 0x00007f7e03263576 in do_futex_wait.constprop () from /lib64/libpthread.so.0
#1 0x00007f7e03263668 in __new_sem_wait_slow.constprop.0 () from /lib64/libpthread.so.0
#2 0x0000000000698552 in PGSemaphoreLock ()
#3 0x0000000000702804 in LWLockAcquire ()
#4 0x00000000004bc862 in RelationGetBufferForTuple ()
#5 0x00000000004b60db in heap_insert ()
#6 0x000000000060ad3b in ExecModifyTable ()
#7 0x00000000005ed6fa in standard_ExecutorRun ()
#8 0x0000000000713622 in ProcessQuery ()
#9 0x0000000000713885 in PortalRunMulti ()
#10 0x00000000007143a5 in PortalRun ()
#11 0x0000000000711cf1 in PostgresMain ()
#12 0x00000000006a708b in backend_main_proc ()
#13 0x00007f7e0325a36d in start_thread () from /lib64/libpthread.so.0
#14 0x00007f7e02870b8f in clone () from /lib64/libc.so.6
Thread 995 (Thread 0x7f6e08912700 (LWP 61343)):
#0 0x00007f7e03263576 in do_futex_wait.constprop () from /lib64/libpthread.so.0
#1 0x00007f7e03263668 in __new_sem_wait_slow.constprop.0 () from /lib64/libpthread.so.0
#2 0x0000000000698552 in PGSemaphoreLock ()
#3 0x00000000006f1dad in ProcArrayEndTransaction ()
#4 0x00000000004efae0 in CommitTransaction ()
#5 0x00000000004f0bd5 in CommitTransactionCommand ()
#6 0x000000000070e9cf in finish_xact_command ()
#7 0x0000000000711d13 in PostgresMain ()
#8 0x00000000006a708b in backend_main_proc ()
#9 0x00007f7e0325a36d in start_thread () from /lib64/libpthread.so.0
#10 0x00007f7e02870b8f in clone () from /lib64/libc.so.6
---Type <return> to continue, or q <return> to quit---
Thread 994 (Thread 0x7f6e08993700 (LWP 61342)):
#0 0x00007f7e03263576 in do_futex_wait.constprop () from /lib64/libpthread.so.0
#1 0x00007f7e03263668 in __new_sem_wait_slow.constprop.0 () from /lib64/libpthread.so.0
#2 0x0000000000698552 in PGSemaphoreLock ()
#3 0x00000000006f1dad in ProcArrayEndTransaction ()
#4 0x00000000004efae0 in CommitTransaction ()
#5 0x00000000004f0bd5 in CommitTransactionCommand ()
#6 0x000000000070e9cf in finish_xact_command ()
#7 0x0000000000711d13 in PostgresMain ()
#8 0x00000000006a708b in backend_main_proc ()
#9 0x00007f7e0325a36d in start_thread () from /lib64/libpthread.so.0
#10 0x00007f7e02870b8f in clone () from /lib64/libc.so.6
Thread 993 (Thread 0x7f6e08a14700 (LWP 61341)):
#0 0x00007f7e03263576 in do_futex_wait.constprop () from /lib64/libpthread.so.0
#1 0x00007f7e03263668 in __new_sem_wait_slow.constprop.0 () from /lib64/libpthread.so.0
#2 0x0000000000698552 in PGSemaphoreLock ()
#3 0x00000000006f1dad in ProcArrayEndTransaction ()
#4 0x00000000004efae0 in CommitTransaction ()
#5 0x00000000004f0bd5 in CommitTransactionCommand ()
#6 0x000000000070e9cf in finish_xact_command ()
#7 0x0000000000711d13 in PostgresMain ()
#8 0x00000000006a708b in backend_main_proc ()
#9 0x00007f7e0325a36d in start_thread () from /lib64/libpthread.so.0
#10 0x00007f7e02870b8f in clone () from /lib64/libc.so.6
Thread 992 (Thread 0x7f6e08a95700 (LWP 61340)):
#0 0x00007f7e03263576 in do_futex_wait.constprop () from /lib64/libpthread.so.0
#1 0x00007f7e03263668 in __new_sem_wait_slow.constprop.0 () from /lib64/libpthread.so.0
#2 0x0000000000698552 in PGSemaphoreLock ()
#3 0x00000000006f1dad in ProcArrayEndTransaction ()
#4 0x00000000004efae0 in CommitTransaction ()
#5 0x00000000004f0bd5 in CommitTransactionCommand ()
#6 0x000000000070e9cf in finish_xact_command ()
#7 0x0000000000711d13 in PostgresMain ()
#8 0x00000000006a708b in backend_main_proc ()
#9 0x00007f7e0325a36d in start_thread () from /lib64/libpthread.so.0
#10 0x00007f7e02870b8f in clone () from /lib64/libc.so.6
Thread 991 (Thread 0x7f6e08b16700 (LWP 61339)):
#0 0x00007f7e03263576 in do_futex_wait.constprop () from /lib64/libpthread.so.0
#1 0x00007f7e03263668 in __new_sem_wait_slow.constprop.0 () from /lib64/libpthread.so.0
#2 0x0000000000698552 in PGSemaphoreLock ()
#3 0x00000000006f1dad in ProcArrayEndTransaction ()
---Type <return> to continue, or q <return> to quit---
#4 0x00000000004efae0 in CommitTransaction ()
#5 0x00000000004f0bd5 in CommitTransactionCommand ()
#6 0x000000000070e9cf in finish_xact_command ()
#7 0x0000000000711d13 in PostgresMain ()
#8 0x00000000006a708b in backend_main_proc ()
#9 0x00007f7e0325a36d in start_thread () from /lib64/libpthread.so.0
#10 0x00007f7e02870b8f in clone () from /lib64/libc.so.6
....
I am not going to show stack traces of all 1000 threads.
But you may notice that proc array lock really seems be be a bottleneck.
^ permalink raw reply [nested|flat] 51+ messages in thread
* Re: Postgres with pthread
@ 2017-12-08 22:28 Alexander Korotkov <[email protected]>
parent: konstantin knizhnik <[email protected]>
0 siblings, 0 replies; 51+ messages in thread
From: Alexander Korotkov @ 2017-12-08 22:28 UTC (permalink / raw)
To: konstantin knizhnik <[email protected]>; +Cc: Simon Riggs <[email protected]>; pgsql-hackers
On Sat, Dec 9, 2017 at 1:09 AM, konstantin knizhnik <
[email protected]> wrote:
> I am not going to show stack traces of all 1000 threads.
> But you may notice that proc array lock really seems be be a bottleneck.
>
Yes, proc array lock easily becomes a bottleneck on multicore machine with
large number of connections. Related to this, another patch helping to
large number of connections is CSN. When our snapshot model was invented,
xip was just array of few elements, and that cause no problem. Now, we're
considering threads to help us handling thousands of connections. Snapshot
with thousands of xips looks ridiculous. Collecting such a large snapshot
could be more expensive than single index lookup.
These two patches threads and CSN are both complicated and require hard
work during multiple release cycles to get committed. But I really hope
that their cumulative effect can dramatically improve situation on high
number of connections. There are already some promising benchmarks in CSN
thread. I wonder if we already can do some cumulative benchmarks of
threads + CSN?
------
Alexander Korotkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company
^ permalink raw reply [nested|flat] 51+ messages in thread
* Re: Postgres with pthread
@ 2017-12-10 16:24 james <[email protected]>
parent: Andreas Karlsson <[email protected]>
0 siblings, 0 replies; 51+ messages in thread
From: james @ 2017-12-10 16:24 UTC (permalink / raw)
To: Andreas Karlsson <[email protected]>; Andres Freund <[email protected]>; Konstantin Knizhnik <[email protected]>; +Cc: pgsql-hackers
On 06/12/2017 17:26, Andreas Karlsson wrote:
> An additional issue is that this could break a lot of extensions and
> in a way that it is not apparent at compile time. This means we may
> need to break all extensions to force extensions authors to check if
> they are thread safe.
>
> I do not like making life hard for out extension community, but if the
> gains are big enough it might be worth it.
It seems to me that the counter-argument is that extensions that
naturally support threading will benefit. For example it may be a lot
more practical to have CLR or JVM extensions.
^ permalink raw reply [nested|flat] 51+ messages in thread
* Re: Postgres with pthread
@ 2017-12-21 13:25 Konstantin Knizhnik <[email protected]>
parent: Konstantin Knizhnik <[email protected]>
1 sibling, 2 replies; 51+ messages in thread
From: Konstantin Knizhnik @ 2017-12-21 13:25 UTC (permalink / raw)
To: [email protected]
I continue experiments with my pthread prototype.
Latest results are the following:
1. I have eliminated all (I hope) calls of non-reentrant functions
(getopt, setlocale, setitimer, localtime, ...). So now parallel tests
are passed.
2. I have implemented deallocation of top memory context (at thread
exit) and cleanup of all opened file descriptors.
I have to replace several place where malloc is used with top_malloc:
allocation in top context.
3. Now my prototype is passing all regression tests now. But handling of
errors is still far from completion.
4. I have performed experiments with replacing synchronization
primitives used in Postgres with pthread analogues.
Unfortunately it has almost now influence on performance.
5. Handling large number of connections.
The maximal number of postgres connections is almost the same: 100k.
But memory footprint in case of pthreads was significantly smaller: 18Gb
vs 38Gb.
And difference in performance was much higher: 60k TPS vs . 600k TPS.
Compare it with performance for 10k clients: 1300k TPS.
It is read-only pgbench -S test with 1000 connections.
As far as pgbench doesn't allow to specify more than 1000 clients, I
spawned several instances of pgbench.
Why handling large number of connections is important?
It allows applications to access postgres directly, not using pgbouncer
or any other external connection pooling tool.
In this case an application can use prepared statements which can reduce
speed of simple queries almost twice.
Unfortunately Postgres sessions are not lightweight. Each backend
maintains its private catalog and relation caches, prepared statement
cache,...
For real database size of this caches in memory will be several
megabytes and warming this caches can take significant amount of time.
So if we really want to support large number of connections, we should
rewrite caches to be global (shared).
It will allow to save a lot of memory but add synchronization overhead.
Also at NUMA private caches may be more efficient than one global cache.
My proptotype can be found at:
git://github.com/postgrespro/postgresql.pthreads.git
--
Konstantin Knizhnik
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company
^ permalink raw reply [nested|flat] 51+ messages in thread
* Re: Postgres with pthread
@ 2017-12-21 13:46 Pavel Stehule <[email protected]>
parent: Konstantin Knizhnik <[email protected]>
1 sibling, 0 replies; 51+ messages in thread
From: Pavel Stehule @ 2017-12-21 13:46 UTC (permalink / raw)
To: Konstantin Knizhnik <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]>
2017-12-21 14:25 GMT+01:00 Konstantin Knizhnik <[email protected]>:
> I continue experiments with my pthread prototype.
> Latest results are the following:
>
> 1. I have eliminated all (I hope) calls of non-reentrant functions
> (getopt, setlocale, setitimer, localtime, ...). So now parallel tests are
> passed.
>
> 2. I have implemented deallocation of top memory context (at thread exit)
> and cleanup of all opened file descriptors.
> I have to replace several place where malloc is used with top_malloc:
> allocation in top context.
>
> 3. Now my prototype is passing all regression tests now. But handling of
> errors is still far from completion.
>
> 4. I have performed experiments with replacing synchronization primitives
> used in Postgres with pthread analogues.
> Unfortunately it has almost now influence on performance.
>
> 5. Handling large number of connections.
> The maximal number of postgres connections is almost the same: 100k.
> But memory footprint in case of pthreads was significantly smaller: 18Gb
> vs 38Gb.
> And difference in performance was much higher: 60k TPS vs . 600k TPS.
> Compare it with performance for 10k clients: 1300k TPS.
> It is read-only pgbench -S test with 1000 connections.
> As far as pgbench doesn't allow to specify more than 1000 clients, I
> spawned several instances of pgbench.
>
> Why handling large number of connections is important?
> It allows applications to access postgres directly, not using pgbouncer or
> any other external connection pooling tool.
> In this case an application can use prepared statements which can reduce
> speed of simple queries almost twice.
>
What I know MySQL has not good experience with high number of threads - and
there is thread pool in enterprise (and now in Mariadb0 versions.
Regards
Pavel
> Unfortunately Postgres sessions are not lightweight. Each backend
> maintains its private catalog and relation caches, prepared statement
> cache,...
> For real database size of this caches in memory will be several megabytes
> and warming this caches can take significant amount of time.
> So if we really want to support large number of connections, we should
> rewrite caches to be global (shared).
> It will allow to save a lot of memory but add synchronization overhead.
> Also at NUMA private caches may be more efficient than one global cache.
>
> My proptotype can be found at: git://github.com/postgrespro/p
> ostgresql.pthreads.git
>
>
> --
>
> Konstantin Knizhnik
> Postgres Professional: http://www.postgrespro.com
> The Russian Postgres Company
>
>
>
^ permalink raw reply [nested|flat] 51+ messages in thread
* Re: Postgres with pthread
@ 2017-12-27 08:34 Konstantin Knizhnik <[email protected]>
parent: Konstantin Knizhnik <[email protected]>
1 sibling, 1 reply; 51+ messages in thread
From: Konstantin Knizhnik @ 2017-12-27 08:34 UTC (permalink / raw)
To: [email protected]
On 21.12.2017 16:25, Konstantin Knizhnik wrote:
> I continue experiments with my pthread prototype.
> Latest results are the following:
>
> 1. I have eliminated all (I hope) calls of non-reentrant functions
> (getopt, setlocale, setitimer, localtime, ...). So now parallel tests
> are passed.
>
> 2. I have implemented deallocation of top memory context (at thread
> exit) and cleanup of all opened file descriptors.
> I have to replace several place where malloc is used with top_malloc:
> allocation in top context.
>
> 3. Now my prototype is passing all regression tests now. But handling
> of errors is still far from completion.
>
> 4. I have performed experiments with replacing synchronization
> primitives used in Postgres with pthread analogues.
> Unfortunately it has almost now influence on performance.
>
> 5. Handling large number of connections.
> The maximal number of postgres connections is almost the same: 100k.
> But memory footprint in case of pthreads was significantly smaller:
> 18Gb vs 38Gb.
> And difference in performance was much higher: 60k TPS vs . 600k TPS.
> Compare it with performance for 10k clients: 1300k TPS.
> It is read-only pgbench -S test with 1000 connections.
> As far as pgbench doesn't allow to specify more than 1000 clients, I
> spawned several instances of pgbench.
>
> Why handling large number of connections is important?
> It allows applications to access postgres directly, not using
> pgbouncer or any other external connection pooling tool.
> In this case an application can use prepared statements which can
> reduce speed of simple queries almost twice.
>
> Unfortunately Postgres sessions are not lightweight. Each backend
> maintains its private catalog and relation caches, prepared statement
> cache,...
> For real database size of this caches in memory will be several
> megabytes and warming this caches can take significant amount of time.
> So if we really want to support large number of connections, we should
> rewrite caches to be global (shared).
> It will allow to save a lot of memory but add synchronization
> overhead. Also at NUMA private caches may be more efficient than one
> global cache.
>
> My proptotype can be found at:
> git://github.com/postgrespro/postgresql.pthreads.git
>
>
Finally I managed to run Postgres with 100k active connections.
Not sure that this result can pretend for been mentioned in Guiness
records, but I am almost sure that nobody has done it before (at least
with original version of Postgres).
But it was really "Pyrrhic victory". Performance for 100k connections is
1000 times slower than for 10k. All threads are blocked in semaphores.
This is more or less expected result, but still scale of degradation is
impressive:
#Connections
TPS
100k
550
10k
558k
6k
745k
4k
882k
2k
1100k
1k
1300k
As it is clear from this stacktraces, shared catalog and statement cache
are highly needed to provide good performance with such large number of
active backends:
(gdb) thread apply all bt
Thread 17807 (LWP 660863):
#0 0x00007f4c1cb46576 in do_futex_wait.constprop () from
/lib64/libpthread.so.0
#1 0x00007f4c1cb46668 in __new_sem_wait_slow.constprop.0 () from
/lib64/libpthread.so.0
#2 0x0000000000697a32 in PGSemaphoreLock ()
#3 0x0000000000702a64 in LWLockAcquire ()
#4 0x00000000006fbf2d in LockAcquireExtended ()
#5 0x00000000006f9fa3 in LockRelationOid ()
#6 0x00000000004b2ffd in relation_open ()
#7 0x00000000004b31d6 in heap_open ()
#8 0x00000000007f1ed1 in CatalogCacheInitializeCache ()
#9 0x00000000007f3835 in SearchCatCache1 ()
#10 0x0000000000800510 in get_tablespace ()
#11 0x00000000008006e1 in get_tablespace_page_costs ()
#12 0x000000000065a4e1 in cost_seqscan ()
#13 0x000000000068bf92 in create_seqscan_path ()
#14 0x00000000006568b4 in set_rel_pathlist ()
#15 0x0000000000656eb8 in make_one_rel ()
#16 0x00000000006740d0 in query_planner ()
#17 0x0000000000676526 in grouping_planner ()
#18 0x0000000000679812 in subquery_planner ()
#19 0x000000000067a66c in standard_planner ()
#20 0x000000000070ffe1 in pg_plan_query ()
#21 0x00000000007100b6 in pg_plan_queries ()
#22 0x00000000007f6c6f in BuildCachedPlan ()
#23 0x00000000007f6e5c in GetCachedPlan ()
#24 0x0000000000711ccf in PostgresMain ()
#25 0x00000000006a5535 in backend_main_proc ()
#26 0x00000000006a353d in thread_trampoline ()
#27 0x00007f4c1cb3d36d in start_thread () from /lib64/libpthread.so.0
#28 0x00007f4c1c153b8f in clone () from /lib64/libc.so.6
Thread 17806 (LWP 660861):
#0 0x00007f4c1cb46576 in do_futex_wait.constprop () from
/lib64/libpthread.so.0
#1 0x00007f4c1cb46668 in __new_sem_wait_slow.constprop.0 () from
/lib64/libpthread.so.0
#2 0x0000000000697a32 in PGSemaphoreLock ()
#3 0x0000000000702a64 in LWLockAcquire ()
#4 0x00000000006fbf2d in LockAcquireExtended ()
#5 0x00000000006f9fa3 in LockRelationOid ()
#6 0x00000000004b2ffd in relation_open ()
#7 0x00000000004b31d6 in heap_open ()
#8 0x00000000007f1ed1 in CatalogCacheInitializeCache ()
#9 0x00000000007f3835 in SearchCatCache1 ()
#10 0x0000000000800510 in get_tablespace ()
#11 0x00000000008006e1 in get_tablespace_page_costs ()
#12 0x000000000065a4e1 in cost_seqscan ()
#13 0x000000000068bf92 in create_seqscan_path ()
#14 0x00000000006568b4 in set_rel_pathlist ()
#15 0x0000000000656eb8 in make_one_rel ()
#16 0x00000000006740d0 in query_planner ()
#17 0x0000000000676526 in grouping_planner ()
#18 0x0000000000679812 in subquery_planner ()
#19 0x000000000067a66c in standard_planner ()
#20 0x000000000070ffe1 in pg_plan_query ()
#21 0x00000000007100b6 in pg_plan_queries ()
#22 0x00000000007f6c6f in BuildCachedPlan ()
---Type <return> to continue, or q <return> to quit---
#23 0x00000000007f6e5c in GetCachedPlan ()
#24 0x0000000000711ccf in PostgresMain ()
#25 0x00000000006a5535 in backend_main_proc ()
#26 0x00000000006a353d in thread_trampoline ()
#27 0x00007f4c1cb3d36d in start_thread () from /lib64/libpthread.so.0
#28 0x00007f4c1c153b8f in clone () from /lib64/libc.so.6
Thread 17805 (LWP 660856):
#0 0x00007f4c1cb46576 in do_futex_wait.constprop () from
/lib64/libpthread.so.0
#1 0x00007f4c1cb46668 in __new_sem_wait_slow.constprop.0 () from
/lib64/libpthread.so.0
#2 0x0000000000697a32 in PGSemaphoreLock ()
#3 0x0000000000702a64 in LWLockAcquire ()
#4 0x00000000006fcb1c in LockRelease ()
#5 0x00000000006fa059 in UnlockRelationId ()
#6 0x00000000004b31c5 in relation_close ()
#7 0x00000000007f2e86 in SearchCatCacheMiss ()
#8 0x00000000007f37fd in SearchCatCache1 ()
#9 0x0000000000800510 in get_tablespace ()
#10 0x00000000008006e1 in get_tablespace_page_costs ()
#11 0x000000000065a4e1 in cost_seqscan ()
#12 0x000000000068bf92 in create_seqscan_path ()
#13 0x00000000006568b4 in set_rel_pathlist ()
#14 0x0000000000656eb8 in make_one_rel ()
#15 0x00000000006740d0 in query_planner ()
#16 0x0000000000676526 in grouping_planner ()
#17 0x0000000000679812 in subquery_planner ()
#18 0x000000000067a66c in standard_planner ()
#19 0x000000000070ffe1 in pg_plan_query ()
#20 0x00000000007100b6 in pg_plan_queries ()
#21 0x00000000007f6c6f in BuildCachedPlan ()
#22 0x00000000007f6e5c in GetCachedPlan ()
#23 0x0000000000711ccf in PostgresMain ()
#24 0x00000000006a5535 in backend_main_proc ()
#25 0x00000000006a353d in thread_trampoline ()
#26 0x00007f4c1cb3d36d in start_thread () from /lib64/libpthread.so.0
#27 0x00007f4c1c153b8f in clone () from /lib64/libc.so.6
...
--
Konstantin Knizhnik
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company
^ permalink raw reply [nested|flat] 51+ messages in thread
* Re: Postgres with pthread
@ 2017-12-27 10:05 james <[email protected]>
parent: Konstantin Knizhnik <[email protected]>
0 siblings, 1 reply; 51+ messages in thread
From: james @ 2017-12-27 10:05 UTC (permalink / raw)
To: Konstantin Knizhnik <[email protected]>; [email protected]
> All threads are blocked in semaphores.
That they are blocked is inevitable - I guess the issue is that they are
thrashing.
I guess it would be necessary to separate the internals to have some
internal queueing and effectively reduce the number of actively
executing threads.
In effect make the connection pooling work internally.
Would it be possible to make the caches have persistent (functional)
data structures - effectively CoW?
And how easy would it be to abort if the master view had subsequently
changed when it comes to execution?
^ permalink raw reply [nested|flat] 51+ messages in thread
* Re: Postgres with pthread
@ 2017-12-27 10:08 Andres Freund <[email protected]>
parent: james <[email protected]>
0 siblings, 2 replies; 51+ messages in thread
From: Andres Freund @ 2017-12-27 10:08 UTC (permalink / raw)
To: [email protected]; james <[email protected]>; Konstantin Knizhnik <[email protected]>; [email protected]
On December 27, 2017 11:05:52 AM GMT+01:00, james <[email protected]> wrote:
> > All threads are blocked in semaphores.
>That they are blocked is inevitable - I guess the issue is that they
>are
>thrashing.
>I guess it would be necessary to separate the internals to have some
>internal queueing and effectively reduce the number of actively
>executing threads.
>In effect make the connection pooling work internally.
>
>Would it be possible to make the caches have persistent (functional)
>data structures - effectively CoW?
>
>And how easy would it be to abort if the master view had subsequently
>changed when it comes to execution?
Optimizing for this seems like a pointless exercise. If the goal is efficient processing of 100k connections the solution is a session / connection abstraction and a scheduler. Optimizing for this amount of concurrency just will add complexity and slowdowns for a workload that nobody will run.
Andres
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.
^ permalink raw reply [nested|flat] 51+ messages in thread
* Re: Postgres with pthread
@ 2017-12-27 11:13 james <[email protected]>
parent: Andres Freund <[email protected]>
1 sibling, 0 replies; 51+ messages in thread
From: james @ 2017-12-27 11:13 UTC (permalink / raw)
To: Andres Freund <[email protected]>; Konstantin Knizhnik <[email protected]>; [email protected]
On 27/12/2017 10:08, Andres Freund wrote:
> Optimizing for this seems like a pointless exercise. If the goal is efficient processing of 100k connections the solution is a session / connection abstraction and a scheduler. Optimizing for this amount of concurrency just will add complexity and slowdowns for a workload that nobody will run.
Isn't that what's suggested?
The difference is that the session scheduler is inside the server.
Accepting 100k connections is no problem these days - giving each of
them an active thread seems to be the issue.
^ permalink raw reply [nested|flat] 51+ messages in thread
* Re: Postgres with pthread
@ 2017-12-27 11:17 Konstantin Knizhnik <[email protected]>
parent: Andres Freund <[email protected]>
1 sibling, 0 replies; 51+ messages in thread
From: Konstantin Knizhnik @ 2017-12-27 11:17 UTC (permalink / raw)
To: Andres Freund <[email protected]>; [email protected]; [email protected]
On 27.12.2017 13:08, Andres Freund wrote:
>
> On December 27, 2017 11:05:52 AM GMT+01:00, james <[email protected]> wrote:
>>> All threads are blocked in semaphores.
>> That they are blocked is inevitable - I guess the issue is that they
>> are
>> thrashing.
>> I guess it would be necessary to separate the internals to have some
>> internal queueing and effectively reduce the number of actively
>> executing threads.
>> In effect make the connection pooling work internally.
>>
>> Would it be possible to make the caches have persistent (functional)
>> data structures - effectively CoW?
>>
>> And how easy would it be to abort if the master view had subsequently
>> changed when it comes to execution?
> Optimizing for this seems like a pointless exercise. If the goal is efficient processing of 100k connections the solution is a session / connection abstraction and a scheduler. Optimizing for this amount of concurrency just will add complexity and slowdowns for a workload that nobody will run.
I agree with you that supporting 100k active connections has not so much
practical sense now.
But there are many systems with hundreds of cores and to utilize them we
still need spawn thousands of backends.
In this case Postgres snaphots and local caches becomes inefficient.
Switching to CSN allows to somehow solve the problem with snapshots.
But the problems with private caches should also be addressed: it seems
to be very stupid to perform the same work 1000x times and maintain
1000x copies.
Also, in case of global prepared statements, presence of global cache
allows to spend more time in plan optimization use manual tuning.
Switching to pthreads model significantly simplify development of
shared caches: there are no problems with statically allocated shared
address space or dynamic segments mapped on different address, not
allowing to use normal pointer. Also invalidation of shared cache is
easier: on need to send invalidation notifications to all backends.
But still it requires a lot of work. For example catalog cache is
tightly integrated with resource owner's information.
Also shared cache requires synchronization and this synchronization
itself can become a bottleneck.
> Andres
--
Konstantin Knizhnik
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company
^ permalink raw reply [nested|flat] 51+ messages in thread
* [PATCH v7 1/2] Move pg_upgrade kludges to sql script
@ 2021-03-07 00:35 Justin Pryzby <[email protected]>
0 siblings, 0 replies; 51+ messages in thread
From: Justin Pryzby @ 2021-03-07 00:35 UTC (permalink / raw)
NOTE, "IF EXISTS" isn't necessary in fa66b6dee
---
src/bin/pg_upgrade/test-upgrade.sql | 52 +++++++++++++++++++++++++++++
src/bin/pg_upgrade/test.sh | 48 +-------------------------
2 files changed, 53 insertions(+), 47 deletions(-)
create mode 100644 src/bin/pg_upgrade/test-upgrade.sql
diff --git a/src/bin/pg_upgrade/test-upgrade.sql b/src/bin/pg_upgrade/test-upgrade.sql
new file mode 100644
index 0000000000..5d74232c2b
--- /dev/null
+++ b/src/bin/pg_upgrade/test-upgrade.sql
@@ -0,0 +1,52 @@
+-- This file has a bunch of kludges needed for testing upgrades across major versions
+-- It supports testing the most recent version of an old release (not any arbitrary minor version).
+
+SELECT
+ ver <= 804 AS oldpgversion_le84,
+ ver < 1000 AS oldpgversion_lt10,
+ ver < 1200 AS oldpgversion_lt12,
+ ver < 1400 AS oldpgversion_lt14
+ FROM (SELECT current_setting('server_version_num')::int/100 AS ver) AS v;
+\gset
+
+\if :oldpgversion_le84
+DROP FUNCTION public.myfunc(integer);
+\endif
+
+\if :oldpgversion_lt10
+-- last in 9.6 -- commit 5ded4bd21
+DROP FUNCTION public.oldstyle_length(integer, text);
+\endif
+
+\if :oldpgversion_lt14
+-- last in v13 commit 7ca37fb04
+DROP FUNCTION IF EXISTS public.putenv(text);
+
+-- last in v13 commit 76f412ab3
+-- public.!=- This one is only needed for v11+ ??
+-- Note, until v10, operators could only be dropped one at a time
+DROP OPERATOR public.#@# (pg_catalog.int8, NONE);
+DROP OPERATOR public.#%# (pg_catalog.int8, NONE);
+DROP OPERATOR public.!=- (pg_catalog.int8, NONE);
+DROP OPERATOR public.#@%# (pg_catalog.int8, NONE);
+\endif
+
+\if :oldpgversion_lt12
+-- WITH OIDS is not supported anymore in v12, so remove support
+-- for any relations marked as such.
+DO $stmt$
+ DECLARE
+ rec text;
+ BEGIN
+ FOR rec in
+ SELECT oid::regclass::text
+ FROM pg_class
+ WHERE relname !~ '^pg_'
+ AND relhasoids
+ AND relkind in ('r','m')
+ ORDER BY 1
+ LOOP
+ execute 'ALTER TABLE ' || rec || ' SET WITHOUT OIDS';
+ END LOOP;
+ END; $stmt$;
+\endif
diff --git a/src/bin/pg_upgrade/test.sh b/src/bin/pg_upgrade/test.sh
index 8593488907..46a1ebb4ab 100644
--- a/src/bin/pg_upgrade/test.sh
+++ b/src/bin/pg_upgrade/test.sh
@@ -181,53 +181,7 @@ if "$MAKE" -C "$oldsrc" installcheck-parallel; then
# Before dumping, tweak the database of the old instance depending
# on its version.
if [ "$newsrc" != "$oldsrc" ]; then
- fix_sql=""
- # Get rid of objects not feasible in later versions
- case $oldpgversion in
- 804??)
- fix_sql="DROP FUNCTION public.myfunc(integer);"
- ;;
- esac
-
- # Last appeared in v9.6
- if [ $oldpgversion -lt 100000 ]; then
- fix_sql="$fix_sql
- DROP FUNCTION IF EXISTS
- public.oldstyle_length(integer, text);"
- fi
- # Last appeared in v13
- if [ $oldpgversion -lt 140000 ]; then
- fix_sql="$fix_sql
- DROP FUNCTION IF EXISTS
- public.putenv(text); -- last in v13
- DROP OPERATOR IF EXISTS -- last in v13
- public.#@# (pg_catalog.int8, NONE),
- public.#%# (pg_catalog.int8, NONE),
- public.!=- (pg_catalog.int8, NONE),
- public.#@%# (pg_catalog.int8, NONE);"
- fi
- psql -X -d regression -c "$fix_sql;" || psql_fix_sql_status=$?
-
- # WITH OIDS is not supported anymore in v12, so remove support
- # for any relations marked as such.
- if [ $oldpgversion -lt 120000 ]; then
- fix_sql="DO \$stmt\$
- DECLARE
- rec text;
- BEGIN
- FOR rec in
- SELECT oid::regclass::text
- FROM pg_class
- WHERE relname !~ '^pg_'
- AND relhasoids
- AND relkind in ('r','m')
- ORDER BY 1
- LOOP
- execute 'ALTER TABLE ' || rec || ' SET WITHOUT OIDS';
- END LOOP;
- END; \$stmt\$;"
- psql -X -d regression -c "$fix_sql;" || psql_fix_sql_status=$?
- fi
+ psql -X -d regression -f "test-upgrade.sql" || psql_fix_sql_status=$?
# Handling of --extra-float-digits gets messy after v12.
# Note that this changes the dumps from the old and new
--
2.17.0
--qD80nKKiJWXm4UaL
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v7-0002-wip-support-pg_upgrade-from-older-versions.patch"
^ permalink raw reply [nested|flat] 51+ messages in thread
* [PATCH v5 4/4] Move pg_upgrade kludges to sql script
@ 2021-03-07 00:35 Justin Pryzby <[email protected]>
0 siblings, 0 replies; 51+ messages in thread
From: Justin Pryzby @ 2021-03-07 00:35 UTC (permalink / raw)
---
src/bin/pg_upgrade/test-upgrade.sql | 70 +++++++++++++++++++++
src/bin/pg_upgrade/test.sh | 97 +----------------------------
2 files changed, 71 insertions(+), 96 deletions(-)
create mode 100644 src/bin/pg_upgrade/test-upgrade.sql
diff --git a/src/bin/pg_upgrade/test-upgrade.sql b/src/bin/pg_upgrade/test-upgrade.sql
new file mode 100644
index 0000000000..8c7cceb211
--- /dev/null
+++ b/src/bin/pg_upgrade/test-upgrade.sql
@@ -0,0 +1,70 @@
+-- This file has a bunch of kludges needed for testing upgrades across major versions
+
+SELECT
+ ver >= 804 AND ver <= 1100 AS oldpgversion_84_11,
+ ver >= 905 AND ver <= 1300 AS oldpgversion_95_13,
+ ver >= 906 AND ver <= 1300 AS oldpgversion_96_13,
+ ver >= 906 AND ver <= 1000 AS oldpgversion_96_10,
+ ver >= 1000 AS oldpgversion_ge10,
+ ver <= 804 AS oldpgversion_le84,
+ ver <= 1300 AS oldpgversion_le13
+ FROM (SELECT current_setting('server_version_num')::int/100 AS ver) AS v;
+\gset
+
+\if :oldpgversion_le84
+DROP FUNCTION public.myfunc(integer);
+\endif
+
+-- last in 9.6 -- commit 5ded4bd21
+DROP FUNCTION IF EXISTS public.oldstyle_length(integer, text);
+DROP FUNCTION IF EXISTS public.putenv(text);
+
+\if :oldpgversion_le13
+-- last in v13 commit 76f412ab3
+-- public.!=- This one is only needed for v11+ ??
+-- Note, until v10, operators could only be dropped one at a time
+DROP OPERATOR IF EXISTS public.#@# (pg_catalog.int8, NONE);
+DROP OPERATOR IF EXISTS public.#%# (pg_catalog.int8, NONE);
+DROP OPERATOR IF EXISTS public.!=- (pg_catalog.int8, NONE);
+DROP OPERATOR IF EXISTS public.#@%# (pg_catalog.int8, NONE);
+\endif
+
+\if :oldpgversion_ge10
+-- commit 068503c76511cdb0080bab689662a20e86b9c845
+DROP TRANSFORM FOR integer LANGUAGE sql CASCADE;
+\endif
+
+\if :oldpgversion_96_10
+-- commit db3af9feb19f39827e916145f88fa5eca3130cb2
+DROP FUNCTION boxarea(box);
+DROP FUNCTION funny_dup17();
+
+-- commit cda6a8d01d391eab45c4b3e0043a1b2b31072f5f
+DROP TABLE abstime_tbl;
+DROP TABLE reltime_tbl;
+DROP TABLE tinterval_tbl;
+\endif
+
+\if :oldpgversion_96_13
+-- Various things removed for v14
+DROP AGGREGATE first_el_agg_any(anyelement);
+\endif
+
+\if :oldpgversion_95_13
+-- commit 9e38c2bb5 and 97f73a978
+-- DROP AGGREGATE array_larger_accum(anyarray);
+DROP AGGREGATE array_cat_accum(anyarray);
+
+-- commit 76f412ab3
+-- DROP OPERATOR @#@(bigint,NONE);
+DROP OPERATOR @#@(NONE,bigint);
+\endif
+
+\if :oldpgversion_84_11
+-- commit 578b22971: OIDS removed in v12
+ALTER TABLE public.tenk1 SET WITHOUT OIDS;
+ALTER TABLE public.tenk1 SET WITHOUT OIDS;
+-- fix_sql="$fix_sql ALTER TABLE public.stud_emp SET WITHOUT OIDS;" # inherited
+ALTER TABLE public.emp SET WITHOUT OIDS;
+ALTER TABLE public.tt7 SET WITHOUT OIDS;
+\endif
diff --git a/src/bin/pg_upgrade/test.sh b/src/bin/pg_upgrade/test.sh
index 2bdd8c19de..61bcca3673 100644
--- a/src/bin/pg_upgrade/test.sh
+++ b/src/bin/pg_upgrade/test.sh
@@ -177,104 +177,9 @@ if "$MAKE" -C "$oldsrc" installcheck-parallel; then
# before dumping, get rid of objects not feasible in later versions
if [ "$newsrc" != "$oldsrc" ]; then
- fix_sql=""
- case $oldpgversion in
- 804??)
- fix_sql="DROP FUNCTION public.myfunc(integer);"
- ;;
- esac
- fix_sql="$fix_sql
- DROP FUNCTION IF EXISTS
- public.oldstyle_length(integer, text);" # last in 9.6 -- commit 5ded4bd21
- fix_sql="$fix_sql
- DROP FUNCTION IF EXISTS
- public.putenv(text);" # last in v13
- # last in v13 commit 76f412ab3
- # public.!=- This one is only needed for v11+ ??
- # Note, until v10, operators could only be dropped one at a time
- fix_sql="$fix_sql
- DROP OPERATOR IF EXISTS
- public.#@# (pg_catalog.int8, NONE);"
- fix_sql="$fix_sql
- DROP OPERATOR IF EXISTS
- public.#%# (pg_catalog.int8, NONE);"
- fix_sql="$fix_sql
- DROP OPERATOR IF EXISTS
- public.!=- (pg_catalog.int8, NONE);"
- fix_sql="$fix_sql
- DROP OPERATOR IF EXISTS
- public.#@%# (pg_catalog.int8, NONE);"
-
- # commit 068503c76511cdb0080bab689662a20e86b9c845
- case $oldpgversion in
- 10????)
- fix_sql="$fix_sql
- DROP TRANSFORM FOR integer LANGUAGE sql CASCADE;"
- ;;
- esac
-
- # commit db3af9feb19f39827e916145f88fa5eca3130cb2
- case $oldpgversion in
- 10????)
- fix_sql="$fix_sql
- DROP FUNCTION boxarea(box);"
- fix_sql="$fix_sql
- DROP FUNCTION funny_dup17();"
- ;;
- esac
-
- # commit cda6a8d01d391eab45c4b3e0043a1b2b31072f5f
- case $oldpgversion in
- 10????)
- fix_sql="$fix_sql
- DROP TABLE abstime_tbl;"
- fix_sql="$fix_sql
- DROP TABLE reltime_tbl;"
- fix_sql="$fix_sql
- DROP TABLE tinterval_tbl;"
- ;;
- esac
-
- # Various things removed for v14
- case $oldpgversion in
- 906??|10????|11????|12????|13????)
- fix_sql="$fix_sql
- DROP AGGREGATE first_el_agg_any(anyelement);"
- ;;
- esac
- case $oldpgversion in
- 90[56]??|10????|11????|12????|13????)
- # commit 9e38c2bb5 and 97f73a978
- # fix_sql="$fix_sql DROP AGGREGATE array_larger_accum(anyarray);"
- fix_sql="$fix_sql
- DROP AGGREGATE array_cat_accum(anyarray);"
-
- # commit 76f412ab3
- #fix_sql="$fix_sql DROP OPERATOR @#@(bigint,NONE);"
- fix_sql="$fix_sql
- DROP OPERATOR @#@(NONE,bigint);"
- ;;
- esac
-
- # commit 578b22971: OIDS removed in v12
- case $oldpgversion in
- 804??|9????|10????|11????)
- fix_sql="$fix_sql
- ALTER TABLE public.tenk1 SET WITHOUT OIDS;"
- fix_sql="$fix_sql
- ALTER TABLE public.tenk1 SET WITHOUT OIDS;"
- #fix_sql="$fix_sql ALTER TABLE public.stud_emp SET WITHOUT OIDS;" # inherited
- fix_sql="$fix_sql
- ALTER TABLE public.emp SET WITHOUT OIDS;"
- fix_sql="$fix_sql
- ALTER TABLE public.tt7 SET WITHOUT OIDS;"
- ;;
- esac
-
- psql -X -d regression -c "$fix_sql;" || psql_fix_sql_status=$?
+ psql -X -d regression -f "test-upgrade.sql" || psql_fix_sql_status=$?
fi
- echo "fix_sql: $oldpgversion: $fix_sql" >&2
pg_dumpall --extra-float-digits=0 --no-sync -f "$temp_root"/dump1.sql || pg_dumpall1_status=$?
if [ "$newsrc" != "$oldsrc" ]; then
--
2.17.0
--54u2kuW9sGWg/X+X
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v5-0003-pg_upgrade-test-to-exercise-binary-compatibility.patch"
^ permalink raw reply [nested|flat] 51+ messages in thread
* [PATCH v5 4/4] Move pg_upgrade kludges to sql script
@ 2021-03-07 00:35 Justin Pryzby <[email protected]>
0 siblings, 0 replies; 51+ messages in thread
From: Justin Pryzby @ 2021-03-07 00:35 UTC (permalink / raw)
---
src/bin/pg_upgrade/test-upgrade.sql | 89 ++++++++++++++++++++++++++
src/bin/pg_upgrade/test.sh | 96 +----------------------------
2 files changed, 90 insertions(+), 95 deletions(-)
create mode 100644 src/bin/pg_upgrade/test-upgrade.sql
diff --git a/src/bin/pg_upgrade/test-upgrade.sql b/src/bin/pg_upgrade/test-upgrade.sql
new file mode 100644
index 0000000000..3dadd6ea74
--- /dev/null
+++ b/src/bin/pg_upgrade/test-upgrade.sql
@@ -0,0 +1,89 @@
+-- This file has a bunch of kludges needed for upgrading testing across major versions
+
+SELECT
+ ver >= 804 AND ver <= 1100 AS fromv84v11,
+ ver >= 905 AND ver <= 1300 AS fromv95v13,
+ ver >= 906 AND ver <= 1300 AS fromv96v13,
+ ver <= 80400 AS fromv84,
+ ver <= 90500 AS fromv95,
+ ver <= 90600 AS fromv96,
+ ver <= 100000 AS fromv10,
+ ver <= 110000 AS fromv11,
+ ver <= 120000 AS fromv12,
+ ver <= 130000 AS fromv13
+ FROM (SELECT current_setting('server_version_num')::int/100 AS ver) AS v;
+\gset
+
+\if :fromv84
+DROP FUNCTION public.myfunc(integer);
+\endif
+
+-- last in 9.6 -- commit 5ded4bd21
+DROP FUNCTION IF EXISTS public.oldstyle_length(integer, text);
+DROP FUNCTION IF EXISTS public.putenv(text);
+
+\if :fromv13
+-- last in v13 commit 76f412ab3
+-- public.!=- This one is only needed for v11+ ??
+-- Note, until v10, operators could only be dropped one at a time
+DROP OPERATOR IF EXISTS public.#@# (pg_catalog.int8, NONE);
+DROP OPERATOR IF EXISTS public.#%# (pg_catalog.int8, NONE);
+DROP OPERATOR IF EXISTS public.!=- (pg_catalog.int8, NONE);
+DROP OPERATOR IF EXISTS public.#@%# (pg_catalog.int8, NONE);
+\endif
+
+\if :fromv10
+-- commit 068503c76511cdb0080bab689662a20e86b9c845
+DROP TRANSFORM FOR integer LANGUAGE sql CASCADE;
+
+-- commit db3af9feb19f39827e916145f88fa5eca3130cb2
+DROP FUNCTION boxarea(box);
+DROP FUNCTION funny_dup17();
+
+-- commit cda6a8d01d391eab45c4b3e0043a1b2b31072f5f
+DROP TABLE abstime_tbl;
+DROP TABLE reltime_tbl;
+DROP TABLE tinterval_tbl;
+\endif
+
+\if :fromv96v13
+-- Various things removed for v14
+DROP AGGREGATE first_el_agg_any(anyelement);
+\endif
+
+\if :fromv95v13
+-- commit 9e38c2bb5 and 97f73a978
+-- DROP AGGREGATE array_larger_accum(anyarray);
+DROP AGGREGATE array_cat_accum(anyarray);
+
+-- commit 76f412ab3
+-- DROP OPERATOR @#@(bigint,NONE);
+DROP OPERATOR @#@(NONE,bigint);
+\endif
+
+-- \if :fromv84v11
+\if :fromv11
+-- commit 578b22971: OIDS removed in v12
+ALTER TABLE public.tenk1 SET WITHOUT OIDS;
+ALTER TABLE public.tenk1 SET WITHOUT OIDS;
+-- fix_sql="$fix_sql ALTER TABLE public.stud_emp SET WITHOUT OIDS;" # inherited
+ALTER TABLE public.emp SET WITHOUT OIDS;
+ALTER TABLE public.tt7 SET WITHOUT OIDS;
+\endif
+
+-- if [ "$newsrc" != "$oldsrc" ]; then
+-- # update references to old source tree's regress.so etc
+-- fix_sql=""
+-- case $oldpgversion in
+-- 804??)
+-- fix_sql="UPDATE pg_proc SET probin = replace(probin::text, '$oldsrc', '$newsrc')::bytea WHERE probin LIKE '$oldsrc%';"
+-- ;;
+-- *)
+-- fix_sql="UPDATE pg_proc SET probin = replace(probin, '$oldsrc', '$newsrc') WHERE probin LIKE '$oldsrc%';"
+-- ;;
+-- esac
+-- psql -X -d regression -c "$fix_sql;" || psql_fix_sql_status=$?
+--
+-- mv "$temp_root"/dump1.sql "$temp_root"/dump1.sql.orig
+-- sed "s;$oldsrc;$newsrc;g" "$temp_root"/dump1.sql.orig >"$temp_root"/dump1.sql
+-- fi
diff --git a/src/bin/pg_upgrade/test.sh b/src/bin/pg_upgrade/test.sh
index 74c29229ac..a3df427f1d 100644
--- a/src/bin/pg_upgrade/test.sh
+++ b/src/bin/pg_upgrade/test.sh
@@ -178,101 +178,7 @@ if "$MAKE" -C "$oldsrc" installcheck-parallel; then
# before dumping, get rid of objects not feasible in later versions
if [ "$newsrc" != "$oldsrc" ]; then
- fix_sql=""
- case $oldpgversion in
- 804??)
- fix_sql="DROP FUNCTION public.myfunc(integer);"
- ;;
- esac
- fix_sql="$fix_sql
- DROP FUNCTION IF EXISTS
- public.oldstyle_length(integer, text);" # last in 9.6 -- commit 5ded4bd21
- fix_sql="$fix_sql
- DROP FUNCTION IF EXISTS
- public.putenv(text);" # last in v13
- # last in v13 commit 76f412ab3
- # public.!=- This one is only needed for v11+ ??
- # Note, until v10, operators could only be dropped one at a time
- fix_sql="$fix_sql
- DROP OPERATOR IF EXISTS
- public.#@# (pg_catalog.int8, NONE);"
- fix_sql="$fix_sql
- DROP OPERATOR IF EXISTS
- public.#%# (pg_catalog.int8, NONE);"
- fix_sql="$fix_sql
- DROP OPERATOR IF EXISTS
- public.!=- (pg_catalog.int8, NONE);"
- fix_sql="$fix_sql
- DROP OPERATOR IF EXISTS
- public.#@%# (pg_catalog.int8, NONE);"
-
- # commit 068503c76511cdb0080bab689662a20e86b9c845
- case $oldpgversion in
- 10????)
- fix_sql="$fix_sql
- DROP TRANSFORM FOR integer LANGUAGE sql CASCADE;"
- ;;
- esac
-
- # commit db3af9feb19f39827e916145f88fa5eca3130cb2
- case $oldpgversion in
- 10????)
- fix_sql="$fix_sql
- DROP FUNCTION boxarea(box);"
- fix_sql="$fix_sql
- DROP FUNCTION funny_dup17();"
- ;;
- esac
-
- # commit cda6a8d01d391eab45c4b3e0043a1b2b31072f5f
- case $oldpgversion in
- 10????)
- fix_sql="$fix_sql
- DROP TABLE abstime_tbl;"
- fix_sql="$fix_sql
- DROP TABLE reltime_tbl;"
- fix_sql="$fix_sql
- DROP TABLE tinterval_tbl;"
- ;;
- esac
-
- # Various things removed for v14
- case $oldpgversion in
- 906??|10????|11????|12????|13????)
- fix_sql="$fix_sql
- DROP AGGREGATE first_el_agg_any(anyelement);"
- ;;
- esac
- case $oldpgversion in
- 90[56]??|10????|11????|12????|13????)
- # commit 9e38c2bb5 and 97f73a978
- # fix_sql="$fix_sql DROP AGGREGATE array_larger_accum(anyarray);"
- fix_sql="$fix_sql
- DROP AGGREGATE array_cat_accum(anyarray);"
-
- # commit 76f412ab3
- #fix_sql="$fix_sql DROP OPERATOR @#@(bigint,NONE);"
- fix_sql="$fix_sql
- DROP OPERATOR @#@(NONE,bigint);"
- ;;
- esac
-
- # commit 578b22971: OIDS removed in v12
- case $oldpgversion in
- 804??|9????|10????|11????)
- fix_sql="$fix_sql
- ALTER TABLE public.tenk1 SET WITHOUT OIDS;"
- fix_sql="$fix_sql
- ALTER TABLE public.tenk1 SET WITHOUT OIDS;"
- #fix_sql="$fix_sql ALTER TABLE public.stud_emp SET WITHOUT OIDS;" # inherited
- fix_sql="$fix_sql
- ALTER TABLE public.emp SET WITHOUT OIDS;"
- fix_sql="$fix_sql
- ALTER TABLE public.tt7 SET WITHOUT OIDS;"
- ;;
- esac
-
- psql -X -d regression -c "$fix_sql;" || psql_fix_sql_status=$?
+ psql -X -d regression -f "test-upgrade.sql" || psql_fix_sql_status=$?
fi
echo "fix_sql: $oldpgversion: $fix_sql" >&2
--
2.17.0
--EuxKj2iCbKjpUGkD--
^ permalink raw reply [nested|flat] 51+ messages in thread
* [PATCH v6 2/2] Move pg_upgrade kludges to sql script
@ 2021-03-07 00:35 Justin Pryzby <[email protected]>
0 siblings, 0 replies; 51+ messages in thread
From: Justin Pryzby @ 2021-03-07 00:35 UTC (permalink / raw)
NOTE, "IF EXISTS" isn't necessary in fa66b6dee
---
src/bin/pg_upgrade/test-upgrade.sql | 51 +++++++++++++++++++++++++++++
src/bin/pg_upgrade/test.sh | 48 +--------------------------
2 files changed, 52 insertions(+), 47 deletions(-)
create mode 100644 src/bin/pg_upgrade/test-upgrade.sql
diff --git a/src/bin/pg_upgrade/test-upgrade.sql b/src/bin/pg_upgrade/test-upgrade.sql
new file mode 100644
index 0000000000..74fad312cb
--- /dev/null
+++ b/src/bin/pg_upgrade/test-upgrade.sql
@@ -0,0 +1,51 @@
+-- This file has a bunch of kludges needed for testing upgrades across major versions
+-- It supports testing the most recent version of an old release (not any arbitrary minor version).
+
+SELECT
+ ver <= 804 AS oldpgversion_le84,
+ ver < 1000 AS oldpgversion_lt10,
+ ver < 1200 AS oldpgversion_lt12,
+ ver < 1400 AS oldpgversion_lt14
+ FROM (SELECT current_setting('server_version_num')::int/100 AS ver) AS v;
+\gset
+
+\if :oldpgversion_le84
+DROP FUNCTION public.myfunc(integer);
+\endif
+
+\if :oldpgversion_lt10
+-- last in 9.6 -- commit 5ded4bd21
+DROP FUNCTION public.oldstyle_length(integer, text);
+\endif
+
+\if :oldpgversion_lt14
+-- last in v13 commit 7ca37fb04
+DROP FUNCTION IF EXISTS public.putenv(text);
+-- last in v13 commit 76f412ab3
+-- public.!=- This one is only needed for v11+ ??
+-- Note, until v10, operators could only be dropped one at a time
+DROP OPERATOR public.#@# (pg_catalog.int8, NONE);
+DROP OPERATOR public.#%# (pg_catalog.int8, NONE);
+DROP OPERATOR public.!=- (pg_catalog.int8, NONE);
+DROP OPERATOR public.#@%# (pg_catalog.int8, NONE);
+\endif
+
+\if :oldpgversion_lt12
+-- WITH OIDS is not supported anymore in v12, so remove support
+-- for any relations marked as such.
+DO $stmt$
+ DECLARE
+ rec text;
+ BEGIN
+ FOR rec in
+ SELECT oid::regclass::text
+ FROM pg_class
+ WHERE relname !~ '^pg_'
+ AND relhasoids
+ AND relkind in ('r','m')
+ ORDER BY 1
+ LOOP
+ execute 'ALTER TABLE ' || rec || ' SET WITHOUT OIDS';
+ END LOOP;
+ END; $stmt$;
+\endif
diff --git a/src/bin/pg_upgrade/test.sh b/src/bin/pg_upgrade/test.sh
index 8593488907..46a1ebb4ab 100644
--- a/src/bin/pg_upgrade/test.sh
+++ b/src/bin/pg_upgrade/test.sh
@@ -181,53 +181,7 @@ if "$MAKE" -C "$oldsrc" installcheck-parallel; then
# Before dumping, tweak the database of the old instance depending
# on its version.
if [ "$newsrc" != "$oldsrc" ]; then
- fix_sql=""
- # Get rid of objects not feasible in later versions
- case $oldpgversion in
- 804??)
- fix_sql="DROP FUNCTION public.myfunc(integer);"
- ;;
- esac
-
- # Last appeared in v9.6
- if [ $oldpgversion -lt 100000 ]; then
- fix_sql="$fix_sql
- DROP FUNCTION IF EXISTS
- public.oldstyle_length(integer, text);"
- fi
- # Last appeared in v13
- if [ $oldpgversion -lt 140000 ]; then
- fix_sql="$fix_sql
- DROP FUNCTION IF EXISTS
- public.putenv(text); -- last in v13
- DROP OPERATOR IF EXISTS -- last in v13
- public.#@# (pg_catalog.int8, NONE),
- public.#%# (pg_catalog.int8, NONE),
- public.!=- (pg_catalog.int8, NONE),
- public.#@%# (pg_catalog.int8, NONE);"
- fi
- psql -X -d regression -c "$fix_sql;" || psql_fix_sql_status=$?
-
- # WITH OIDS is not supported anymore in v12, so remove support
- # for any relations marked as such.
- if [ $oldpgversion -lt 120000 ]; then
- fix_sql="DO \$stmt\$
- DECLARE
- rec text;
- BEGIN
- FOR rec in
- SELECT oid::regclass::text
- FROM pg_class
- WHERE relname !~ '^pg_'
- AND relhasoids
- AND relkind in ('r','m')
- ORDER BY 1
- LOOP
- execute 'ALTER TABLE ' || rec || ' SET WITHOUT OIDS';
- END LOOP;
- END; \$stmt\$;"
- psql -X -d regression -c "$fix_sql;" || psql_fix_sql_status=$?
- fi
+ psql -X -d regression -f "test-upgrade.sql" || psql_fix_sql_status=$?
# Handling of --extra-float-digits gets messy after v12.
# Note that this changes the dumps from the old and new
--
2.17.0
--g4MvFqI7wmANiPDo--
^ permalink raw reply [nested|flat] 51+ messages in thread
* Re: log_min_messages per backend type
@ 2025-02-05 18:51 Álvaro Herrera <[email protected]>
0 siblings, 1 reply; 51+ messages in thread
From: Álvaro Herrera @ 2025-02-05 18:51 UTC (permalink / raw)
To: Euler Taveira <[email protected]>; +Cc: [email protected]
Hello Euler,
On 2024-Dec-17, Euler Taveira wrote:
> Sometimes you need to inspect some debug messages from autovacuum worker but
> you cannot apply the same setting for backends (that could rapidly fill the log
> file). This proposal aims to change log_min_messages to have different log
> levels depending on which backend type the message comes from.
>
> The syntax was changed from enum to string and it accepts a list of elements.
>
> Instead of enum, it now accepts a comma-separated list of elements (string).
> Each element is LOGLEVEL:BACKENDTYPE.
This format seems unintuitive. I would have thought you do it the other
way around, "backendtype:loglevel" ... that seems more natural because
it's like assigning the 'loglevel' value to the 'backendtype' element.
SET log_min_messages TO 'checkpointer:debug2, autovacuum:debug1';
I dislike the array of names in variable.c. We already have an array in
launch_backend.c (child_process_kinds), plus GetBackendTypeDesc in
miscinit.c. Maybe not for this patch to clean up though.
I think it should be acceptable to configure one global setting with
exceptions for particular backend types:
log_min_messages = WARNING, autovacuum:DEBUG1
Right now I think the code only accepts the unadorned log level if there
are no other items in the list. I think the proposal downthread is to
use the keyword ALL for this,
log_min_messages = all:WARNING, autovacuum:DEBUG1 # I don't like this
but I think it's inferior, because then "all" is not really "all", and I
think it would be different if I had said
log_min_messages = autovacuum:DEBUG1, all:WARNING # I don't like this
because it looks like the "all" entry should override the one I set for
autovacuum before, which frankly would not make sense to me.
So I think these two lines,
log_min_messages = WARNING, autovacuum:DEBUG1
log_min_messages = autovacuum:DEBUG1, WARNING
should behave identically and mean "set the level for autovacuum to
DEBUG1, and to any other backend type to WARNING.
Also, I think it'd be better to reject duplicates in the list. Right
now it looks like the last entry for one backend type overrides prior
ones. I mean
log_min_messages = autovacuum:DEBUG1, autovacuum:ERROR
would set autovacuum to error, but that might be mistake prone if your
string is long. So implementation-wise I suggest to initialize the
whole newlogminmsgs array to -1, then scan the list of entries (saving
an entry without backend type as the one to use later and) setting every
backend type to the number specified; if we see trying to set a value
that's already different from -1, throw error. After scanning the whole
log_min_messages array, we scan the newlogminmsgs and set any entries
that are still -1 to the value that we saved before.
The new code in variable.c should be before the /* DATESTYLE */ comment
rather than at the end of the file.
You still have many XXX comments. Also, postgresql.conf should list the
valid values for backendtype, as well as show an example of a valid
setting. Please don't use ALLCAPS backend types in the docs, this looks
ugly:
> + Valid <literal>BACKENDTYPE</literal> values are <literal>ARCHIVER</literal>,
> + <literal>AUTOVACUUM</literal>, <literal>BACKEND</literal>,
> + <literal>BGWORKER</literal>, <literal>BGWRITER</literal>,
> + <literal>CHECKPOINTER</literal>, <literal>LOGGER</literal>,
> + <literal>SLOTSYNCWORKER</literal>, <literal>WALRECEIVER</literal>,
> + <literal>WALSENDER</literal>, <literal>WALSUMMARIZER</literal>, and
> + <literal>WALWRITER</literal>.
--
Álvaro Herrera Breisgau, Deutschland — https://www.EnterpriseDB.com/
"No necesitamos banderas
No reconocemos fronteras" (Jorge González)
^ permalink raw reply [nested|flat] 51+ messages in thread
* Re: log_min_messages per backend type
@ 2025-03-05 00:33 Euler Taveira <[email protected]>
parent: Álvaro Herrera <[email protected]>
0 siblings, 3 replies; 51+ messages in thread
From: Euler Taveira @ 2025-03-05 00:33 UTC (permalink / raw)
To: Alvaro Herrera <[email protected]>; +Cc: [email protected]
On Wed, Feb 5, 2025, at 3:51 PM, Álvaro Herrera wrote:
> On 2024-Dec-17, Euler Taveira wrote:
>
> > Sometimes you need to inspect some debug messages from autovacuum worker but
> > you cannot apply the same setting for backends (that could rapidly fill the log
> > file). This proposal aims to change log_min_messages to have different log
> > levels depending on which backend type the message comes from.
> >
> > The syntax was changed from enum to string and it accepts a list of elements.
> >
> > Instead of enum, it now accepts a comma-separated list of elements (string).
> > Each element is LOGLEVEL:BACKENDTYPE.
>
> This format seems unintuitive. I would have thought you do it the other
> way around, "backendtype:loglevel" ... that seems more natural because
> it's like assigning the 'loglevel' value to the 'backendtype' element.
> SET log_min_messages TO 'checkpointer:debug2, autovacuum:debug1';
Alvaro, thanks for your feedback. Your suggestion makes sense to me.
> I dislike the array of names in variable.c. We already have an array in
> launch_backend.c (child_process_kinds), plus GetBackendTypeDesc in
> miscinit.c. Maybe not for this patch to clean up though.
I thought about using child_process_kinds but two details made me give
up on the idea: (a) multiple names per backend type (for example,
B_BACKEND, B_DEAD_END_BACKEND, B_STANDALONE_BACKEND) and (b) spaces into
names. Maybe we should have a group into child_process_kinds but as you
said it seems material for another patch.
> I think it should be acceptable to configure one global setting with
> exceptions for particular backend types:
>
> log_min_messages = WARNING, autovacuum:DEBUG1
>
> Right now I think the code only accepts the unadorned log level if there
> are no other items in the list. I think the proposal downthread is to
> use the keyword ALL for this,
>
> log_min_messages = all:WARNING, autovacuum:DEBUG1 # I don't like this
>
> but I think it's inferior, because then "all" is not really "all", and I
> think it would be different if I had said
>
> log_min_messages = autovacuum:DEBUG1, all:WARNING # I don't like this
>
> because it looks like the "all" entry should override the one I set for
> autovacuum before, which frankly would not make sense to me.
Good point. After reflection, I agree that "all" is not a good keyword.
This patch turns backend type as optional so WARNING means apply this
log level as a final step to the backend types that are not specified in
the list.
> So I think these two lines,
>
> log_min_messages = WARNING, autovacuum:DEBUG1
> log_min_messages = autovacuum:DEBUG1, WARNING
>
> should behave identically and mean "set the level for autovacuum to
> DEBUG1, and to any other backend type to WARNING.
Done.
> Also, I think it'd be better to reject duplicates in the list. Right
> now it looks like the last entry for one backend type overrides prior
> ones. I mean
>
> log_min_messages = autovacuum:DEBUG1, autovacuum:ERROR
>
> would set autovacuum to error, but that might be mistake prone if your
> string is long. So implementation-wise I suggest to initialize the
> whole newlogminmsgs array to -1, then scan the list of entries (saving
> an entry without backend type as the one to use later and) setting every
> backend type to the number specified; if we see trying to set a value
> that's already different from -1, throw error. After scanning the whole
> log_min_messages array, we scan the newlogminmsgs and set any entries
> that are still -1 to the value that we saved before.
>
>
> The new code in variable.c should be before the /* DATESTYLE */ comment
> rather than at the end of the file.
It was added into the MISCELLANEOUS section.
>
> You still have many XXX comments. Also, postgresql.conf should list the
> valid values for backendtype, as well as show an example of a valid
> setting. Please don't use ALLCAPS backend types in the docs, this looks
> ugly:
>
> > + Valid <literal>BACKENDTYPE</literal> values are <literal>ARCHIVER</literal>,
> > + <literal>AUTOVACUUM</literal>, <literal>BACKEND</literal>,
> > + <literal>BGWORKER</literal>, <literal>BGWRITER</literal>,
> > + <literal>CHECKPOINTER</literal>, <literal>LOGGER</literal>,
> > + <literal>SLOTSYNCWORKER</literal>, <literal>WALRECEIVER</literal>,
> > + <literal>WALSENDER</literal>, <literal>WALSUMMARIZER</literal>, and
> > + <literal>WALWRITER</literal>.
Done.
Just to recap what was changed:
- patch was rebased
- backend type is optional and means all unspecified backend types
- generic log level (without backend type) is mandatory and the order it
ppears is not important (it is applied for the remaining backend types)
- fix Windows build
- new tests to cover the changes
--
Euler Taveira
EDB https://www.enterprisedb.com/
Attachments:
[text/x-patch] v2-0001-log_min_messages-per-backend-type.patch (21.2K, ../../[email protected]/3-v2-0001-log_min_messages-per-backend-type.patch)
download | inline diff:
From 0640ddb6e5510f1b600982331490188d20bfa7e9 Mon Sep 17 00:00:00 2001
From: Euler Taveira <[email protected]>
Date: Mon, 11 Dec 2023 17:24:50 -0300
Subject: [PATCH v2] log_min_messages per backend type
Change log_min_messages from a single element to a comma-separated list
of elements. Each element is backendtype:loglevel. The backendtype are
archiver, autovacuum (includes launcher and workers), backend, bgworker,
bgwriter, checkpointer, logger, slotsyncworker, walreceiver, walsender,
walsummarizer and walwriter. It should be part of this list a single log
level that is applied as a final step to the backend types that are not
informed.
The old syntax (a single log level) is still accepted for backward
compatibility. In this case, it means the informed log level is applied
for the backend types that are not informed. The default log level is
the same (WARNING) for all backend types.
---
doc/src/sgml/config.sgml | 30 ++-
src/backend/commands/extension.c | 2 +-
src/backend/commands/variable.c | 187 ++++++++++++++++++
src/backend/utils/error/elog.c | 2 +-
src/backend/utils/misc/guc_tables.c | 81 ++++++--
src/backend/utils/misc/postgresql.conf.sample | 1 +
src/include/miscadmin.h | 2 +
src/include/utils/guc.h | 2 +-
src/include/utils/guc_hooks.h | 2 +
src/include/utils/guc_tables.h | 4 +
src/test/regress/expected/guc.out | 48 +++++
src/test/regress/sql/guc.sql | 16 ++
12 files changed, 352 insertions(+), 25 deletions(-)
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index d2fa5f7d1a9..3e8df85556d 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -6861,7 +6861,7 @@ local0.* /var/log/postgresql
<variablelist>
<varlistentry id="guc-log-min-messages" xreflabel="log_min_messages">
- <term><varname>log_min_messages</varname> (<type>enum</type>)
+ <term><varname>log_min_messages</varname> (<type>string</type>)
<indexterm>
<primary><varname>log_min_messages</varname> configuration parameter</primary>
</indexterm>
@@ -6870,14 +6870,26 @@ local0.* /var/log/postgresql
<para>
Controls which <link linkend="runtime-config-severity-levels">message
levels</link> are written to the server log.
- Valid values are <literal>DEBUG5</literal>, <literal>DEBUG4</literal>,
- <literal>DEBUG3</literal>, <literal>DEBUG2</literal>, <literal>DEBUG1</literal>,
- <literal>INFO</literal>, <literal>NOTICE</literal>, <literal>WARNING</literal>,
- <literal>ERROR</literal>, <literal>LOG</literal>, <literal>FATAL</literal>, and
- <literal>PANIC</literal>. Each level includes all the levels that
- follow it. The later the level, the fewer messages are sent
- to the log. The default is <literal>WARNING</literal>. Note that
- <literal>LOG</literal> has a different rank here than in
+ Valid values are a comma-separated list of <literal>backendtype:level</literal>
+ and a single <literal>level</literal>. The list allows it to use
+ different levels per backend type.
+ Valid <literal>backendtype</literal> values are <literal>archiver</literal>,
+ <literal>autovacuum</literal>, <literal>backend</literal>,
+ <literal>bgworker</literal>, <literal>bgwriter</literal>,
+ <literal>checkpointer</literal>, <literal>logger</literal>,
+ <literal>slotsyncworker</literal>, <literal>walreceiver</literal>,
+ <literal>walsender</literal>, <literal>walsummarizer</literal>, and
+ <literal>walwriter</literal>.
+ Valid <literal>LEVEL</literal> values are <literal>DEBUG5</literal>,
+ <literal>DEBUG4</literal>, <literal>DEBUG3</literal>, <literal>DEBUG2</literal>,
+ <literal>DEBUG1</literal>, <literal>INFO</literal>, <literal>NOTICE</literal>,
+ <literal>WARNING</literal>, <literal>ERROR</literal>, <literal>LOG</literal>,
+ <literal>FATAL</literal>, and <literal>PANIC</literal>. Each level includes
+ all the levels that follow it. The later the level, the fewer messages are sent
+ to the log. The single <literal>LEVEL</literal> is mandatory and it is
+ assigned to the backend types that are not specified in the list. The
+ default is <literal>WARNING</literal> for all backend types.
+ Note that <literal>LOG</literal> has a different rank here than in
<xref linkend="guc-client-min-messages"/>.
Only superusers and users with the appropriate <literal>SET</literal>
privilege can change this setting.
diff --git a/src/backend/commands/extension.c b/src/backend/commands/extension.c
index d9bb4ce5f1e..93923c26a7e 100644
--- a/src/backend/commands/extension.c
+++ b/src/backend/commands/extension.c
@@ -1059,7 +1059,7 @@ execute_extension_script(Oid extensionOid, ExtensionControlFile *control,
(void) set_config_option("client_min_messages", "warning",
PGC_USERSET, PGC_S_SESSION,
GUC_ACTION_SAVE, true, 0, false);
- if (log_min_messages < WARNING)
+ if (log_min_messages[MyBackendType] < WARNING)
(void) set_config_option_ext("log_min_messages", "warning",
PGC_SUSET, PGC_S_SESSION,
BOOTSTRAP_SUPERUSERID,
diff --git a/src/backend/commands/variable.c b/src/backend/commands/variable.c
index 4ad6e236d69..0f646298bf1 100644
--- a/src/backend/commands/variable.c
+++ b/src/backend/commands/variable.c
@@ -35,6 +35,7 @@
#include "utils/datetime.h"
#include "utils/fmgrprotos.h"
#include "utils/guc_hooks.h"
+#include "utils/guc_tables.h"
#include "utils/snapmgr.h"
#include "utils/syscache.h"
#include "utils/timestamp.h"
@@ -1271,3 +1272,189 @@ check_ssl(bool *newval, void **extra, GucSource source)
#endif
return true;
}
+
+/*
+ * GUC check_hook for log_min_messages
+ *
+ * The parsing consists of a comma-separated list of BACKENDTYPENAME:LEVEL
+ * elements. BACKENDTYPENAME is log_min_messages_backend_types. LEVEL is
+ * server_message_level_options. A single LEVEL element should be part of this
+ * list and it is applied as a final step to the backend types that are not
+ * specified. For backward compatibility, the old syntax is still accepted and
+ * it means to apply this level for all backend types.
+ */
+bool
+check_log_min_messages(char **newval, void **extra, GucSource source)
+{
+ char *rawstring;
+ List *elemlist;
+ ListCell *l;
+ int newlevels[BACKEND_NUM_TYPES];
+ bool assigned[BACKEND_NUM_TYPES];
+ int genericlevel = -1; /* -1 means not assigned */
+
+ /* Initialize the array. */
+ memset(newlevels, WARNING, BACKEND_NUM_TYPES * sizeof(int));
+ memset(assigned, false, BACKEND_NUM_TYPES * sizeof(bool));
+
+ /* Need a modifiable copy of string. */
+ rawstring = pstrdup(*newval);
+
+ /* Parse string into list of identifiers. */
+ if (!SplitGUCList(rawstring, ',', &elemlist))
+ {
+ /* syntax error in list */
+ GUC_check_errdetail("List syntax is invalid.");
+ pfree(rawstring);
+ list_free(elemlist);
+ return false;
+ }
+
+ /* Validate and assign log level and backend type. */
+ foreach(l, elemlist)
+ {
+ char *tok = (char *) lfirst(l);
+ char *sep;
+ const struct config_enum_entry *entry;
+
+ /*
+ * Check whether there is a backend type following the log level. If
+ * there is no separator, it means this log level should be applied
+ * for all backend types (backward compatibility).
+ */
+ sep = strchr(tok, ':');
+ if (sep == NULL)
+ {
+ bool found = false;
+
+ /* Reject duplicates for generic log level. */
+ if (genericlevel != -1)
+ {
+ GUC_check_errdetail("Generic log level was already assigned.");
+ pfree(rawstring);
+ list_free(elemlist);
+ return false;
+ }
+
+ /* Is the log level valid? */
+ for (entry = server_message_level_options; entry && entry->name; entry++)
+ {
+ if (pg_strcasecmp(entry->name, tok) == 0)
+ {
+ genericlevel = entry->val;
+ found = true;
+ break;
+ }
+ }
+
+ if (!found)
+ {
+ GUC_check_errdetail("Unrecognized log level: \"%s\".", tok);
+ pfree(rawstring);
+ list_free(elemlist);
+ return false;
+ }
+ }
+ else
+ {
+ char *loglevel;
+ char *btype;
+ bool found = false;
+
+ btype = palloc((sep - tok) + 1);
+ memcpy(btype, tok, sep - tok);
+ btype[sep - tok] = '\0';
+ loglevel = pstrdup(sep + 1);
+
+ /* Is the log level valid? */
+ for (entry = server_message_level_options; entry && entry->name; entry++)
+ {
+ if (pg_strcasecmp(entry->name, loglevel) == 0)
+ {
+ found = true;
+ break;
+ }
+ }
+
+ if (!found)
+ {
+ GUC_check_errdetail("Unrecognized log level: \"%s\".", loglevel);
+ pfree(rawstring);
+ list_free(elemlist);
+ return false;
+ }
+
+ /*
+ * Is the backend type name valid? There might be multiple entries
+ * per backend type, don't bail out when find first occurrence.
+ */
+ found = false;
+ for (int i = 0; i < BACKEND_NUM_TYPES; i++)
+ {
+ if (pg_strcasecmp(log_min_messages_backend_types[i], btype) == 0)
+ {
+ newlevels[i] = entry->val;
+ assigned[i] = true;
+ found = true;
+ }
+ }
+
+ if (!found)
+ {
+ GUC_check_errdetail("Unrecognized backend type: \"%s\".", btype);
+ pfree(rawstring);
+ list_free(elemlist);
+ return false;
+ }
+ }
+ }
+
+ /*
+ * Generic log level must be specified. It is a good idea to specify a
+ * generic log level to make it clear that it is the fallback value.
+ * Although, we can document it, it might confuse users that used to
+ * specify a single log level in prior releases.
+ */
+ if (genericlevel == -1)
+ {
+ GUC_check_errdetail("Generic log level was not defined.");
+ pfree(rawstring);
+ list_free(elemlist);
+ return false;
+ }
+
+ /*
+ * Apply the generic log level (the one without a backend type) after all
+ * of the specific backend type have been assigned. Hence, it doesn't
+ * matter the order you specify the generic log level, the final result
+ * will be the same.
+ */
+ for (int i = 0; i < BACKEND_NUM_TYPES; i++)
+ {
+ if (!assigned[i])
+ newlevels[i] = genericlevel;
+ }
+
+ pfree(rawstring);
+ list_free(elemlist);
+
+ /*
+ * Pass back data for assign_log_min_messages to use.
+ */
+ *extra = guc_malloc(LOG, BACKEND_NUM_TYPES * sizeof(int));
+ if (!*extra)
+ return false;
+ memcpy(*extra, newlevels, BACKEND_NUM_TYPES * sizeof(int));
+
+ return true;
+}
+
+/*
+ * GUC assign_hook for log_min_messages
+ */
+void
+assign_log_min_messages(const char *newval, void *extra)
+{
+ for (int i = 0; i < BACKEND_NUM_TYPES; i++)
+ log_min_messages[i] = ((int *) extra)[i];
+}
diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c
index 860bbd40d42..fc5aedab0c8 100644
--- a/src/backend/utils/error/elog.c
+++ b/src/backend/utils/error/elog.c
@@ -234,7 +234,7 @@ is_log_level_output(int elevel, int log_min_level)
static inline bool
should_output_to_server(int elevel)
{
- return is_log_level_output(elevel, log_min_messages);
+ return is_log_level_output(elevel, log_min_messages[MyBackendType]);
}
/*
diff --git a/src/backend/utils/misc/guc_tables.c b/src/backend/utils/misc/guc_tables.c
index ad25cbb39c5..9d38474fc79 100644
--- a/src/backend/utils/misc/guc_tables.c
+++ b/src/backend/utils/misc/guc_tables.c
@@ -139,7 +139,7 @@ static const struct config_enum_entry client_message_level_options[] = {
{NULL, 0, false}
};
-static const struct config_enum_entry server_message_level_options[] = {
+const struct config_enum_entry server_message_level_options[] = {
{"debug5", DEBUG5, false},
{"debug4", DEBUG4, false},
{"debug3", DEBUG3, false},
@@ -521,7 +521,6 @@ static bool default_with_oids = false;
bool current_role_is_superuser;
int log_min_error_statement = ERROR;
-int log_min_messages = WARNING;
int client_min_messages = NOTICE;
int log_min_duration_sample = -1;
int log_min_duration_statement = -1;
@@ -579,6 +578,7 @@ static char *server_version_string;
static int server_version_num;
static char *debug_io_direct_string;
static char *restrict_nonsystem_relation_kind_string;
+static char *log_min_messages_string;
#ifdef HAVE_SYSLOG
#define DEFAULT_SYSLOG_FACILITY LOG_LOCAL0
@@ -623,6 +623,60 @@ char *role_string;
/* should be static, but guc.c needs to get at this */
bool in_hot_standby_guc;
+/*
+ * This must match enum BackendType! It should be static, but
+ * commands/variable.c needs to get at this.
+ */
+int log_min_messages[] = {
+ [B_INVALID] = WARNING,
+ [B_BACKEND] = WARNING,
+ [B_DEAD_END_BACKEND] = WARNING,
+ [B_AUTOVAC_LAUNCHER] = WARNING,
+ [B_AUTOVAC_WORKER] = WARNING,
+ [B_BG_WORKER] = WARNING,
+ [B_WAL_SENDER] = WARNING,
+ [B_SLOTSYNC_WORKER] = WARNING,
+ [B_STANDALONE_BACKEND] = WARNING,
+ [B_ARCHIVER] = WARNING,
+ [B_BG_WRITER] = WARNING,
+ [B_CHECKPOINTER] = WARNING,
+ [B_STARTUP] = WARNING,
+ [B_WAL_RECEIVER] = WARNING,
+ [B_WAL_SUMMARIZER] = WARNING,
+ [B_WAL_WRITER] = WARNING,
+ [B_LOGGER] = WARNING,
+};
+
+StaticAssertDecl(lengthof(log_min_messages) == BACKEND_NUM_TYPES,
+ "array length mismatch");
+
+/*
+ * This must match enum BackendType! It might be in commands/variable.c but for
+ * convenience it is near log_min_messages.
+ */
+const char *const log_min_messages_backend_types[] = {
+ [B_INVALID] = "backend", /* XXX same as backend? */
+ [B_BACKEND] = "backend",
+ [B_DEAD_END_BACKEND] = "backend", /* XXX same as backend? */
+ [B_AUTOVAC_LAUNCHER] = "autovacuum",
+ [B_AUTOVAC_WORKER] = "autovacuum",
+ [B_BG_WORKER] = "bgworker",
+ [B_WAL_SENDER] = "walsender",
+ [B_SLOTSYNC_WORKER] = "slotsyncworker",
+ [B_STANDALONE_BACKEND] = "backend", /* XXX same as backend? */
+ [B_ARCHIVER] = "archiver",
+ [B_BG_WRITER] = "bgwriter",
+ [B_CHECKPOINTER] = "checkpointer",
+ [B_STARTUP] = "backend", /* XXX same as backend? */
+ [B_WAL_RECEIVER] = "walreceiver",
+ [B_WAL_SUMMARIZER] = "walsummarizer",
+ [B_WAL_WRITER] = "walwriter",
+ [B_LOGGER] = "logger",
+};
+
+StaticAssertDecl(lengthof(log_min_messages_backend_types) == BACKEND_NUM_TYPES,
+ "array length mismatch");
+
/*
* Displayable names for context types (enum GucContext)
@@ -4223,6 +4277,18 @@ struct config_string ConfigureNamesString[] =
check_client_encoding, assign_client_encoding, NULL
},
+ {
+ {"log_min_messages", PGC_SUSET, LOGGING_WHEN,
+ gettext_noop("Sets the message levels that are logged."),
+ gettext_noop("Each level includes all the levels that follow it. The later"
+ " the level, the fewer messages are sent."),
+ GUC_LIST_INPUT
+ },
+ &log_min_messages_string,
+ "WARNING",
+ check_log_min_messages, assign_log_min_messages, NULL
+ },
+
{
{"log_line_prefix", PGC_SIGHUP, LOGGING_WHAT,
gettext_noop("Controls information prefixed to each log line."),
@@ -5011,17 +5077,6 @@ struct config_enum ConfigureNamesEnum[] =
NULL, NULL, NULL
},
- {
- {"log_min_messages", PGC_SUSET, LOGGING_WHEN,
- gettext_noop("Sets the message levels that are logged."),
- gettext_noop("Each level includes all the levels that follow it. The later"
- " the level, the fewer messages are sent.")
- },
- &log_min_messages,
- WARNING, server_message_level_options,
- NULL, NULL, NULL
- },
-
{
{"log_min_error_statement", PGC_SUSET, LOGGING_WHEN,
gettext_noop("Causes all statements generating error at or above this level to be logged."),
diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample
index 2d1de9c37bd..c99e1fb90ae 100644
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -526,6 +526,7 @@
# log
# fatal
# panic
+ # or a comma-separated list of backend_type:level
#log_min_error_statement = error # values in order of decreasing detail:
# debug5
diff --git a/src/include/miscadmin.h b/src/include/miscadmin.h
index a2b63495eec..36b0b82cf3e 100644
--- a/src/include/miscadmin.h
+++ b/src/include/miscadmin.h
@@ -332,6 +332,8 @@ extern void SwitchBackToLocalLatch(void);
*
* If you add entries, please also update the child_process_kinds array in
* launch_backend.c.
+ * XXX If you add a new backend type or change the order, update
+ * log_min_messages because it relies on this order to work correctly.
*/
typedef enum BackendType
{
diff --git a/src/include/utils/guc.h b/src/include/utils/guc.h
index 1233e07d7da..bb23ca65449 100644
--- a/src/include/utils/guc.h
+++ b/src/include/utils/guc.h
@@ -271,7 +271,7 @@ extern PGDLLIMPORT bool log_duration;
extern PGDLLIMPORT int log_parameter_max_length;
extern PGDLLIMPORT int log_parameter_max_length_on_error;
extern PGDLLIMPORT int log_min_error_statement;
-extern PGDLLIMPORT int log_min_messages;
+extern PGDLLIMPORT int log_min_messages[];
extern PGDLLIMPORT int client_min_messages;
extern PGDLLIMPORT int log_min_duration_sample;
extern PGDLLIMPORT int log_min_duration_statement;
diff --git a/src/include/utils/guc_hooks.h b/src/include/utils/guc_hooks.h
index 951451a9765..cf27960e239 100644
--- a/src/include/utils/guc_hooks.h
+++ b/src/include/utils/guc_hooks.h
@@ -176,5 +176,7 @@ extern bool check_synchronized_standby_slots(char **newval, void **extra,
extern void assign_synchronized_standby_slots(const char *newval, void *extra);
extern bool check_idle_replication_slot_timeout(int *newval, void **extra,
GucSource source);
+extern bool check_log_min_messages(char **newval, void **extra, GucSource source);
+extern void assign_log_min_messages(const char *newval, void *extra);
#endif /* GUC_HOOKS_H */
diff --git a/src/include/utils/guc_tables.h b/src/include/utils/guc_tables.h
index ab47145ec36..8a60ec5997a 100644
--- a/src/include/utils/guc_tables.h
+++ b/src/include/utils/guc_tables.h
@@ -297,6 +297,10 @@ struct config_enum
void *reset_extra;
};
+/* log_min_messages */
+extern PGDLLIMPORT const char *const log_min_messages_backend_types[];
+extern PGDLLIMPORT const struct config_enum_entry server_message_level_options[];
+
/* constant tables corresponding to enums above and in guc.h */
extern PGDLLIMPORT const char *const config_group_names[];
extern PGDLLIMPORT const char *const config_type_names[];
diff --git a/src/test/regress/expected/guc.out b/src/test/regress/expected/guc.out
index 7f9e29c765c..7572e0addbc 100644
--- a/src/test/regress/expected/guc.out
+++ b/src/test/regress/expected/guc.out
@@ -913,3 +913,51 @@ SELECT name FROM tab_settings_flags
(0 rows)
DROP TABLE tab_settings_flags;
+-- Test log_min_messages
+SET log_min_messages TO fatal;
+SHOW log_min_messages;
+ log_min_messages
+------------------
+ fatal
+(1 row)
+
+SET log_min_messages TO 'fatal';
+SHOW log_min_messages;
+ log_min_messages
+------------------
+ fatal
+(1 row)
+
+SET log_min_messages TO 'checkpointer:debug2, autovacuum:debug1'; --fail
+ERROR: invalid value for parameter "log_min_messages": "checkpointer:debug2, autovacuum:debug1"
+DETAIL: Generic log level was not defined.
+SET log_min_messages TO 'debug1, backend:error, fatal'; -- fail
+ERROR: invalid value for parameter "log_min_messages": "debug1, backend:error, fatal"
+DETAIL: Generic log level was already assigned.
+SET log_min_messages TO 'backend:error, foo:fatal, archiver:debug1'; -- fail
+ERROR: invalid value for parameter "log_min_messages": "backend:error, foo:fatal, archiver:debug1"
+DETAIL: Unrecognized backend type: "foo".
+SET log_min_messages TO 'backend:error, checkpointer:bar, archiver:debug1'; -- fail
+ERROR: invalid value for parameter "log_min_messages": "backend:error, checkpointer:bar, archiver:debug1"
+DETAIL: Unrecognized log level: "bar".
+SET log_min_messages TO 'backend:error, checkpointer:debug3, fatal, archiver:debug2, autovacuum:debug1, walsender:debug3';
+SHOW log_min_messages;
+ log_min_messages
+-------------------------------------------------------------------------------------------------
+ backend:error, checkpointer:debug3, fatal, archiver:debug2, autovacuum:debug1, walsender:debug3
+(1 row)
+
+SET log_min_messages TO 'warning, autovacuum:debug1';
+SHOW log_min_messages;
+ log_min_messages
+----------------------------
+ warning, autovacuum:debug1
+(1 row)
+
+SET log_min_messages TO 'autovacuum:debug1, warning';
+SHOW log_min_messages;
+ log_min_messages
+----------------------------
+ autovacuum:debug1, warning
+(1 row)
+
diff --git a/src/test/regress/sql/guc.sql b/src/test/regress/sql/guc.sql
index f65f84a2632..e9dddb4e72d 100644
--- a/src/test/regress/sql/guc.sql
+++ b/src/test/regress/sql/guc.sql
@@ -368,3 +368,19 @@ SELECT name FROM tab_settings_flags
WHERE no_reset AND NOT no_reset_all
ORDER BY 1;
DROP TABLE tab_settings_flags;
+
+-- Test log_min_messages
+SET log_min_messages TO fatal;
+SHOW log_min_messages;
+SET log_min_messages TO 'fatal';
+SHOW log_min_messages;
+SET log_min_messages TO 'checkpointer:debug2, autovacuum:debug1'; --fail
+SET log_min_messages TO 'debug1, backend:error, fatal'; -- fail
+SET log_min_messages TO 'backend:error, foo:fatal, archiver:debug1'; -- fail
+SET log_min_messages TO 'backend:error, checkpointer:bar, archiver:debug1'; -- fail
+SET log_min_messages TO 'backend:error, checkpointer:debug3, fatal, archiver:debug2, autovacuum:debug1, walsender:debug3';
+SHOW log_min_messages;
+SET log_min_messages TO 'warning, autovacuum:debug1';
+SHOW log_min_messages;
+SET log_min_messages TO 'autovacuum:debug1, warning';
+SHOW log_min_messages;
--
2.39.5
^ permalink raw reply [nested|flat] 51+ messages in thread
* Re: log_min_messages per backend type
@ 2025-03-05 16:40 Andrew Dunstan <[email protected]>
parent: Euler Taveira <[email protected]>
2 siblings, 2 replies; 51+ messages in thread
From: Andrew Dunstan @ 2025-03-05 16:40 UTC (permalink / raw)
To: Euler Taveira <[email protected]>; Alvaro Herrera <[email protected]>; +Cc: [email protected]
On 2025-03-04 Tu 7:33 PM, Euler Taveira wrote:
>> I think it should be acceptable to configure one global setting with
>> exceptions for particular backend types:
>>
>> log_min_messages = WARNING, autovacuum:DEBUG1
>>
>> Right now I think the code only accepts the unadorned log level if there
>> are no other items in the list. I think the proposal downthread is to
>> use the keyword ALL for this,
>>
>> log_min_messages = all:WARNING, autovacuum:DEBUG1 # I don't like this
>>
>> but I think it's inferior, because then "all" is not really "all", and I
>> think it would be different if I had said
>>
>> log_min_messages = autovacuum:DEBUG1, all:WARNING # I don't like this
>>
>> because it looks like the "all" entry should override the one I set for
>> autovacuum before, which frankly would not make sense to me.
>
> Good point. After reflection, I agree that "all" is not a good keyword.
> This patch turns backend type as optional so WARNING means apply this
> log level as a final step to the backend types that are not specified in
> the list.
>
>> So I think these two lines,
>>
>> log_min_messages = WARNING, autovacuum:DEBUG1
>> log_min_messages = autovacuum:DEBUG1, WARNING
>>
>> should behave identically and mean "set the level for autovacuum to
>> DEBUG1, and to any other backend type to WARNING.
>
> Done.
Just bikeshedding a bit ...
I'm not mad keen on this design. I think the value should be either a
single setting like "WARNING" or a set of type:setting pairs. I agree
that "all" is a bad name, but I think "default" would make sense.
I can live with it but I think this just looks a bit odd.
cheers
andrew
--
Andrew Dunstan
EDB:https://www.enterprisedb.com
^ permalink raw reply [nested|flat] 51+ messages in thread
* Re: log_min_messages per backend type
@ 2025-03-06 02:53 Fujii Masao <[email protected]>
parent: Euler Taveira <[email protected]>
2 siblings, 0 replies; 51+ messages in thread
From: Fujii Masao @ 2025-03-06 02:53 UTC (permalink / raw)
To: Euler Taveira <[email protected]>; Alvaro Herrera <[email protected]>; +Cc: [email protected]
On 2025/03/05 9:33, Euler Taveira wrote:
>> > + Valid <literal>BACKENDTYPE</literal> values are <literal>ARCHIVER</literal>,
>> > + <literal>AUTOVACUUM</literal>, <literal>BACKEND</literal>,
>> > + <literal>BGWORKER</literal>, <literal>BGWRITER</literal>,
>> > + <literal>CHECKPOINTER</literal>, <literal>LOGGER</literal>,
>> > + <literal>SLOTSYNCWORKER</literal>, <literal>WALRECEIVER</literal>,
>> > + <literal>WALSENDER</literal>, <literal>WALSUMMARIZER</literal>, and
>> > + <literal>WALWRITER</literal>.
What about postmaster?
For parallel workers launched for parallel queries, should they follow
the backend's log level or the background worker's? Since they operate
as part of a parallel query executed by a backend, it seems more logical
for them to follow the backend's setting.
+ [B_CHECKPOINTER] = "checkpointer",
+ [B_STARTUP] = "backend", /* XXX same as backend? */
I like the idea of allowing log levels to be set per process.
There were times I wanted to use debug5 specifically for
the startup process when troubleshooting WAL replay. It would be
helpful to distinguish the startup process from a regular backend,
so we can set its log level independently.
Regards,
--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
^ permalink raw reply [nested|flat] 51+ messages in thread
* Re: log_min_messages per backend type
@ 2025-03-06 13:20 Matheus Alcantara <[email protected]>
parent: Andrew Dunstan <[email protected]>
1 sibling, 0 replies; 51+ messages in thread
From: Matheus Alcantara @ 2025-03-06 13:20 UTC (permalink / raw)
To: Andrew Dunstan <[email protected]>; Euler Taveira <[email protected]>; Alvaro Herrera <[email protected]>; +Cc: [email protected]
Hi,
>
> On 2025-03-04 Tu 7:33 PM, Euler Taveira wrote:
>>> I think it should be acceptable to configure one global setting with
>>> exceptions for particular backend types:
>>>
>>> log_min_messages = WARNING, autovacuum:DEBUG1
>>>
>>> Right now I think the code only accepts the unadorned log level if
>>> there
>>> are no other items in the list. I think the proposal downthread is to
>>> use the keyword ALL for this,
>>>
>>> log_min_messages = all:WARNING, autovacuum:DEBUG1 # I don't
>>> like this
>>>
>>> but I think it's inferior, because then "all" is not really "all",
>>> and I
>>> think it would be different if I had said
>>>
>>> log_min_messages = autovacuum:DEBUG1, all:WARNING # I don't
>>> like this
>>>
>>> because it looks like the "all" entry should override the one I set
>>> for
>>> autovacuum before, which frankly would not make sense to me.
>>
>> Good point. After reflection, I agree that "all" is not a good keyword.
>> This patch turns backend type as optional so WARNING means apply this
>> log level as a final step to the backend types that are not
>> specified in
>> the list.
>>
>>> So I think these two lines,
>>>
>>> log_min_messages = WARNING, autovacuum:DEBUG1
>>> log_min_messages = autovacuum:DEBUG1, WARNING
>>>
>>> should behave identically and mean "set the level for autovacuum to
>>> DEBUG1, and to any other backend type to WARNING.
>>
>> Done.
>
>
> Just bikeshedding a bit ...
>
> I'm not mad keen on this design. I think the value should be either a
> single setting like "WARNING" or a set of type:setting pairs. I agree
> that "all" is a bad name, but I think "default" would make sense.
>
> I can live with it but I think this just looks a bit odd.
>
Just bringing some thoughts about it...
How about using something like *:WARNING? I'm not sure if it could also be
confusing as the "all" keyword, but I think it could also be interpreted as
"anything else use WARNING".
--
Matheus Alcantara
^ permalink raw reply [nested|flat] 51+ messages in thread
* Re: log_min_messages per backend type
@ 2025-03-06 13:33 Andres Freund <[email protected]>
parent: Euler Taveira <[email protected]>
2 siblings, 1 reply; 51+ messages in thread
From: Andres Freund @ 2025-03-06 13:33 UTC (permalink / raw)
To: Euler Taveira <[email protected]>; +Cc: Alvaro Herrera <[email protected]>; [email protected]
Hi,
On 2025-03-04 21:33:39 -0300, Euler Taveira wrote:
> +/*
> + * This must match enum BackendType! It should be static, but
> + * commands/variable.c needs to get at this.
> + */
> +int log_min_messages[] = {
> + [B_INVALID] = WARNING,
> + [B_BACKEND] = WARNING,
> + [B_DEAD_END_BACKEND] = WARNING,
> + [B_AUTOVAC_LAUNCHER] = WARNING,
> + [B_AUTOVAC_WORKER] = WARNING,
> + [B_BG_WORKER] = WARNING,
> + [B_WAL_SENDER] = WARNING,
> + [B_SLOTSYNC_WORKER] = WARNING,
> + [B_STANDALONE_BACKEND] = WARNING,
> + [B_ARCHIVER] = WARNING,
> + [B_BG_WRITER] = WARNING,
> + [B_CHECKPOINTER] = WARNING,
> + [B_STARTUP] = WARNING,
> + [B_WAL_RECEIVER] = WARNING,
> + [B_WAL_SUMMARIZER] = WARNING,
> + [B_WAL_WRITER] = WARNING,
> + [B_LOGGER] = WARNING,
> +};
> +StaticAssertDecl(lengthof(log_min_messages) == BACKEND_NUM_TYPES,
> + "array length mismatch");
> +
> +/*
> + * This must match enum BackendType! It might be in commands/variable.c but for
> + * convenience it is near log_min_messages.
> + */
> +const char *const log_min_messages_backend_types[] = {
> + [B_INVALID] = "backend", /* XXX same as backend? */
> + [B_BACKEND] = "backend",
> + [B_DEAD_END_BACKEND] = "backend", /* XXX same as backend? */
> + [B_AUTOVAC_LAUNCHER] = "autovacuum",
> + [B_AUTOVAC_WORKER] = "autovacuum",
> + [B_BG_WORKER] = "bgworker",
> + [B_WAL_SENDER] = "walsender",
> + [B_SLOTSYNC_WORKER] = "slotsyncworker",
> + [B_STANDALONE_BACKEND] = "backend", /* XXX same as backend? */
> + [B_ARCHIVER] = "archiver",
> + [B_BG_WRITER] = "bgwriter",
> + [B_CHECKPOINTER] = "checkpointer",
> + [B_STARTUP] = "backend", /* XXX same as backend? */
Huh, the startup process is among the most crucial things to monitor?
> + [B_WAL_RECEIVER] = "walreceiver",
> + [B_WAL_SUMMARIZER] = "walsummarizer",
> + [B_WAL_WRITER] = "walwriter",
> + [B_LOGGER] = "logger",
> +};
> +
> +StaticAssertDecl(lengthof(log_min_messages_backend_types) == BACKEND_NUM_TYPES,
> + "array length mismatch");
> +
I don't know what I think about the whole patch, but I do want to voice
*strong* opposition to duplicating a list of all backend types into multiple
places. It's already painfull enough to add a new backend type, without having
to pointlessly go around and manually add a new backend type to mulltiple
arrays that have completely predictable content.
Greetings,
Andres Freund
^ permalink raw reply [nested|flat] 51+ messages in thread
* Re: log_min_messages per backend type
@ 2025-03-06 21:33 Euler Taveira <[email protected]>
parent: Andrew Dunstan <[email protected]>
1 sibling, 0 replies; 51+ messages in thread
From: Euler Taveira @ 2025-03-06 21:33 UTC (permalink / raw)
To: Andrew Dunstan <[email protected]>; Alvaro Herrera <[email protected]>; +Cc: [email protected]
On Wed, Mar 5, 2025, at 1:40 PM, Andrew Dunstan wrote:
> Just bikeshedding a bit ...
>
> I'm not mad keen on this design. I think the value should be either a single setting like "WARNING" or a set of type:setting pairs. I agree that "all" is a bad name, but I think "default" would make sense.
>
One of my main concerns was a clear interface. I think "default" is less
confusing than "all". Your suggestion about single or list is aligned with what
Alvaro suggested. IIUC you are suggesting default:loglevel only if it is part
of the list; the single loglevel shouldn't contain the backend type to keep the
backward compatibility. The advantage of your proposal is that it make it clear
what the fallback log level is. However, someone could be confused asking if
the "default" is a valid backend type and if there is a difference between
WARNING and default:WARNING (both is a fallback for non-specified backend type
elements).
--
Euler Taveira
EDB https://www.enterprisedb.com/
^ permalink raw reply [nested|flat] 51+ messages in thread
* Re: log_min_messages per backend type
@ 2025-07-31 14:19 Euler Taveira <[email protected]>
parent: Andres Freund <[email protected]>
0 siblings, 2 replies; 51+ messages in thread
From: Euler Taveira @ 2025-07-31 14:19 UTC (permalink / raw)
To: Andres Freund <[email protected]>; +Cc: Alvaro Herrera <[email protected]>; [email protected]
On Thu, Mar 6, 2025, at 10:33 AM, Andres Freund wrote:
> Huh, the startup process is among the most crucial things to monitor?
>
Good point. Fixed.
After collecting some suggestions, I'm attaching a new patch contains the
following changes:
- patch was rebased
- include Alvaro's patch (v2-0001) [1] as a basis for this patch
- add ioworker as new backend type
- add startup as new backend type per Andres suggestion
- small changes into documentation
> I don't know what I think about the whole patch, but I do want to voice
> *strong* opposition to duplicating a list of all backend types into multiple
> places. It's already painfull enough to add a new backend type, without having
> to pointlessly go around and manually add a new backend type to mulltiple
> arrays that have completely predictable content.
>
I'm including Alvaro's patch as is just to make the CF bot happy and to
illustrate how it would be if we adopt his solution to centralize the list of
backend types. I think Alvaro's proposal overcomes the objection [2], right?
[1] https://www.postgresql.org/message-id/[email protected]
[2] https://www.postgresql.org/message-id/y5tgui75jrcj6mm5nmoq4yqwage2432akx4kp2ogtcnim3wskx@2ipmtfi4qvp...
--
Euler Taveira
EDB https://www.enterprisedb.com/
Attachments:
[text/x-patch] v3-0001-Create-a-separate-file-listing-backend-types.patch (7.0K, ../../[email protected]/2-v3-0001-Create-a-separate-file-listing-backend-types.patch)
download | inline diff:
From ec409dada269cf54b9bce16cae473a46b2400598 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C3=81lvaro=20Herrera?= <[email protected]>
Date: Tue, 15 Jul 2025 18:19:27 +0200
Subject: [PATCH v3 1/2] Create a separate file listing backend types
Use our established coding pattern to reduce maintenance pain when
adding other per-process-type characteristics.
Like PG_KEYWORD, PG_CMDTAG, PG_RMGR.
---
src/backend/postmaster/launch_backend.c | 32 ++------------
src/backend/utils/init/miscinit.c | 59 ++-----------------------
src/include/postmaster/proctypelist.h | 50 +++++++++++++++++++++
3 files changed, 58 insertions(+), 83 deletions(-)
create mode 100644 src/include/postmaster/proctypelist.h
diff --git a/src/backend/postmaster/launch_backend.c b/src/backend/postmaster/launch_backend.c
index bf6b55ee830..8b2f1a0cf41 100644
--- a/src/backend/postmaster/launch_backend.c
+++ b/src/backend/postmaster/launch_backend.c
@@ -177,34 +177,10 @@ typedef struct
} child_process_kind;
static child_process_kind child_process_kinds[] = {
- [B_INVALID] = {"invalid", NULL, false},
-
- [B_BACKEND] = {"backend", BackendMain, true},
- [B_DEAD_END_BACKEND] = {"dead-end backend", BackendMain, true},
- [B_AUTOVAC_LAUNCHER] = {"autovacuum launcher", AutoVacLauncherMain, true},
- [B_AUTOVAC_WORKER] = {"autovacuum worker", AutoVacWorkerMain, true},
- [B_BG_WORKER] = {"bgworker", BackgroundWorkerMain, true},
-
- /*
- * WAL senders start their life as regular backend processes, and change
- * their type after authenticating the client for replication. We list it
- * here for PostmasterChildName() but cannot launch them directly.
- */
- [B_WAL_SENDER] = {"wal sender", NULL, true},
- [B_SLOTSYNC_WORKER] = {"slot sync worker", ReplSlotSyncWorkerMain, true},
-
- [B_STANDALONE_BACKEND] = {"standalone backend", NULL, false},
-
- [B_ARCHIVER] = {"archiver", PgArchiverMain, true},
- [B_BG_WRITER] = {"bgwriter", BackgroundWriterMain, true},
- [B_CHECKPOINTER] = {"checkpointer", CheckpointerMain, true},
- [B_IO_WORKER] = {"io_worker", IoWorkerMain, true},
- [B_STARTUP] = {"startup", StartupProcessMain, true},
- [B_WAL_RECEIVER] = {"wal_receiver", WalReceiverMain, true},
- [B_WAL_SUMMARIZER] = {"wal_summarizer", WalSummarizerMain, true},
- [B_WAL_WRITER] = {"wal_writer", WalWriterMain, true},
-
- [B_LOGGER] = {"syslogger", SysLoggerMain, false},
+#define PG_PROCTYPE(bktype, description, main_func, shmem_attach) \
+ [bktype] = {description, main_func, shmem_attach},
+#include "postmaster/proctypelist.h"
+#undef PG_PROCTYPE
};
const char *
diff --git a/src/backend/utils/init/miscinit.c b/src/backend/utils/init/miscinit.c
index 43b4dbccc3d..dec220a61f5 100644
--- a/src/backend/utils/init/miscinit.c
+++ b/src/backend/utils/init/miscinit.c
@@ -266,62 +266,11 @@ GetBackendTypeDesc(BackendType backendType)
switch (backendType)
{
- case B_INVALID:
- backendDesc = gettext_noop("not initialized");
- break;
- case B_ARCHIVER:
- backendDesc = gettext_noop("archiver");
- break;
- case B_AUTOVAC_LAUNCHER:
- backendDesc = gettext_noop("autovacuum launcher");
- break;
- case B_AUTOVAC_WORKER:
- backendDesc = gettext_noop("autovacuum worker");
- break;
- case B_BACKEND:
- backendDesc = gettext_noop("client backend");
- break;
- case B_DEAD_END_BACKEND:
- backendDesc = gettext_noop("dead-end client backend");
- break;
- case B_BG_WORKER:
- backendDesc = gettext_noop("background worker");
- break;
- case B_BG_WRITER:
- backendDesc = gettext_noop("background writer");
- break;
- case B_CHECKPOINTER:
- backendDesc = gettext_noop("checkpointer");
- break;
- case B_IO_WORKER:
- backendDesc = gettext_noop("io worker");
- break;
- case B_LOGGER:
- backendDesc = gettext_noop("logger");
- break;
- case B_SLOTSYNC_WORKER:
- backendDesc = gettext_noop("slotsync worker");
- break;
- case B_STANDALONE_BACKEND:
- backendDesc = gettext_noop("standalone backend");
- break;
- case B_STARTUP:
- backendDesc = gettext_noop("startup");
- break;
- case B_WAL_RECEIVER:
- backendDesc = gettext_noop("walreceiver");
- break;
- case B_WAL_SENDER:
- backendDesc = gettext_noop("walsender");
- break;
- case B_WAL_SUMMARIZER:
- backendDesc = gettext_noop("walsummarizer");
- break;
- case B_WAL_WRITER:
- backendDesc = gettext_noop("walwriter");
- break;
+#define PG_PROCTYPE(bktype, description, main_func, shmem_attach) \
+ case bktype: backendDesc = gettext_noop(description); break;
+#include "postmaster/proctypelist.h"
+#undef PG_PROCTYPE
}
-
return backendDesc;
}
diff --git a/src/include/postmaster/proctypelist.h b/src/include/postmaster/proctypelist.h
new file mode 100644
index 00000000000..919f00bb3a7
--- /dev/null
+++ b/src/include/postmaster/proctypelist.h
@@ -0,0 +1,50 @@
+/*-------------------------------------------------------------------------
+ *
+ * proctypelist.h
+ *
+ * The list of process types is kept on its own source file for use by
+ * automatic tools. The exact representation of a process type is
+ * determined by the PG_PROCTYPE macro, which is not defined in this
+ * file; it can be defined by the caller for special purposes.
+ *
+ * Portions Copyright (c) 1996-2025, PostgreSQL Global Development Group
+ * Portions Copyright (c) 1994, Regents of the University of California
+ *
+ * IDENTIFICATION
+ * src/include/postmaster/proctypelist.h
+ *
+ *-------------------------------------------------------------------------
+ */
+
+/* there is deliberately not an #ifndef PROCTYPELIST_H here */
+
+/*
+ * WAL senders start their life as regular backend processes, and change their
+ * type after authenticating the client for replication. We list it here for
+ * PostmasterChildName() but cannot launch them directly.
+ */
+
+/*
+ * List of process types (symbol, description, Main function, shmem_attach)
+ * entries.
+ */
+
+/* bktype, description, main_func, shmem_attach */
+PG_PROCTYPE(B_ARCHIVER, "archiver", PgArchiverMain, true)
+PG_PROCTYPE(B_AUTOVAC_LAUNCHER, "autovacuum launcher", AutoVacLauncherMain, true)
+PG_PROCTYPE(B_AUTOVAC_WORKER, "autovacuum worker", AutoVacWorkerMain, true)
+PG_PROCTYPE(B_BACKEND, "client backend", BackendMain, true)
+PG_PROCTYPE(B_BG_WORKER, "background worker", BackgroundWorkerMain, true)
+PG_PROCTYPE(B_BG_WRITER, "background writer", BackgroundWriterMain, true)
+PG_PROCTYPE(B_CHECKPOINTER, "checkpointer", CheckpointerMain, true)
+PG_PROCTYPE(B_DEAD_END_BACKEND, "dead-end client backend", BackendMain, true)
+PG_PROCTYPE(B_INVALID, "unrecognized", NULL, false)
+PG_PROCTYPE(B_IO_WORKER, "io worker", IoWorkerMain, true)
+PG_PROCTYPE(B_LOGGER, "syslogger", SysLoggerMain, false)
+PG_PROCTYPE(B_SLOTSYNC_WORKER, "slotsync worker", ReplSlotSyncWorkerMain, true)
+PG_PROCTYPE(B_STANDALONE_BACKEND, "standalone backend", NULL, false)
+PG_PROCTYPE(B_STARTUP, "startup", StartupProcessMain, true)
+PG_PROCTYPE(B_WAL_RECEIVER, "walreceiver", WalReceiverMain, true)
+PG_PROCTYPE(B_WAL_SENDER, "walsender", NULL, true)
+PG_PROCTYPE(B_WAL_SUMMARIZER, "walsummarizer", WalSummarizerMain, true)
+PG_PROCTYPE(B_WAL_WRITER, "walwriter", WalWriterMain, true)
--
2.39.5
[text/x-patch] v3-0002-log_min_messages-per-backend-type.patch (24.7K, ../../[email protected]/3-v3-0002-log_min_messages-per-backend-type.patch)
download | inline diff:
From a36fb59413fc6cca46a87accebebd08e515429a8 Mon Sep 17 00:00:00 2001
From: Euler Taveira <[email protected]>
Date: Mon, 11 Dec 2023 17:24:50 -0300
Subject: [PATCH v3 2/2] log_min_messages per backend type
Change log_min_messages from a single element to a comma-separated list
of elements. Each element is backendtype:loglevel. The backendtype are
archiver, autovacuum (includes launcher and workers), backend, bgworker,
bgwriter, checkpointer, ioworker, logger, slotsyncworker, walreceiver,
walsender, walsummarizer and walwriter. A single log level should be
part of this list that is applied as a final step to the backend types
that are not informed.
The old syntax (a single log level) is still accepted for backward
compatibility. In this case, it means the informed log level is applied
for the backend types that are not informed. The default log level is
the same (WARNING) for all backend types.
---
doc/src/sgml/config.sgml | 32 ++-
src/backend/commands/extension.c | 2 +-
src/backend/commands/variable.c | 196 ++++++++++++++++++
src/backend/postmaster/launch_backend.c | 2 +-
src/backend/utils/error/elog.c | 2 +-
src/backend/utils/init/miscinit.c | 2 +-
src/backend/utils/misc/guc_tables.c | 48 +++--
src/backend/utils/misc/postgresql.conf.sample | 1 +
src/include/postmaster/proctypelist.h | 38 ++--
src/include/utils/guc.h | 2 +-
src/include/utils/guc_hooks.h | 2 +
src/include/utils/guc_tables.h | 4 +
src/test/regress/expected/guc.out | 51 +++++
src/test/regress/sql/guc.sql | 17 ++
14 files changed, 353 insertions(+), 46 deletions(-)
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 20ccb2d6b54..7c4002f2266 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -7021,7 +7021,7 @@ local0.* /var/log/postgresql
<variablelist>
<varlistentry id="guc-log-min-messages" xreflabel="log_min_messages">
- <term><varname>log_min_messages</varname> (<type>enum</type>)
+ <term><varname>log_min_messages</varname> (<type>string</type>)
<indexterm>
<primary><varname>log_min_messages</varname> configuration parameter</primary>
</indexterm>
@@ -7030,14 +7030,28 @@ local0.* /var/log/postgresql
<para>
Controls which <link linkend="runtime-config-severity-levels">message
levels</link> are written to the server log.
- Valid values are <literal>DEBUG5</literal>, <literal>DEBUG4</literal>,
- <literal>DEBUG3</literal>, <literal>DEBUG2</literal>, <literal>DEBUG1</literal>,
- <literal>INFO</literal>, <literal>NOTICE</literal>, <literal>WARNING</literal>,
- <literal>ERROR</literal>, <literal>LOG</literal>, <literal>FATAL</literal>, and
- <literal>PANIC</literal>. Each level includes all the levels that
- follow it. The later the level, the fewer messages are sent
- to the log. The default is <literal>WARNING</literal>. Note that
- <literal>LOG</literal> has a different rank here than in
+ Valid values are a comma-separated list of <literal>backendtype:level</literal>
+ and a single <literal>level</literal>. The list allows it to use
+ different levels per backend type.
+ Valid <literal>backendtype</literal> values are <literal>archiver</literal>,
+ <literal>autovacuum</literal>, <literal>backend</literal>,
+ <literal>bgworker</literal>, <literal>bgwriter</literal>,
+ <literal>checkpointer</literal>, <literal>ioworker</literal>,
+ <literal>logger</literal>, <literal>slotsyncworker</literal>,
+ <literal>startup</literal>, <literal>walreceiver</literal>,
+ <literal>walsender</literal>, <literal>walsummarizer</literal>, and
+ <literal>walwriter</literal>.
+ Valid <literal>LEVEL</literal> values are <literal>DEBUG5</literal>,
+ <literal>DEBUG4</literal>, <literal>DEBUG3</literal>, <literal>DEBUG2</literal>,
+ <literal>DEBUG1</literal>, <literal>INFO</literal>, <literal>NOTICE</literal>,
+ <literal>WARNING</literal>, <literal>ERROR</literal>, <literal>LOG</literal>,
+ <literal>FATAL</literal>, and <literal>PANIC</literal>. Each level includes
+ all the levels that follow it. The later the level, the fewer messages are sent
+ to the log. A single <literal>LEVEL</literal> is mandatory (order does
+ not matter) and it is assigned to the backend types that are not
+ specified in the list. The default is <literal>WARNING</literal> for
+ all backend types.
+ Note that <literal>LOG</literal> has a different rank here than in
<xref linkend="guc-client-min-messages"/>.
Only superusers and users with the appropriate <literal>SET</literal>
privilege can change this setting.
diff --git a/src/backend/commands/extension.c b/src/backend/commands/extension.c
index e6f9ab6dfd6..b6055431d8f 100644
--- a/src/backend/commands/extension.c
+++ b/src/backend/commands/extension.c
@@ -1143,7 +1143,7 @@ execute_extension_script(Oid extensionOid, ExtensionControlFile *control,
(void) set_config_option("client_min_messages", "warning",
PGC_USERSET, PGC_S_SESSION,
GUC_ACTION_SAVE, true, 0, false);
- if (log_min_messages < WARNING)
+ if (log_min_messages[MyBackendType] < WARNING)
(void) set_config_option_ext("log_min_messages", "warning",
PGC_SUSET, PGC_S_SESSION,
BOOTSTRAP_SUPERUSERID,
diff --git a/src/backend/commands/variable.c b/src/backend/commands/variable.c
index 608f10d9412..5c769dd7bcc 100644
--- a/src/backend/commands/variable.c
+++ b/src/backend/commands/variable.c
@@ -35,6 +35,7 @@
#include "utils/datetime.h"
#include "utils/fmgrprotos.h"
#include "utils/guc_hooks.h"
+#include "utils/guc_tables.h"
#include "utils/snapmgr.h"
#include "utils/syscache.h"
#include "utils/timestamp.h"
@@ -1257,3 +1258,198 @@ check_ssl(bool *newval, void **extra, GucSource source)
#endif
return true;
}
+
+/*
+ * GUC check_hook for log_min_messages
+ *
+ * The parsing consists of a comma-separated list of BACKENDTYPENAME:LEVEL
+ * elements. BACKENDTYPENAME is log_min_messages_backend_types. LEVEL is
+ * server_message_level_options. A single LEVEL element should be part of this
+ * list and it is applied as a final step to the backend types that are not
+ * specified. For backward compatibility, the old syntax is still accepted and
+ * it means to apply this level for all backend types.
+ */
+bool
+check_log_min_messages(char **newval, void **extra, GucSource source)
+{
+ char *rawstring;
+ List *elemlist;
+ ListCell *l;
+ int newlevels[BACKEND_NUM_TYPES];
+ bool assigned[BACKEND_NUM_TYPES];
+ int genericlevel = -1; /* -1 means not assigned */
+
+ /* Initialize the array. */
+ memset(newlevels, WARNING, BACKEND_NUM_TYPES * sizeof(int));
+ memset(assigned, false, BACKEND_NUM_TYPES * sizeof(bool));
+
+ /* Need a modifiable copy of string. */
+ rawstring = pstrdup(*newval);
+
+ /* Parse string into list of identifiers. */
+ if (!SplitGUCList(rawstring, ',', &elemlist))
+ {
+ /* syntax error in list */
+ GUC_check_errdetail("List syntax is invalid.");
+ pfree(rawstring);
+ list_free(elemlist);
+ return false;
+ }
+
+ /* Validate and assign log level and backend type. */
+ foreach(l, elemlist)
+ {
+ char *tok = (char *) lfirst(l);
+ char *sep;
+ const struct config_enum_entry *entry;
+
+ /*
+ * Check whether there is a backend type following the log level. If
+ * there is no separator, it means this log level should be applied
+ * for all backend types (backward compatibility).
+ */
+ sep = strchr(tok, ':');
+ if (sep == NULL)
+ {
+ bool found = false;
+
+ /* Reject duplicates for generic log level. */
+ if (genericlevel != -1)
+ {
+ GUC_check_errdetail("Generic log level was already assigned.");
+ pfree(rawstring);
+ list_free(elemlist);
+ return false;
+ }
+
+ /* Is the log level valid? */
+ for (entry = server_message_level_options; entry && entry->name; entry++)
+ {
+ if (pg_strcasecmp(entry->name, tok) == 0)
+ {
+ genericlevel = entry->val;
+ found = true;
+ break;
+ }
+ }
+
+ if (!found)
+ {
+ GUC_check_errdetail("Unrecognized log level: \"%s\".", tok);
+ pfree(rawstring);
+ list_free(elemlist);
+ return false;
+ }
+ }
+ else
+ {
+ char *loglevel;
+ char *btype;
+ bool found = false;
+
+ btype = palloc((sep - tok) + 1);
+ memcpy(btype, tok, sep - tok);
+ btype[sep - tok] = '\0';
+ loglevel = pstrdup(sep + 1);
+
+ /* Is the log level valid? */
+ for (entry = server_message_level_options; entry && entry->name; entry++)
+ {
+ if (pg_strcasecmp(entry->name, loglevel) == 0)
+ {
+ found = true;
+ break;
+ }
+ }
+
+ if (!found)
+ {
+ GUC_check_errdetail("Unrecognized log level: \"%s\".", loglevel);
+ pfree(rawstring);
+ list_free(elemlist);
+ return false;
+ }
+
+ /*
+ * Is the backend type name valid? There might be multiple entries
+ * per backend type, don't bail out when find first occurrence.
+ */
+ found = false;
+ for (int i = 0; i < BACKEND_NUM_TYPES; i++)
+ {
+ if (pg_strcasecmp(log_min_messages_backend_types[i], btype) == 0)
+ {
+ /* Reject duplicates for a backend type. */
+ if (assigned[i])
+ {
+ GUC_check_errdetail("Backend type \"%s\" was already assigned.", btype);
+ pfree(rawstring);
+ list_free(elemlist);
+ return false;
+ }
+
+ newlevels[i] = entry->val;
+ assigned[i] = true;
+ found = true;
+ }
+ }
+
+ if (!found)
+ {
+ GUC_check_errdetail("Unrecognized backend type: \"%s\".", btype);
+ pfree(rawstring);
+ list_free(elemlist);
+ return false;
+ }
+ }
+ }
+
+ /*
+ * Generic log level must be specified. It is a good idea to specify a
+ * generic log level to make it clear that it is the fallback value.
+ * Although, we can document it, it might confuse users that used to
+ * specify a single log level in prior releases.
+ */
+ if (genericlevel == -1)
+ {
+ GUC_check_errdetail("Generic log level was not defined.");
+ pfree(rawstring);
+ list_free(elemlist);
+ return false;
+ }
+
+ /*
+ * Apply the generic log level (the one without a backend type) after all
+ * of the specific backend type have been assigned. Hence, it doesn't
+ * matter the order you specify the generic log level, the final result
+ * will be the same.
+ */
+ for (int i = 0; i < BACKEND_NUM_TYPES; i++)
+ {
+ if (!assigned[i])
+ newlevels[i] = genericlevel;
+ }
+
+ pfree(rawstring);
+ list_free(elemlist);
+
+ /*
+ * Pass back data for assign_log_min_messages to use.
+ */
+ *extra = guc_malloc(LOG, BACKEND_NUM_TYPES * sizeof(int));
+ if (!*extra)
+ return false;
+ memcpy(*extra, newlevels, BACKEND_NUM_TYPES * sizeof(int));
+
+ return true;
+}
+
+/*
+ * GUC assign_hook for log_min_messages
+ */
+void
+assign_log_min_messages(const char *newval, void *extra)
+{
+ for (int i = 0; i < BACKEND_NUM_TYPES; i++)
+ log_min_messages[i] = ((int *) extra)[i];
+}
diff --git a/src/backend/postmaster/launch_backend.c b/src/backend/postmaster/launch_backend.c
index 8b2f1a0cf41..08395dafe04 100644
--- a/src/backend/postmaster/launch_backend.c
+++ b/src/backend/postmaster/launch_backend.c
@@ -177,7 +177,7 @@ typedef struct
} child_process_kind;
static child_process_kind child_process_kinds[] = {
-#define PG_PROCTYPE(bktype, description, main_func, shmem_attach) \
+#define PG_PROCTYPE(bktype, bkcategory, description, main_func, shmem_attach, log_min_messages) \
[bktype] = {description, main_func, shmem_attach},
#include "postmaster/proctypelist.h"
#undef PG_PROCTYPE
diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c
index 47af743990f..77cafe6e7b5 100644
--- a/src/backend/utils/error/elog.c
+++ b/src/backend/utils/error/elog.c
@@ -235,7 +235,7 @@ is_log_level_output(int elevel, int log_min_level)
static inline bool
should_output_to_server(int elevel)
{
- return is_log_level_output(elevel, log_min_messages);
+ return is_log_level_output(elevel, log_min_messages[MyBackendType]);
}
/*
diff --git a/src/backend/utils/init/miscinit.c b/src/backend/utils/init/miscinit.c
index dec220a61f5..8a919148fd7 100644
--- a/src/backend/utils/init/miscinit.c
+++ b/src/backend/utils/init/miscinit.c
@@ -266,7 +266,7 @@ GetBackendTypeDesc(BackendType backendType)
switch (backendType)
{
-#define PG_PROCTYPE(bktype, description, main_func, shmem_attach) \
+#define PG_PROCTYPE(bktype, bkcategory, description, main_func, shmem_attach, log_min_messages) \
case bktype: backendDesc = gettext_noop(description); break;
#include "postmaster/proctypelist.h"
#undef PG_PROCTYPE
diff --git a/src/backend/utils/misc/guc_tables.c b/src/backend/utils/misc/guc_tables.c
index d14b1678e7f..300e172aa82 100644
--- a/src/backend/utils/misc/guc_tables.c
+++ b/src/backend/utils/misc/guc_tables.c
@@ -146,7 +146,7 @@ static const struct config_enum_entry client_message_level_options[] = {
{NULL, 0, false}
};
-static const struct config_enum_entry server_message_level_options[] = {
+const struct config_enum_entry server_message_level_options[] = {
{"debug5", DEBUG5, false},
{"debug4", DEBUG4, false},
{"debug3", DEBUG3, false},
@@ -536,7 +536,6 @@ static bool default_with_oids = false;
bool current_role_is_superuser;
int log_min_error_statement = ERROR;
-int log_min_messages = WARNING;
int client_min_messages = NOTICE;
int log_min_duration_sample = -1;
int log_min_duration_statement = -1;
@@ -594,6 +593,7 @@ static char *server_version_string;
static int server_version_num;
static char *debug_io_direct_string;
static char *restrict_nonsystem_relation_kind_string;
+static char *log_min_messages_string;
#ifdef HAVE_SYSLOG
#define DEFAULT_SYSLOG_FACILITY LOG_LOCAL0
@@ -638,6 +638,27 @@ char *role_string;
/* should be static, but guc.c needs to get at this */
bool in_hot_standby_guc;
+/*
+ * It should be static, but commands/variable.c needs to get at this.
+ */
+int log_min_messages[] = {
+#define PG_PROCTYPE(bktype, bkcategory, description, main_func, shmem_attach, log_min_messages) \
+ [bktype] = log_min_messages,
+#include "postmaster/proctypelist.h"
+#undef PG_PROCTYPE
+};
+
+/*
+ * It might be in commands/variable.c but for convenience it is near
+ * log_min_messages.
+ */
+const char *const log_min_messages_backend_types[] = {
+#define PG_PROCTYPE(bktype, bkcategory, description, main_func, shmem_attach, log_min_messages) \
+ [bktype] = bkcategory,
+#include "postmaster/proctypelist.h"
+#undef PG_PROCTYPE
+};
+
/*
* Displayable names for context types (enum GucContext)
@@ -4298,6 +4319,18 @@ struct config_string ConfigureNamesString[] =
check_client_encoding, assign_client_encoding, NULL
},
+ {
+ {"log_min_messages", PGC_SUSET, LOGGING_WHEN,
+ gettext_noop("Sets the message levels that are logged."),
+ gettext_noop("Each level includes all the levels that follow it. The later"
+ " the level, the fewer messages are sent."),
+ GUC_LIST_INPUT
+ },
+ &log_min_messages_string,
+ "WARNING",
+ check_log_min_messages, assign_log_min_messages, NULL
+ },
+
{
{"log_line_prefix", PGC_SIGHUP, LOGGING_WHAT,
gettext_noop("Controls information prefixed to each log line."),
@@ -5110,17 +5143,6 @@ struct config_enum ConfigureNamesEnum[] =
NULL, NULL, NULL
},
- {
- {"log_min_messages", PGC_SUSET, LOGGING_WHEN,
- gettext_noop("Sets the message levels that are logged."),
- gettext_noop("Each level includes all the levels that follow it. The later"
- " the level, the fewer messages are sent.")
- },
- &log_min_messages,
- WARNING, server_message_level_options,
- NULL, NULL, NULL
- },
-
{
{"log_min_error_statement", PGC_SUSET, LOGGING_WHEN,
gettext_noop("Causes all statements generating error at or above this level to be logged."),
diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample
index a9d8293474a..c266effc597 100644
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -539,6 +539,7 @@
# log
# fatal
# panic
+ # or a comma-separated list of backend_type:level
#log_min_error_statement = error # values in order of decreasing detail:
# debug5
diff --git a/src/include/postmaster/proctypelist.h b/src/include/postmaster/proctypelist.h
index 919f00bb3a7..356cd45815a 100644
--- a/src/include/postmaster/proctypelist.h
+++ b/src/include/postmaster/proctypelist.h
@@ -29,22 +29,22 @@
* entries.
*/
-/* bktype, description, main_func, shmem_attach */
-PG_PROCTYPE(B_ARCHIVER, "archiver", PgArchiverMain, true)
-PG_PROCTYPE(B_AUTOVAC_LAUNCHER, "autovacuum launcher", AutoVacLauncherMain, true)
-PG_PROCTYPE(B_AUTOVAC_WORKER, "autovacuum worker", AutoVacWorkerMain, true)
-PG_PROCTYPE(B_BACKEND, "client backend", BackendMain, true)
-PG_PROCTYPE(B_BG_WORKER, "background worker", BackgroundWorkerMain, true)
-PG_PROCTYPE(B_BG_WRITER, "background writer", BackgroundWriterMain, true)
-PG_PROCTYPE(B_CHECKPOINTER, "checkpointer", CheckpointerMain, true)
-PG_PROCTYPE(B_DEAD_END_BACKEND, "dead-end client backend", BackendMain, true)
-PG_PROCTYPE(B_INVALID, "unrecognized", NULL, false)
-PG_PROCTYPE(B_IO_WORKER, "io worker", IoWorkerMain, true)
-PG_PROCTYPE(B_LOGGER, "syslogger", SysLoggerMain, false)
-PG_PROCTYPE(B_SLOTSYNC_WORKER, "slotsync worker", ReplSlotSyncWorkerMain, true)
-PG_PROCTYPE(B_STANDALONE_BACKEND, "standalone backend", NULL, false)
-PG_PROCTYPE(B_STARTUP, "startup", StartupProcessMain, true)
-PG_PROCTYPE(B_WAL_RECEIVER, "walreceiver", WalReceiverMain, true)
-PG_PROCTYPE(B_WAL_SENDER, "walsender", NULL, true)
-PG_PROCTYPE(B_WAL_SUMMARIZER, "walsummarizer", WalSummarizerMain, true)
-PG_PROCTYPE(B_WAL_WRITER, "walwriter", WalWriterMain, true)
+/* bktype, bkcategory, description, main_func, shmem_attach, log_min_messages */
+PG_PROCTYPE(B_ARCHIVER, "archiver", "archiver", PgArchiverMain, true, WARNING)
+PG_PROCTYPE(B_AUTOVAC_LAUNCHER, "autovacuum", "autovacuum launcher", AutoVacLauncherMain, true, WARNING)
+PG_PROCTYPE(B_AUTOVAC_WORKER, "autovacuum", "autovacuum worker", AutoVacWorkerMain, true, WARNING)
+PG_PROCTYPE(B_BACKEND, "backend", "client backend", BackendMain, true, WARNING)
+PG_PROCTYPE(B_BG_WORKER, "bgworker", "background worker", BackgroundWorkerMain, true, WARNING)
+PG_PROCTYPE(B_BG_WRITER, "bgwriter", "background writer", BackgroundWriterMain, true, WARNING)
+PG_PROCTYPE(B_CHECKPOINTER, "checkpointer", "checkpointer", CheckpointerMain, true, WARNING)
+PG_PROCTYPE(B_DEAD_END_BACKEND, "backend", "dead-end client backend", BackendMain, true, WARNING)
+PG_PROCTYPE(B_INVALID, "backend", "invalid", NULL, false, WARNING)
+PG_PROCTYPE(B_IO_WORKER, "ioworker", "io worker", IoWorkerMain, true, WARNING)
+PG_PROCTYPE(B_LOGGER, "logger", "syslogger", SysLoggerMain, false, WARNING)
+PG_PROCTYPE(B_SLOTSYNC_WORKER, "slotsyncworker", "slotsync worker", ReplSlotSyncWorkerMain, true, WARNING)
+PG_PROCTYPE(B_STANDALONE_BACKEND, "backend", "standalone backend", NULL, false, WARNING)
+PG_PROCTYPE(B_STARTUP, "startup", "startup", StartupProcessMain, true, WARNING)
+PG_PROCTYPE(B_WAL_RECEIVER, "walreceiver", "walreceiver", WalReceiverMain, true, WARNING)
+PG_PROCTYPE(B_WAL_SENDER, "walsender", "walsender", NULL, true, WARNING)
+PG_PROCTYPE(B_WAL_SUMMARIZER, "walsummarizer", "walsummarizer", WalSummarizerMain, true, WARNING)
+PG_PROCTYPE(B_WAL_WRITER, "walwriter", "walwriter", WalWriterMain, true, WARNING)
diff --git a/src/include/utils/guc.h b/src/include/utils/guc.h
index f619100467d..e10e24940a7 100644
--- a/src/include/utils/guc.h
+++ b/src/include/utils/guc.h
@@ -271,7 +271,7 @@ extern PGDLLIMPORT bool log_duration;
extern PGDLLIMPORT int log_parameter_max_length;
extern PGDLLIMPORT int log_parameter_max_length_on_error;
extern PGDLLIMPORT int log_min_error_statement;
-extern PGDLLIMPORT int log_min_messages;
+extern PGDLLIMPORT int log_min_messages[];
extern PGDLLIMPORT int client_min_messages;
extern PGDLLIMPORT int log_min_duration_sample;
extern PGDLLIMPORT int log_min_duration_statement;
diff --git a/src/include/utils/guc_hooks.h b/src/include/utils/guc_hooks.h
index 82ac8646a8d..e6ffc96468f 100644
--- a/src/include/utils/guc_hooks.h
+++ b/src/include/utils/guc_hooks.h
@@ -174,5 +174,7 @@ extern void assign_wal_sync_method(int new_wal_sync_method, void *extra);
extern bool check_synchronized_standby_slots(char **newval, void **extra,
GucSource source);
extern void assign_synchronized_standby_slots(const char *newval, void *extra);
+extern bool check_log_min_messages(char **newval, void **extra, GucSource source);
+extern void assign_log_min_messages(const char *newval, void *extra);
#endif /* GUC_HOOKS_H */
diff --git a/src/include/utils/guc_tables.h b/src/include/utils/guc_tables.h
index f72ce944d7f..2aefc653f20 100644
--- a/src/include/utils/guc_tables.h
+++ b/src/include/utils/guc_tables.h
@@ -298,6 +298,10 @@ struct config_enum
void *reset_extra;
};
+/* log_min_messages */
+extern PGDLLIMPORT const char *const log_min_messages_backend_types[];
+extern PGDLLIMPORT const struct config_enum_entry server_message_level_options[];
+
/* constant tables corresponding to enums above and in guc.h */
extern PGDLLIMPORT const char *const config_group_names[];
extern PGDLLIMPORT const char *const config_type_names[];
diff --git a/src/test/regress/expected/guc.out b/src/test/regress/expected/guc.out
index 7f9e29c765c..741ad6bf062 100644
--- a/src/test/regress/expected/guc.out
+++ b/src/test/regress/expected/guc.out
@@ -913,3 +913,54 @@ SELECT name FROM tab_settings_flags
(0 rows)
DROP TABLE tab_settings_flags;
+-- Test log_min_messages
+SET log_min_messages TO fatal;
+SHOW log_min_messages;
+ log_min_messages
+------------------
+ fatal
+(1 row)
+
+SET log_min_messages TO 'fatal';
+SHOW log_min_messages;
+ log_min_messages
+------------------
+ fatal
+(1 row)
+
+SET log_min_messages TO 'checkpointer:debug2, autovacuum:debug1'; --fail
+ERROR: invalid value for parameter "log_min_messages": "checkpointer:debug2, autovacuum:debug1"
+DETAIL: Generic log level was not defined.
+SET log_min_messages TO 'debug1, backend:error, fatal'; -- fail
+ERROR: invalid value for parameter "log_min_messages": "debug1, backend:error, fatal"
+DETAIL: Generic log level was already assigned.
+SET log_min_messages TO 'backend:error, debug1, backend:warning'; -- fail
+ERROR: invalid value for parameter "log_min_messages": "backend:error, debug1, backend:warning"
+DETAIL: Backend type "backend" was already assigned.
+SET log_min_messages TO 'backend:error, foo:fatal, archiver:debug1'; -- fail
+ERROR: invalid value for parameter "log_min_messages": "backend:error, foo:fatal, archiver:debug1"
+DETAIL: Unrecognized backend type: "foo".
+SET log_min_messages TO 'backend:error, checkpointer:bar, archiver:debug1'; -- fail
+ERROR: invalid value for parameter "log_min_messages": "backend:error, checkpointer:bar, archiver:debug1"
+DETAIL: Unrecognized log level: "bar".
+SET log_min_messages TO 'backend:error, checkpointer:debug3, fatal, archiver:debug2, autovacuum:debug1, walsender:debug3';
+SHOW log_min_messages;
+ log_min_messages
+-------------------------------------------------------------------------------------------------
+ backend:error, checkpointer:debug3, fatal, archiver:debug2, autovacuum:debug1, walsender:debug3
+(1 row)
+
+SET log_min_messages TO 'warning, autovacuum:debug1';
+SHOW log_min_messages;
+ log_min_messages
+----------------------------
+ warning, autovacuum:debug1
+(1 row)
+
+SET log_min_messages TO 'autovacuum:debug1, warning';
+SHOW log_min_messages;
+ log_min_messages
+----------------------------
+ autovacuum:debug1, warning
+(1 row)
+
diff --git a/src/test/regress/sql/guc.sql b/src/test/regress/sql/guc.sql
index f65f84a2632..12fa1812f35 100644
--- a/src/test/regress/sql/guc.sql
+++ b/src/test/regress/sql/guc.sql
@@ -368,3 +368,20 @@ SELECT name FROM tab_settings_flags
WHERE no_reset AND NOT no_reset_all
ORDER BY 1;
DROP TABLE tab_settings_flags;
+
+-- Test log_min_messages
+SET log_min_messages TO fatal;
+SHOW log_min_messages;
+SET log_min_messages TO 'fatal';
+SHOW log_min_messages;
+SET log_min_messages TO 'checkpointer:debug2, autovacuum:debug1'; --fail
+SET log_min_messages TO 'debug1, backend:error, fatal'; -- fail
+SET log_min_messages TO 'backend:error, debug1, backend:warning'; -- fail
+SET log_min_messages TO 'backend:error, foo:fatal, archiver:debug1'; -- fail
+SET log_min_messages TO 'backend:error, checkpointer:bar, archiver:debug1'; -- fail
+SET log_min_messages TO 'backend:error, checkpointer:debug3, fatal, archiver:debug2, autovacuum:debug1, walsender:debug3';
+SHOW log_min_messages;
+SET log_min_messages TO 'warning, autovacuum:debug1';
+SHOW log_min_messages;
+SET log_min_messages TO 'autovacuum:debug1, warning';
+SHOW log_min_messages;
--
2.39.5
^ permalink raw reply [nested|flat] 51+ messages in thread
* Re: log_min_messages per backend type
@ 2025-07-31 16:22 Japin Li <[email protected]>
parent: Euler Taveira <[email protected]>
1 sibling, 1 reply; 51+ messages in thread
From: Japin Li @ 2025-07-31 16:22 UTC (permalink / raw)
To: Euler Taveira <[email protected]>; +Cc: Andres Freund <[email protected]>; Alvaro Herrera <[email protected]>; [email protected]
On Thu, 31 Jul 2025 at 11:19, "Euler Taveira" <[email protected]> wrote:
> On Thu, Mar 6, 2025, at 10:33 AM, Andres Freund wrote:
>> Huh, the startup process is among the most crucial things to monitor?
>>
>
> Good point. Fixed.
>
> After collecting some suggestions, I'm attaching a new patch contains the
> following changes:
>
> - patch was rebased
> - include Alvaro's patch (v2-0001) [1] as a basis for this patch
> - add ioworker as new backend type
> - add startup as new backend type per Andres suggestion
> - small changes into documentation
>
>> I don't know what I think about the whole patch, but I do want to voice
>> *strong* opposition to duplicating a list of all backend types into multiple
>> places. It's already painfull enough to add a new backend type, without having
>> to pointlessly go around and manually add a new backend type to mulltiple
>> arrays that have completely predictable content.
>>
>
> I'm including Alvaro's patch as is just to make the CF bot happy and to
> illustrate how it would be if we adopt his solution to centralize the list of
> backend types. I think Alvaro's proposal overcomes the objection [2], right?
>
If we set the log level for all backend types, I don't think a generic log
level is necessary.
--
Regards,
Japin Li
ChengDu WenWu Information Technology Co., Ltd.
^ permalink raw reply [nested|flat] 51+ messages in thread
* Re: log_min_messages per backend type
@ 2025-07-31 16:51 Alvaro Herrera <[email protected]>
parent: Japin Li <[email protected]>
0 siblings, 1 reply; 51+ messages in thread
From: Alvaro Herrera @ 2025-07-31 16:51 UTC (permalink / raw)
To: Japin Li <[email protected]>; +Cc: Euler Taveira <[email protected]>; Andres Freund <[email protected]>; [email protected]
On 2025-Aug-01, Japin Li wrote:
> If we set the log level for all backend types, I don't think a generic log
> level is necessary.
I don't understand what you mean by this. Can you elaborate?
--
Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/
"If it is not right, do not do it.
If it is not true, do not say it." (Marcus Aurelius, Meditations)
^ permalink raw reply [nested|flat] 51+ messages in thread
* Re: log_min_messages per backend type
@ 2025-07-31 23:34 Japin Li <[email protected]>
parent: Alvaro Herrera <[email protected]>
0 siblings, 0 replies; 51+ messages in thread
From: Japin Li @ 2025-07-31 23:34 UTC (permalink / raw)
To: Alvaro Herrera <[email protected]>; +Cc: Euler Taveira <[email protected]>; Andres Freund <[email protected]>; [email protected]
On Thu, 31 Jul 2025 at 18:51, Alvaro Herrera <[email protected]> wrote:
> On 2025-Aug-01, Japin Li wrote:
>
>> If we set the log level for all backend types, I don't think a generic log
>> level is necessary.
>
> I don't understand what you mean by this. Can you elaborate?
>
For example:
ALTER SYSTEM SET log_min_messages TO
'archiver:info, autovacuum:info, backend:info, bgworker:info, bgwriter:info, checkpointer:info, ioworker:info, logger:info, slotsyncworker:info, startup:info, walreceiver:info, walsender:info, walsummarizer:info, walwriter:info';
Given that we've set a log level for every backend type and
assigned[BACKEND_NUM_TYPES] is true, a generic level seems redundant.
--
Regards,
Japin Li
ChengDu WenWu Information Technology Co., Ltd.
^ permalink raw reply [nested|flat] 51+ messages in thread
* Re: log_min_messages per backend type
@ 2025-08-01 05:53 Japin Li <[email protected]>
parent: Euler Taveira <[email protected]>
1 sibling, 0 replies; 51+ messages in thread
From: Japin Li @ 2025-08-01 05:53 UTC (permalink / raw)
To: Euler Taveira <[email protected]>; +Cc: Andres Freund <[email protected]>; Alvaro Herrera <[email protected]>; [email protected]
On Thu, Jul 31, 2025 at 11:19:48AM -0300, Euler Taveira wrote:
> On Thu, Mar 6, 2025, at 10:33 AM, Andres Freund wrote:
> > Huh, the startup process is among the most crucial things to monitor?
> >
>
> Good point. Fixed.
>
> After collecting some suggestions, I'm attaching a new patch contains the
> following changes:
>
> - patch was rebased
> - include Alvaro's patch (v2-0001) [1] as a basis for this patch
> - add ioworker as new backend type
> - add startup as new backend type per Andres suggestion
> - small changes into documentation
>
> > I don't know what I think about the whole patch, but I do want to voice
> > *strong* opposition to duplicating a list of all backend types into multiple
> > places. It's already painfull enough to add a new backend type, without having
> > to pointlessly go around and manually add a new backend type to mulltiple
> > arrays that have completely predictable content.
> >
>
> I'm including Alvaro's patch as is just to make the CF bot happy and to
> illustrate how it would be if we adopt his solution to centralize the list of
> backend types. I think Alvaro's proposal overcomes the objection [2], right?
>
I think we can avoid memory allocation by using pg_strncasecmp().
diff --git a/src/backend/commands/variable.c b/src/backend/commands/variable.c
index 5c769dd7bcc..f854b2fac93 100644
--- a/src/backend/commands/variable.c
+++ b/src/backend/commands/variable.c
@@ -1343,14 +1343,10 @@ check_log_min_messages(char **newval, void **extra, GucSource source)
}
else
{
- char *loglevel;
- char *btype;
- bool found = false;
- btype = palloc((sep - tok) + 1);
- memcpy(btype, tok, sep - tok);
- btype[sep - tok] = '\0';
- loglevel = pstrdup(sep + 1);
+ char *btype = tok;
+ char *loglevel = sep + 1;
+ bool found = false;
/* Is the log level valid? */
for (entry = server_message_level_options; entry && entry->name; entry++)
@@ -1377,7 +1373,7 @@ check_log_min_messages(char **newval, void **extra, GucSource source)
found = false;
for (int i = 0; i < BACKEND_NUM_TYPES; i++)
{
- if (pg_strcasecmp(log_min_messages_backend_types[i], btype) == 0)
+ if (pg_strncasecmp(log_min_messages_backend_types[i], btype, sep - tok) == 0)
{
/* Reject duplicates for a backend type. */
if (assigned[i])
--
Best regards,
Japin Li
ChengDu WenWu Information Technology Co., LTD.
^ permalink raw reply [nested|flat] 51+ messages in thread
end of thread, other threads:[~2025-08-01 05:53 UTC | newest]
Thread overview: 51+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2017-12-06 16:40 Postgres with pthread Konstantin Knizhnik <[email protected]>
2017-12-06 16:53 ` Tom Lane <[email protected]>
2017-12-06 17:02 ` Adam Brusselback <[email protected]>
2017-12-06 17:13 ` Robert Haas <[email protected]>
2017-12-06 17:17 ` Andres Freund <[email protected]>
2017-12-06 17:24 ` Adam Brusselback <[email protected]>
2017-12-07 03:20 ` Craig Ringer <[email protected]>
2017-12-07 03:44 ` Tsunakawa, Takayuki <[email protected]>
2017-12-07 04:45 ` Craig Ringer <[email protected]>
2017-12-07 17:52 ` Robert Haas <[email protected]>
2017-12-06 17:08 ` Andres Freund <[email protected]>
2017-12-06 17:26 ` Andreas Karlsson <[email protected]>
2017-12-10 16:24 ` james <[email protected]>
2017-12-06 17:28 ` Robert Haas <[email protected]>
2017-12-06 17:42 ` Andres Freund <[email protected]>
2017-12-06 21:58 ` Thomas Munro <[email protected]>
2017-12-07 03:26 ` Craig Ringer <[email protected]>
2017-12-07 19:58 ` Andres Freund <[email protected]>
2017-12-07 20:48 ` Greg Stark <[email protected]>
2017-12-07 20:52 ` Andres Freund <[email protected]>
2017-12-08 01:14 ` Craig Ringer <[email protected]>
2017-12-07 12:13 ` Konstantin Knizhnik <[email protected]>
2017-12-07 12:06 ` Konstantin Knizhnik <[email protected]>
2017-12-07 07:41 ` Simon Riggs <[email protected]>
2017-12-07 11:55 ` Konstantin Knizhnik <[email protected]>
2017-12-07 14:56 ` Craig Ringer <[email protected]>
2017-12-21 13:25 ` Konstantin Knizhnik <[email protected]>
2017-12-21 13:46 ` Pavel Stehule <[email protected]>
2017-12-27 08:34 ` Konstantin Knizhnik <[email protected]>
2017-12-27 10:05 ` james <[email protected]>
2017-12-27 10:08 ` Andres Freund <[email protected]>
2017-12-27 11:13 ` james <[email protected]>
2017-12-27 11:17 ` Konstantin Knizhnik <[email protected]>
2017-12-08 22:09 ` konstantin knizhnik <[email protected]>
2017-12-08 22:28 ` Alexander Korotkov <[email protected]>
2021-03-07 00:35 [PATCH v6 2/2] Move pg_upgrade kludges to sql script Justin Pryzby <[email protected]>
2021-03-07 00:35 [PATCH v5 4/4] Move pg_upgrade kludges to sql script Justin Pryzby <[email protected]>
2021-03-07 00:35 [PATCH v7 1/2] Move pg_upgrade kludges to sql script Justin Pryzby <[email protected]>
2021-03-07 00:35 [PATCH v5 4/4] Move pg_upgrade kludges to sql script Justin Pryzby <[email protected]>
2025-02-05 18:51 Re: log_min_messages per backend type Álvaro Herrera <[email protected]>
2025-03-05 00:33 ` Re: log_min_messages per backend type Euler Taveira <[email protected]>
2025-03-05 16:40 ` Re: log_min_messages per backend type Andrew Dunstan <[email protected]>
2025-03-06 13:20 ` Re: log_min_messages per backend type Matheus Alcantara <[email protected]>
2025-03-06 21:33 ` Re: log_min_messages per backend type Euler Taveira <[email protected]>
2025-03-06 02:53 ` Re: log_min_messages per backend type Fujii Masao <[email protected]>
2025-03-06 13:33 ` Re: log_min_messages per backend type Andres Freund <[email protected]>
2025-07-31 14:19 ` Re: log_min_messages per backend type Euler Taveira <[email protected]>
2025-07-31 16:22 ` Re: log_min_messages per backend type Japin Li <[email protected]>
2025-07-31 16:51 ` Re: log_min_messages per backend type Alvaro Herrera <[email protected]>
2025-07-31 23:34 ` Re: log_min_messages per backend type Japin Li <[email protected]>
2025-08-01 05:53 ` Re: log_min_messages per backend type Japin Li <[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