pg_restore logging inconsistency

Started by Josh Kupershmidtover 13 years ago2 messages
#1Josh Kupershmidt
schmiddy@gmail.com
1 attachment(s)

Hi all,

Bosco Rama recently complained[1]http://archives.postgresql.org/pgsql-general/2012-05/msg00456.php about not seeing a message printed
by pg_restore for each LO to be restored. The culprit seems to be the
different level passed to ahlog() for this status message:

pg_backup_archiver.c: ahlog(AH, 2, "restoring large object with OID %u\n", oid);
pg_backup_tar.c: ahlog(AH, 1, "restoring large object OID %u\n", oid);

depending on whether one is restoring a tar-format or custom-format
dump. I think these messages should be logged at the same level, to
avoid this inconsistency. The attached patch logs them both with
level=1, and makes the message texts identical. Note, as of 9.0 there
is already a line like this printed for each LO:

pg_restore: executing BLOB 135004

so I could see the argument for instead wanting to hide the "restoring
large object" messages. However, the OP was interested in seeing
something like a status indicator for the lo_write() calls which may
take a long time, and the above message isn't really helpful for that
purpose as it is printed earlier in the restore process. Plus it seems
reasonable to make verbose mode, well, verbose.

Josh

[1]: http://archives.postgresql.org/pgsql-general/2012-05/msg00456.php

Attachments:

pg_restore_lo_message.diffapplication/octet-stream; name=pg_restore_lo_message.diffDownload
diff --git a/src/bin/pg_dump/pg_backup_archiver.c b/src/bin/pg_dump/pg_backup_archiver.c
new file mode 100644
index 7aaba36..d28cae5
*** a/src/bin/pg_dump/pg_backup_archiver.c
--- b/src/bin/pg_dump/pg_backup_archiver.c
*************** StartRestoreBlob(ArchiveHandle *AH, Oid 
*** 988,994 ****
  	/* Initialize the LO Buffer */
  	AH->lo_buf_used = 0;
  
! 	ahlog(AH, 2, "restoring large object with OID %u\n", oid);
  
  	/* With an old archive we must do drop and create logic here */
  	if (old_blob_style && drop)
--- 988,994 ----
  	/* Initialize the LO Buffer */
  	AH->lo_buf_used = 0;
  
! 	ahlog(AH, 1, "restoring large object with OID %u\n", oid);
  
  	/* With an old archive we must do drop and create logic here */
  	if (old_blob_style && drop)
diff --git a/src/bin/pg_dump/pg_backup_tar.c b/src/bin/pg_dump/pg_backup_tar.c
new file mode 100644
index 451c957..e3c455b
*** a/src/bin/pg_dump/pg_backup_tar.c
--- b/src/bin/pg_dump/pg_backup_tar.c
*************** _LoadBlobs(ArchiveHandle *AH, RestoreOpt
*** 728,734 ****
  			oid = atooid(&th->targetFile[5]);
  			if (oid != 0)
  			{
! 				ahlog(AH, 1, "restoring large object OID %u\n", oid);
  
  				StartRestoreBlob(AH, oid, ropt->dropSchema);
  
--- 728,734 ----
  			oid = atooid(&th->targetFile[5]);
  			if (oid != 0)
  			{
! 				ahlog(AH, 1, "restoring large object with OID %u\n", oid);
  
  				StartRestoreBlob(AH, oid, ropt->dropSchema);
  
#2Alvaro Herrera
alvherre@commandprompt.com
In reply to: Josh Kupershmidt (#1)
Re: pg_restore logging inconsistency

Excerpts from Josh Kupershmidt's message of mié may 30 14:55:12 -0400 2012:

Hi all,

Bosco Rama recently complained[1] about not seeing a message printed
by pg_restore for each LO to be restored. The culprit seems to be the
different level passed to ahlog() for this status message:

pg_backup_archiver.c: ahlog(AH, 2, "restoring large object with OID %u\n", oid);
pg_backup_tar.c: ahlog(AH, 1, "restoring large object OID %u\n", oid);

depending on whether one is restoring a tar-format or custom-format
dump. I think these messages should be logged at the same level, to
avoid this inconsistency. The attached patch logs them both with
level=1, and makes the message texts identical. Note, as of 9.0 there
is already a line like this printed for each LO:

pg_restore: executing BLOB 135004

so I could see the argument for instead wanting to hide the "restoring
large object" messages. However, the OP was interested in seeing
something like a status indicator for the lo_write() calls which may
take a long time, and the above message isn't really helpful for that
purpose as it is printed earlier in the restore process. Plus it seems
reasonable to make verbose mode, well, verbose.

I applied this patch all the way back to 8.3. Thanks.

--
Álvaro Herrera <alvherre@commandprompt.com>
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support