[BUG] Crash on pgbench initialization.

Started by Anton A. Melnikovover 2 years ago7 messages
#1Anton A. Melnikov
aamelnikov@inbox.ru
2 attachment(s)

Hello!

My colleague Victoria Shepard reported that pgbench might crash
during initialization with some values of shared_buffers and
max_worker_processes in conf.

After some research, found this happens when the LimitAdditionalPins() returns exactly zero.
In the current master, this will happen e.g. if shared_buffers = 10MB and max_worker_processes = 40.
Then the command "pgbench --initialize postgres" will lead to crash.
See the backtrace attached.

There is a fix in the patch applied. Please take a look on it.

With the best regards,

--
Anton A. Melnikov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

Attachments:

bt.txttext/plain; charset=UTF-8; name=bt.txtDownload
v1-0001-nonzero-from-LimitAdditionalPins.patchtext/x-patch; charset=UTF-8; name=v1-0001-nonzero-from-LimitAdditionalPins.patchDownload
commit 9680dfad5a495c8ea1ee7db714fcec715bfa0fed
Author: Anton A. Melnikov <a.melnikov@postgrespro.ru>
Date:   Sun Jul 23 22:24:03 2023 +0300

    Don't allow LimitAdditionalPins() to return zero.
    
    Reported-by Victoria Shepard.

diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c
index a7e3b9bb1d..df22aaa1c5 100644
--- a/src/backend/storage/buffer/bufmgr.c
+++ b/src/backend/storage/buffer/bufmgr.c
@@ -1767,7 +1767,7 @@ LimitAdditionalPins(uint32 *additional_pins)
 	 */
 	max_proportional_pins -= PrivateRefCountOverflowed + REFCOUNT_ARRAY_ENTRIES;
 
-	if (max_proportional_pins < 0)
+	if (max_proportional_pins <= 0)
 		max_proportional_pins = 1;
 
 	if (*additional_pins > max_proportional_pins)
#2Michael Paquier
michael@paquier.xyz
In reply to: Anton A. Melnikov (#1)
Re: [BUG] Crash on pgbench initialization.

On Sun, Jul 23, 2023 at 11:21:47PM +0300, Anton A. Melnikov wrote:

After some research, found this happens when the LimitAdditionalPins() returns exactly zero.
In the current master, this will happen e.g. if shared_buffers = 10MB and max_worker_processes = 40.
Then the command "pgbench --initialize postgres" will lead to crash.
See the backtrace attached.

There is a fix in the patch applied. Please take a look on it.

Nice catch, issue reproduced here so I am adding an open item for now.
(I have not looked at the patch, yet.)
--
Michael

#3Alvaro Herrera
alvherre@alvh.no-ip.org
In reply to: Michael Paquier (#2)
Re: [BUG] Crash on pgbench initialization.

On 2023-Jul-24, Michael Paquier wrote:

On Sun, Jul 23, 2023 at 11:21:47PM +0300, Anton A. Melnikov wrote:

After some research, found this happens when the LimitAdditionalPins() returns exactly zero.
In the current master, this will happen e.g. if shared_buffers = 10MB and max_worker_processes = 40.
Then the command "pgbench --initialize postgres" will lead to crash.
See the backtrace attached.

There is a fix in the patch applied. Please take a look on it.

Nice catch, issue reproduced here so I am adding an open item for now.
(I have not looked at the patch, yet.)

Hmm, I see that all this code was added by 31966b151e6a, which makes
this Andres' item. I see Michael marked it as such in the open items
page, but did not CC Andres, so I'm doing that here now.

I don't know this code at all, but I hope that this can be solved with
just Anton's proposed patch.

--
Álvaro Herrera Breisgau, Deutschland — https://www.EnterpriseDB.com/
"Find a bug in a program, and fix it, and the program will work today.
Show the program how to find and fix a bug, and the program
will work forever" (Oliver Silfridge)

#4Andres Freund
andres@anarazel.de
In reply to: Alvaro Herrera (#3)
Re: [BUG] Crash on pgbench initialization.

Hi,

On 2023-07-24 15:54:33 +0200, Alvaro Herrera wrote:

On 2023-Jul-24, Michael Paquier wrote:

On Sun, Jul 23, 2023 at 11:21:47PM +0300, Anton A. Melnikov wrote:

After some research, found this happens when the LimitAdditionalPins() returns exactly zero.
In the current master, this will happen e.g. if shared_buffers = 10MB and max_worker_processes = 40.
Then the command "pgbench --initialize postgres" will lead to crash.
See the backtrace attached.

There is a fix in the patch applied. Please take a look on it.

Nice catch, issue reproduced here so I am adding an open item for now.
(I have not looked at the patch, yet.)

Hmm, I see that all this code was added by 31966b151e6a, which makes
this Andres' item. I see Michael marked it as such in the open items
page, but did not CC Andres, so I'm doing that here now.

Thanks - I had indeed not seen this. I can't really keep up with the list at
all times...

I don't know this code at all, but I hope that this can be solved with
just Anton's proposed patch.

Yes, it's just that off-by-one. I need to check if there's a similar bug for
local / temp table buffers though.

Greetings,

Andres Freund

#5Michael Paquier
michael@paquier.xyz
In reply to: Alvaro Herrera (#3)
Re: [BUG] Crash on pgbench initialization.

On Mon, Jul 24, 2023 at 03:54:33PM +0200, Alvaro Herrera wrote:

I see Michael marked it as such in the open items
page, but did not CC Andres, so I'm doing that here now.

Indeed, thanks!
--
Michael

#6Andres Freund
andres@anarazel.de
In reply to: Andres Freund (#4)
Re: [BUG] Crash on pgbench initialization.

Hi,

On 2023-07-24 09:42:44 -0700, Andres Freund wrote:

I don't know this code at all, but I hope that this can be solved with
just Anton's proposed patch.

Yes, it's just that off-by-one. I need to check if there's a similar bug for
local / temp table buffers though.

Doesn't appear that way. We *do* fail if there's only 1 remaining buffer, but
we already did before my change (because we also need to pin the fsm). I don't
think that's an issue worth worrying about, if all-1 of local buffers are
pinned, you're going to have problems.

Thanks Anton / Victoria for the report and fix. Pushed.

Greetings,

Andres Freund

#7Anton A. Melnikov
aamelnikov@inbox.ru
In reply to: Andres Freund (#6)
Re: [BUG] Crash on pgbench initialization.

On 25.07.2023 06:24, Andres Freund wrote:

Thanks Anton / Victoria for the report and fix. Pushed.

Thanks!
Have a nice day!

--
Anton A. Melnikov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company