[pgpool-general: 9264] Re: Segmentation fault during shutdown
Emond Papegaaij
emond.papegaaij at gmail.com
Fri Nov 8 23:03:56 JST 2024
Hi,
Your explanation makes sense. In the original backtrace, you can see that
the signal handler is started from memset, which is called from
pool_init_cp. The thing I'm worried about with your patch, is that this
will cause pgpool to no longer inform postgresql that it's about to close
the connection. I don't know how postgresql handles such termination
messages, but it would make sense that it frees resources on receiving a
termination message. These resources will most likely also be freed when
the tcp connection is closed, but what if for some reason that fails? That
would cause the tcp connection to become stale and keep the resources
allocated for a very long time until the tcp connection times out (probably
2 hours).
What if pool_init_cp would set a flag to indicates a successful
init. close_all_backend_connections could then check this flag and only
close the connections if this flag is set. I don't know if is actually
possible, because I'm a Java developer and things work quite different
there. But IMHO, it makes sense to at least try to close the connections
from the side of pgpool if this is possible.
Best regards,
Emond
Op vr 8 nov 2024 om 12:23 schreef Tatsuo Ishii <ishii at postgresql.org>:
> > Hi Emond,
> > Thank you for the report. I will look into this.
> > Best reagards,
> > --
> > Tatsuo Ishii
> > SRA OSS K.K.
> > English: http://www.sraoss.co.jp/index_en/
> > Japanese:http://www.sraoss.co.jp
> >> Hi,
> >> Unfortunately, it seems the patch did not fix this issue. Yesterday we
> had
> >> a segmentation fault at this same point again. The top of the backtrace
> now
> >> is:
> >> #0 close_all_backend_connections () at
> protocol/pool_connection_pool.c:1082
> >> #1 0x0000563a51f9280f in proc_exit_prepare (code=-1) at
> >> ../../src/utils/error/elog.c:2707
> >> #2 0x00007f0926782da7 in __funcs_on_exit () at src/exit/atexit.c:34
> >> #3 0x00007f092677a08f in exit (code=code at entry=0) at
> src/exit/exit.c:29
> >> #4 0x0000563a51f4e4e2 in child_exit (code=0) at protocol/child.c:1378
> >> #5 die (sig=3) at protocol/child.c:1174
> >> #6 <signal handler called>
> >> As you can see, it now crashes at line 1082 in pool_connection_pool.c,
> >> which looks like this in our patched version:
> >> 1074 for (i = 0; i < pool_config->max_pool; i++, p++)
> >> 1075 {
> >> 1076 int backend_id = in_use_backend_id(p);
> >> 1077
> >> 1078 if (backend_id < 0)
> >> 1079 continue;
> >> 1080 if (CONNECTION_SLOT(p, backend_id) == NULL)
> >> 1081 continue;
> >> 1082 if (CONNECTION_SLOT(p, backend_id)->sp == NULL)
> >> 1083 continue;
> >> 1084 if (CONNECTION_SLOT(p, backend_id)->sp->user == NULL)
> >> 1085 continue;
> >> 1086 pool_send_frontend_exits(p);
> >> 1087 }
> >> At the moment of the crash, a lot is happening at the same time. We are
> >> reducing a cluster back to a single node. The crash happens at the very
> >> last moment, when only the final remaining node is still up and running,
> >> but it still is running with cluster configuration (with a watchdog and
> 2
> >> backends, the local one up, the remote one down). Our configuration
> >> management then restarts the database (to force a configuration change
> on
> >> postgresql). Looking at the logs, this shutdown is noticed by pgpool,
> but
> >> the watchdog does not hold a quorum, so it cannot initiate a failover
> >> (also, there's no backend to failover to). Then, within a second, pgpool
> >> itself is also shutdown. This is when the process segfaults. Something
> that
> >> does seem interesting is that the pid (183) that segfaults, seems to be
> >> started during the failover process. pgpool is simultaneously killing
> all
> >> connection pids and starting this one. Also, this pid is killed within a
> >> single ms of being started (see timestamp 2024-11-07T00:48:06.906935
> >> and 2024-11-07T00:48:06.907304 in the logs). I hope this helps in
> tracking
> >> this issue down.
> The crash is inside close_all_backend_connections(), which is called
> when pgpool child process is about to exit. The function should never
> crash if the connection pool has been already initialized. The
> initialization is done in pool_init_cp(). The function is called at
> the very early stage of pgpool child process starts up. It allocates
> memory by using palloc then clear the memory using memset().
> >> connection pids and starting this one. Also, this pid is killed within a
> >> single ms of being started (see timestamp 2024-11-07T00:48:06.906935
> >> and 2024-11-07T00:48:06.907304 in the logs). I hope this helps in
> tracking
> So my guess is, the process (pid 183) was killed in the middle of
> memset() and the connection pool object was left with uninitialized
> data.
> The purpose of close_all_backend_connections() is sending an 'X'
> (terminate) message to backend telling that pgpool is about to
> disconnect. This is a polite way from frontend/backend protocol's
> point of view, but it's hard to make it work in the situation you
> reported because we need to establish a way to not call
> close_all_backend_connections() if the initialization is not
> done. Alternative way is, to not call close_all_backend_connections()
> at all at the process exit. This will solve the problem in simple and
> reliable way. Disconnecting connection to backend without sending 'X'
> message is not prohibited by the protocol, I believe. Also as far as I
> know PostgreSQL can handle the situation (disconnected without a prior
> 'X' message from frontend).
> Patch attached.
> Best reagards,
> --
> Tatsuo Ishii
> SRA OSS K.K.
> English: http://www.sraoss.co.jp/index_en/
> Japanese:http://www.sraoss.co.jp
> Mime-Version: 1.0
> Content-Type: Text/X-Patch; charset=us-ascii
> Content-Transfer-Encoding: 7bit
> Content-Disposition: inline; filename="fix_segfault.patch"
>
> diff --git a/src/protocol/child.c b/src/protocol/child.c
> index c12a5a2c1..4e2a3443b 100644
> --- a/src/protocol/child.c
> +++ b/src/protocol/child.c
> @@ -1350,9 +1350,12 @@ child_will_go_down(int code, Datum arg)
> memcached_disconnect();
> }
>
> - /* let backend know now we are exiting */
> - if (pool_connection_pool)
> - close_all_backend_connections();
> + /*
> + * We used to call close_all_backend_connections() here so that we
> send
> + * 'X' (terminate) message to backend. However it was possible
> that the
> + * function is called while initializing the connection pool
> object, which
> + * leads to crash. So we stopped to call
> close_all_backend_connections().
> + */
> }
>
> /*
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.pgpool.net/pipermail/pgpool-general/attachments/20241108/db74fd8b/attachment.htm>
More information about the pgpool-general
mailing list