Re: [HACKERS] Fix a failure of pg_dump with !HAVE_LIBZ

2016-05-27 Thread Kyotaro HORIGUCHI
At Thu, 26 May 2016 11:54:35 -0400, Tom Lane  wrote in 
<7601.1464278...@sss.pgh.pa.us>
> Kyotaro HORIGUCHI  writes:
> > The warning says that it makes uncompressed archive but it really
> > doesn't since workers die unexpectedly from the succeeding
> > errors. This is because that compressLevel is corrected in
> > ReadHead(), where too late for it to be propagated to workers.
> > One reasonable place would be where the default value of
> > compressLevel is set in main().
> 
> Yeah, agreed.  I also noticed that you get the WARNING even when you
> did not ask for compression, which seems rather unhelpful and annoying.
> So I suppressed it by making the default be 0 not Z_DEFAULT_COMPRESSION
> when we don't HAVE_LIBZ.

Yeah, that's what I thought but didn't. Thanks for adding it and
committing this.

regards,

-- 
Kyotaro Horiguchi
NTT Open Source Software Center




-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Fix a failure of pg_dump with !HAVE_LIBZ

2016-05-26 Thread Tom Lane
Kyotaro HORIGUCHI  writes:
> The warning says that it makes uncompressed archive but it really
> doesn't since workers die unexpectedly from the succeeding
> errors. This is because that compressLevel is corrected in
> ReadHead(), where too late for it to be propagated to workers.
> One reasonable place would be where the default value of
> compressLevel is set in main().

Yeah, agreed.  I also noticed that you get the WARNING even when you
did not ask for compression, which seems rather unhelpful and annoying.
So I suppressed it by making the default be 0 not Z_DEFAULT_COMPRESSION
when we don't HAVE_LIBZ.

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


[HACKERS] Fix a failure of pg_dump with !HAVE_LIBZ

2016-05-26 Thread Kyotaro HORIGUCHI
Hello.

I got the following messages during investigating some other bug,
from pg_dump compiled with --without-zlib.

> $ rm -rf testdump; pg_dump "postgres://horiguti:hoge@localhost/postgres" 
> --jobs=9 -Fd -f testdump; echo $?
> pg_dump: [archiver] WARNING: requested compression not available in this 
> installation -- archive will be uncompressed
> pg_dump: [parallel archiver] not built with zlib support
> pg_dump: [archiver (db)] query failed: ERROR:  could not import the requested 
> snapshot
> DETAIL:  The source transaction 10116 is not running anymore.
> pg_dump: [archiver (db)] query failed: ERROR:  invalid snapshot identifier: 
> "2784-1"
> pg_dump: [archiver (db)] query failed: ERROR:  invalid snapshot identifier: 
> "2784-1"
> 1

The warning says that it makes uncompressed archive but it really
doesn't since workers die unexpectedly from the succeeding
errors. This is because that compressLevel is corrected in
ReadHead(), where too late for it to be propagated to workers.
One reasonable place would be where the default value of
compressLevel is set in main(). (The following errors mentioning
snapshot id are the consequences of unexpected sudden death of
the first worker from this bug, which causes expiration of the
snapshot.)

It works correctly with the patch attached, both on Linux and on
Windows.

> $ rm -rf testdump; pg_dump "postgres://horiguti:hoge@localhost/postgres" 
> --jobs=9 -Fd -f testdump; echo $?
> pg_dump: WARNING: requested compression (-1) not available in this 
> installation -- archive will be uncompressed
> 0

regards,

-- 
Kyotaro Horiguchi
NTT Open Source Software Center

>From ceb471eea3e688be51e7f167ff7b223072790050 Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi 
Date: Thu, 26 May 2016 18:28:14 +0900
Subject: [PATCH] Fix compressLevel correction for parallel mode

Being compiled with --without-zlib, pg_dump unexpectedly fails to
create an archive in parallel mode. This is because compressLevel is
checked too late to be propagated to child workers. This patch moves
it to earlier enough.
---
 src/bin/pg_dump/pg_backup_archiver.c | 8 
 src/bin/pg_dump/pg_dump.c| 8 
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/src/bin/pg_dump/pg_backup_archiver.c b/src/bin/pg_dump/pg_backup_archiver.c
index 9390a6b..5026027 100644
--- a/src/bin/pg_dump/pg_backup_archiver.c
+++ b/src/bin/pg_dump/pg_backup_archiver.c
@@ -3476,14 +3476,6 @@ WriteHead(ArchiveHandle *AH)
 	(*AH->WriteBytePtr) (AH, AH->offSize);
 	(*AH->WriteBytePtr) (AH, AH->format);
 
-#ifndef HAVE_LIBZ
-	if (AH->compression != 0)
-		write_msg(modulename, "WARNING: requested compression not available in this "
-  "installation -- archive will be uncompressed\n");
-
-	AH->compression = 0;
-#endif
-
 	WriteInt(AH, AH->compression);
 
 	crtm = *localtime(>createDate);
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 1267afb..45a62bd 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -594,6 +594,14 @@ main(int argc, char **argv)
 		else
 			compressLevel = 0;
 	}
+#ifndef HAVE_LIBZ
+	if (compressLevel != 0)
+		write_msg(NULL, "WARNING: requested compression (%d) not "
+  "available in this installation -- "
+  "archive will be uncompressed\n",
+			compressLevel);
+	compressLevel = 0;
+#endif
 
 	/*
 	 * On Windows we can only have at most MAXIMUM_WAIT_OBJECTS (= 64 usually)
-- 
1.8.3.1


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers