Re: Introduce "log_connection_stages" setting.

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Jacob Champion <jchampion(at)timescale(dot)com>
Cc: Sergey Dudoladov <sergey(dot)dudoladov(at)gmail(dot)com>, Justin Pryzby <pryzby(at)telsasoft(dot)com>, Euler Taveira <euler(at)eulerto(dot)com>, pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: Introduce "log_connection_stages" setting.
Date: 2023-03-02 22:56:14
Message-ID: 2309020.1677797774@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Jacob Champion <jchampion(at)timescale(dot)com> writes:
> This is looking very good. One bigger comment:

>> + myextra = (int *) guc_malloc(ERROR, sizeof(int));
>> + *myextra = newlogconnect;

> If I've understood Tom correctly in [1], both of these guc_mallocs
> should be using a loglevel less than ERROR, to avoid forcing a
> postmaster exit when out of memory. (I used WARNING in that thread
> instead, which seemed to be acceptable.)

Actually, preferred practice is as seen in e.g. check_datestyle:

myextra = (int *) guc_malloc(LOG, 2 * sizeof(int));
if (!myextra)
return false;
myextra[0] = newDateStyle;
myextra[1] = newDateOrder;
*extra = (void *) myextra;

which gives the guc.c functions an opportunity to manage the
failure.

A quick grep shows that there are existing check functions that
did not get that memo, e.g. check_recovery_target_lsn.
We ought to clean them up.

This is, of course, not super important unless you're allocating
something quite large; the odds of going OOM in the postmaster
should be pretty small.

regards, tom lane

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Jehan-Guillaume de Rorthais 2023-03-02 22:57:21 Re: Memory leak from ExecutorState context?
Previous Message Tom Lane 2023-03-02 22:47:34 pgsql: Harden new test case against force_parallel_mode = regress.