public inbox for [email protected]
help / color / mirror / Atom feedFrom: Mahendra Singh Thalor <[email protected]>
To: Andrew Dunstan <[email protected]>
Cc: Álvaro Herrera <[email protected]>
Cc: jian he <[email protected]>
Cc: Srinath Reddy <[email protected]>
Cc: [email protected]
Subject: Re: Non-text mode for pg_dumpall
Date: Wed, 16 Apr 2025 00:00:25 +0530
Message-ID: <CAKYtNAq6LdEFcpQY0f79FdODfHLhoYz7G_DJXU=DCc4AU++Rew@mail.gmail.com> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
<[email protected]>
<CAKYtNArtO8T-u5=E4ibxFfW+zQ8dBmOZQFgRa36+27NFkdtK0Q@mail.gmail.com>
<[email protected]>
<CAKYtNAoE1xUcMmBq-1qCaZGUDQLfMSVa6hzBVA-PqbmSTzPf5A@mail.gmail.com>
<CAKYtNAq530UTAtnVs5xfONQ0j6vS-Sys50p5+SNfC7G7_ghCVQ@mail.gmail.com>
<[email protected]>
On Sat, 5 Apr 2025 at 01:41, Andrew Dunstan <[email protected]> wrote:
>
>
> On 2025-04-04 Fr 5:12 AM, Mahendra Singh Thalor wrote:
>
> On Fri, 4 Apr 2025 at 13:52, Mahendra Singh Thalor <[email protected]> wrote:
>
> On Fri, 4 Apr 2025 at 01:17, Andrew Dunstan <[email protected]> wrote:
>
> On 2025-04-01 Tu 1:59 AM, Mahendra Singh Thalor wrote:
>
> On Mon, 31 Mar 2025 at 23:43, Álvaro Herrera <[email protected]> wrote:
>
> Hi
>
> FWIW I don't think the on_exit_nicely business is in final shape just
> yet. We're doing something super strange and novel about keeping track
> of an array index, so that we can modify it later. Or something like
> that, I think? That doesn't sound all that nice to me. Elsewhere it
> was suggested that we need some way to keep track of the list of things
> that need cleanup (a list of connections IIRC?) -- perhaps in a
> thread-local variable or a global or something -- and we install the
> cleanup function once, and that reads from the variable. The program
> can add things to the list, or remove them, at will; and we don't need
> to modify the cleanup function in any way.
>
> --
> Álvaro Herrera Breisgau, Deutschland — https://www.EnterpriseDB.com/
>
> Thanks Álvaro for the feedback.
>
> I removed the old handling of on_exit_nicely_list from the last patch
> set and added one simple function to just update the archive handle in
> shutdown_info. (shutdown_info.AHX = AHX;)
>
> For first database, we will add entry into on_exit_nicely_list array
> and for rest database, we will update only shutdown_info as we already
> closed connection for previous database.With this fix, we will not
> touch entry of on_exit_nicely_list for each database.
>
> Here, I am attaching updated patches.
>
>
> OK, looks good. here's my latest. I'm currently working on tidying up
> docco and comments.
>
>
> cheers
>
>
> andrew
>
>
>
>
> --
> Andrew Dunstan
> EDB: https://www.enterprisedb.com
>
> Thanks Andrew for the updated patches.
>
> Here, I am attaching a delta patch with some more TAP-test cases.
>
> Here, I am attaching an updated delta patch which has some more TAP
> tests. Please include these tests also. This patch can be applied on
> v20250403_0004* patch.
>
>
>
> Thanks. I have pushed these now with a few further small tweaks.
>
>
> cheers
>
>
> andrew
>
> --
> Andrew Dunstan
> EDB: https://www.enterprisedb.com
Hi Andrew,
I did some refactoring to find out dump file extensions(.dmp/.tar etc)
in pg_restore. With the attached patch, we will not try to find out
file extension with each database, rather we will find out before the
loop.
Here, I am attaching a patch for the same. Please have a look over this.
--
Thanks and Regards
Mahendra Singh Thalor
EnterpriseDB: http://www.enterprisedb.com
Attachments:
[application/octet-stream] v01-pg_restore-refactor-code-of-dump-file-extenion.patch (4.6K, 2-v01-pg_restore-refactor-code-of-dump-file-extenion.patch)
download | inline diff:
From b4721f4c91017297cfbdeaa458291f7a97b023d7 Mon Sep 17 00:00:00 2001
From: Mahendra Singh Thalor <[email protected]>
Date: Tue, 15 Apr 2025 23:45:44 +0530
Subject: [PATCH] pg_restore: refactor code of dump file extenion
After this refactor, we will find file extension once but earlier we were trying
to get extension for each database in loop.
---
src/bin/pg_dump/pg_restore.c | 82 +++++++++++++++++++++++++++---------
1 file changed, 61 insertions(+), 21 deletions(-)
diff --git a/src/bin/pg_dump/pg_restore.c b/src/bin/pg_dump/pg_restore.c
index ff4bb320fc9..e6486957620 100644
--- a/src/bin/pg_dump/pg_restore.c
+++ b/src/bin/pg_dump/pg_restore.c
@@ -71,6 +71,8 @@ static int get_dbnames_list_to_restore(PGconn *conn,
SimpleStringList db_exclude_patterns);
static int get_dbname_oid_list_from_mfile(const char *dumpdirpath,
SimpleOidStringList *dbname_oid_list);
+static
+char *get_dump_file_exten(const char *dumpdirpath, Oid dboid, const ArchiveFormat format);
int
main(int argc, char **argv)
@@ -1109,6 +1111,7 @@ restore_all_databases(PGconn *conn, const char *dumpdirpath,
bool dumpData = opts->dumpData;
bool dumpSchema = opts->dumpSchema;
bool dumpStatistics = opts->dumpSchema;
+ const char *file_exten;
/* Save db name to reuse it for all the database. */
if (opts->cparams.dbname)
@@ -1163,6 +1166,9 @@ restore_all_databases(PGconn *conn, const char *dumpdirpath,
pg_log_info("need to restore %d databases out of %d databases", num_db_restore, num_total_db);
+ /* Now get dump file extention. */
+ file_exten = get_dump_file_exten(dumpdirpath, dbname_oid_list.head->oid, opts->format);
+
/*
* Till now, we made a list of databases, those needs to be restored after
* skipping names of exclude-database. Now we can launch parallel workers
@@ -1172,8 +1178,6 @@ restore_all_databases(PGconn *conn, const char *dumpdirpath,
db_cell; db_cell = db_cell->next)
{
char subdirpath[MAXPGPATH];
- char subdirdbpath[MAXPGPATH];
- char dbfilename[MAXPGPATH];
int n_errors;
/* ignore dbs marked for skipping */
@@ -1190,25 +1194,8 @@ restore_all_databases(PGconn *conn, const char *dumpdirpath,
opts->cparams.override_dbname = NULL;
}
- snprintf(subdirdbpath, MAXPGPATH, "%s/databases", dumpdirpath);
-
- /*
- * Look for the database dump file/dir. If there is an {oid}.tar or
- * {oid}.dmp file, use it. Otherwise try to use a directory called
- * {oid}
- */
- snprintf(dbfilename, MAXPGPATH, "%u.tar", db_cell->oid);
- if (file_exists_in_directory(subdirdbpath, dbfilename))
- snprintf(subdirpath, MAXPGPATH, "%s/databases/%u.tar", dumpdirpath, db_cell->oid);
- else
- {
- snprintf(dbfilename, MAXPGPATH, "%u.dmp", db_cell->oid);
-
- if (file_exists_in_directory(subdirdbpath, dbfilename))
- snprintf(subdirpath, MAXPGPATH, "%s/databases/%u.dmp", dumpdirpath, db_cell->oid);
- else
- snprintf(subdirpath, MAXPGPATH, "%s/databases/%u", dumpdirpath, db_cell->oid);
- }
+ /* Set particular dump file path. */
+ snprintf(subdirpath, MAXPGPATH, "%s/databases/%u%s", dumpdirpath, db_cell->oid, file_exten);
pg_log_info("restoring database \"%s\"", db_cell->str);
@@ -1384,3 +1371,56 @@ copy_or_print_global_file(const char *outfile, FILE *pfile)
if (strcmp(outfile, "-") != 0)
fclose(OPF);
}
+
+/*
+ * get_dump_file_exten
+ *
+ * Look for the database dump file/dir. If there is an {oid}.tar or
+ * {oid}.dmp file, use it. Otherwise try to use a directory called
+ * {oid}
+ */
+static
+char *get_dump_file_exten(const char *dumpdirpath, Oid dboid, const ArchiveFormat format)
+{
+ char *file_exten = "";
+ char subdirdbpath[MAXPGPATH];
+ char dbfilename[MAXPGPATH];
+
+ snprintf(subdirdbpath, MAXPGPATH, "%s/databases", dumpdirpath);
+
+ switch(format)
+ {
+ case archCustom:
+ file_exten = ".dmp";
+ break;
+ case archTar:
+ file_exten = ".tar";
+ break;
+ case archDirectory:
+ file_exten = "";
+ break;
+ case archUnknown: /* based on file exist, try to get file extension name. */
+ snprintf(dbfilename, MAXPGPATH, "%u", dboid);
+ if (file_exists_in_directory(subdirdbpath, dbfilename))
+ file_exten = "";
+ else
+ {
+ snprintf(dbfilename, MAXPGPATH, "%u.dmp", dboid);
+ if (file_exists_in_directory(subdirdbpath, dbfilename))
+ file_exten = ".dmp";
+ else
+ {
+ snprintf(dbfilename, MAXPGPATH, "%u.tar", dboid);
+ if (file_exists_in_directory(subdirdbpath, dbfilename))
+ file_exten = ".tar";
+ else
+ file_exten = "";
+ }
+ }
+ break;
+ default:
+ pg_fatal("unrecognized file format \"%d\"", format);
+ }
+
+ return file_exten;
+}
--
2.39.3
view thread (100+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: Non-text mode for pg_dumpall
In-Reply-To: <CAKYtNAq6LdEFcpQY0f79FdODfHLhoYz7G_DJXU=DCc4AU++Rew@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox