change on_exit_nicely_list array to the dynamic array to increase slots at run time for pg_restore

Mahendra Singh Thalor <mahi6run@gmail.com> writes:

While doing some code changes with pg_dumpall and pg_rsetore[1], we noticed
that on_exit_nicely_list array has only fixed slots (MAX_ON_EXIT_NICELY=20)
but in some cases, we need more slots for pg_restore.

*Ex: restore more than 20 databases by single pg_restore command.*
We are working on a patch[1] which dumps all the databases in non-text mode
by pg_dumpall and then we are restoring those dumps by pg_restore. So we
need more slots due to multiple databases.

I can't help thinking that this means you're doing it wrong.

That on_exit infrastructure is quite rickety --- see the comments for
exit_nicely() itself --- so I don't think I trust the idea of
dynamically adding callbacks. In the present usage there is actually
only *one* callback and it's registered very near the start of either
pg_dump or pg_restore. That avoids the platform-dependent hazards
mentioned in the comment, but adding more callbacks later might not.

I think you'd be better off tracking the cleanup work you have to do
in some other place.

regards, tom lane

On Mon, Mar 10, 2025 at 2:24 PM Mahendra Singh Thalor
<mahi6run@gmail.com> wrote:

Hi,
(refer file src/bin/pg_dump/pg_backup_utils.c)

While doing some code changes with pg_dumpall and pg_rsetore[1], we noticed that on_exit_nicely_list array has only fixed slots (MAX_ON_EXIT_NICELY=20) but in some cases, we need more slots for pg_restore.
Ex: restore more than 20 databases by single pg_restore command.

We are working on a patch[1] which dumps all the databases in non-text mode by pg_dumpall and then we are restoring those dumps by pg_restore. So we need more slots due to multiple databases.

Apart from the attached patch solution, we thought of some more solutions.
Solution 1: reset array index with each database restore, but this might break some other things.
Solution 2: for each database, we can keep the index of the on_exit_nicely_list array and after restoring a particular database we can reset the index to old value but this looks like a hack.

Here, I am proposing a patch which will dynamically enlarge the on_exit_nicely_list array by doubling the previous size.

Thoughts?

This reply might belong in another thread, but since you raised the
issue here, instead of registering multiple functions with
exit_nicely, why not modify the argument of the exit function
(archive_close_connection()) to handle multiple arguments? In short,
you could change ShutdownInformation[1]typedef struct ShutdownInformation { ParallelState *pstate; Archive *AHX; -> change this to List of Archive* } ShutdownInformation; so that, instead of holding a
single "Archive," it holds a list, allowing you to append different
Archive handles to the list.

[1]: typedef struct ShutdownInformation { ParallelState *pstate; Archive *AHX; -> change this to List of Archive* } ShutdownInformation;
typedef struct ShutdownInformation
{
ParallelState *pstate;
Archive *AHX; -> change this to List of Archive*
} ShutdownInformation;

--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com

On Tue, 11 Mar 2025 at 11:58, Dilip Kumar <dilipbalaut@gmail.com> wrote:

On Mon, Mar 10, 2025 at 2:24 PM Mahendra Singh Thalor
<mahi6run@gmail.com> wrote:

Hi,
(refer file src/bin/pg_dump/pg_backup_utils.c)

While doing some code changes with pg_dumpall and pg_rsetore[1], we noticed that on_exit_nicely_list array has only fixed slots (MAX_ON_EXIT_NICELY=20) but in some cases, we need more slots for pg_restore.
Ex: restore more than 20 databases by single pg_restore command.

We are working on a patch[1] which dumps all the databases in non-text mode by pg_dumpall and then we are restoring those dumps by pg_restore. So we need more slots due to multiple databases.

Apart from the attached patch solution, we thought of some more solutions.
Solution 1: reset array index with each database restore, but this might break some other things.
Solution 2: for each database, we can keep the index of the on_exit_nicely_list array and after restoring a particular database we can reset the index to old value but this looks like a hack.

Here, I am proposing a patch which will dynamically enlarge the on_exit_nicely_list array by doubling the previous size.

Thoughts?

This reply might belong in another thread, but since you raised the
issue here, instead of registering multiple functions with
exit_nicely, why not modify the argument of the exit function
(archive_close_connection()) to handle multiple arguments? In short,
you could change ShutdownInformation[1] so that, instead of holding a
single "Archive," it holds a list, allowing you to append different
Archive handles to the list.

[1]
typedef struct ShutdownInformation
{
ParallelState *pstate;
Archive *AHX; -> change this to List of Archive*
} ShutdownInformation;

Thanks Tom and Dilip for feedback.

As per above discussion, I did cleanup in different ways to fix the
problem in "non-text mde for pg_dumpall" patch.

--
Thanks and Regards
Mahendra Singh Thalor
EnterpriseDB: http://www.enterprisedb.com