Re: Stack overflow issue

Поиск
Список
Период
Сортировка
От Alexander Korotkov
Тема Re: Stack overflow issue
Дата
Msg-id CAPpHfduZqAjF+7rDRP-RGNHjOXy7nvFROQ0MGS436f8FPY5DpQ@mail.gmail.com
обсуждение исходный текст
Ответ на Re: Stack overflow issue  (Robert Haas <robertmhaas@gmail.com>)
Ответы Re: Stack overflow issue
Список pgsql-hackers
Hi!

On Fri, Jan 12, 2024 at 11:00 PM Robert Haas <robertmhaas@gmail.com> wrote:
> On Fri, Jan 12, 2024 at 10:12 AM Heikki Linnakangas <hlinnaka@iki.fi> wrote:
> > Here's one goto-free attempt. It adds a local loop to where the
> > recursion was, so that if you have a chain of subtransactions that need
> > to be aborted in CommitTransactionCommand, they are aborted iteratively.
> > The TBLOCK_SUBCOMMIT case already had such a loop.
> >
> > I added a couple of comments in the patch marked with "REVIEWER NOTE",
> > to explain why I changed some things. They are to be removed before
> > committing.
> >
> > I'm not sure if this is better than a goto. In fact, even if we commit
> > this, I think I'd still prefer to replace the remaining recursive calls
> > with a goto. Recursion feels a weird to me, when we're unwinding the
> > states from the stack as we go.
>
> I'm not able to quickly verify whether this version is correct, but I
> do think the code looks nicer this way.
>
> I understand that's a question of opinion rather than fact, though.

I'd like to revive this thread.  The attached 0001 patch represents my
attempt to remove recursion in
CommitTransactionCommand()/AbortCurrentTransaction() by adding a
wrapper function.  This method doesn't use goto, doesn't require much
code changes and subjectively provides good readability.

Regarding ShowTransactionStateRec() and memory context function, as I
get from this thread they are called in states where abortion can lead
to a panic.  So, it's preferable to change them into loops too rather
than just adding check_stack_depth().  The 0002 and 0003 patches by
Heikki posted in [1] look good to me.  Can we accept them?

Also there are a number of recursive functions, which seem to be not
used in critical states where abortion can lead to a panic.  I've
extracted them from [2] into an attached 0002 patch.  I'd like to push
it if there is no objection.

Links.
1. https://www.postgresql.org/message-id/6b48c746-9704-46dc-b9be-01fe4137c824%40iki.fi
2. https://www.postgresql.org/message-id/4530546a-3216-eaa9-4c92-92d33290a211%40mail.ru

------
Regards,
Alexander Korotkov

Вложения

В списке pgsql-hackers по дате отправления:

Предыдущее
От: Amit Kapila
Дата:
Сообщение: Re: About a recently-added message
Следующее
От: Tomas Vondra
Дата:
Сообщение: Re: index prefetching