Re: [HACKERS] pg_filedump doesn't compile with v10 sources

[Ashutosh Sharma]

Hi,

On Mon, Jun 26, 2017 at 12:25 PM, tushar  wrote:
> Hi,
>
> While trying to do - make of pg_filedump against v10 sources , getting an
> errors
>
> [centos@centos-cpula pg_filedump]$ make
> cc -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector
> --param=ssp-buffer-size=4 -m64 -mtune=generic -DLINUX_OOM_ADJ=0 -Wall
> -Wmissing-prototypes -Wpointer-arith -Wdeclaration-after-statement
> -Wendif-labels -fno-strict-aliasing -fwrapv
> -I/home/centos/pg10_/postgresql/src/include/ pg_filedump.c -c
> pg_filedump.c: In function ‘FormatControl’:
> pg_filedump.c:1650: error: ‘ControlFileData’ has no member named
> ‘enableIntTimes’
> make: *** [pg_filedump.o] Error 1
> [centos@centos-cpula pg_filedump]$
>

That's because the following git commit in v10 has removed
'enableIntTimes' member from 'ControlFileData' structure,

commit d28aafb6dda326688e2f042c95c93ea57963c03c
Author: Tom Lane 
Date:   Thu Feb 23 12:23:12 2017 -0500

Remove pg_control's enableIntTimes field.

We don't need it any more.

pg_controldata continues to report that date/time type storage is
"64-bit integers", but that's now a hard-wired behavior not something
it sees in the data.  This avoids breaking pg_upgrade, and perhaps other
utilities that inspect pg_control this way.  Ditto for pg_resetwal.

I chose to remove the "bigint_timestamps" output column of
pg_control_init(), though, as that function hasn't been around long
and probably doesn't have ossified users.

Discussion: https://postgr.es/m/26788.1487455...@sss.pgh.pa.us

I think we will have change pg_filedump such that it no more refers to
'enableIntTimes'.

--
With Regards,
Ashutosh Sharma
EnterpriseDB:http://www.enterprisedb.com


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

[HACKERS] pg_filedump doesn't compile with v10 sources

[tushar]

Hi,

While trying to do - make of pg_filedump against v10 sources , getting 
an  errors


[centos@centos-cpula pg_filedump]$ make
cc -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions 
-fstack-protector --param=ssp-buffer-size=4 -m64 -mtune=generic 
-DLINUX_OOM_ADJ=0 -Wall -Wmissing-prototypes -Wpointer-arith 
-Wdeclaration-after-statement -Wendif-labels -fno-strict-aliasing 
-fwrapv  -I/home/centos/pg10_/postgresql/src/include/ pg_filedump.c -c

pg_filedump.c: In function ‘FormatControl’:
pg_filedump.c:1650: error: ‘ControlFileData’ has no member named 
‘enableIntTimes’

make: *** [pg_filedump.o] Error 1
[centos@centos-cpula pg_filedump]$

--
regards,tushar
EnterpriseDB  https://www.enterprisedb.com/
The Enterprise PostgreSQL Company



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

[HACKERS] pg_filedump 9.5.0

[Christoph Berg]

Re: To Pavel Raiskup 2016-03-19 <20160319170614.gb8...@msg.df7cb.de>
> thanks for the patches, I've pushed them to the git repo.
> 
> http://git.postgresql.org/gitweb/?p=pg_filedump.git

We don't have any place to put releases, so I'm posting the tar ball
here...

Christoph


pg_filedump-9.5.0.tar.gz
Description: application/gzip

-- 
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] pg_filedump patch for 9.5

[Christoph Berg]

Re: Pavel Raiskup 2016-02-26 <8883822.6jzmttx...@nb.usersys.redhat.com>
> On Saturday 08 of August 2015 20:38:38 Satoshi Nagayasu wrote:
> > I have created a patch for pg_filedump to work with 9.5.
> > Here is a list of changes.
> > 
> >  * Fix to rename CRC32 macros to work with 9.5.
> >  * Fix to add missing DBState: DB_SHUTDOWNED_IN_RECOVERY.
> >  * Fix to add missing page flags for Btree and GIN.
> >  * Update copyright date.
> > 
> > Please take a look. Any comments are welcome.
> 
> Thanks for the patch;  it helps with building against 9.5.
> 
> Hints I can give ATM:
> 
>   * copyright is outdated now 
>   * to allow the build with 'make
> PGSQL_INCLUDE_DIR=/usr/include/pgsql/server' the following patch is
> needed:
> 
> --- a/Makefile
> +++ b/Makefile
> @@ -18,7 +18,7 @@ DISTFILES= README.pg_filedump Makefile Makefile.contrib 
> \
>  all: pg_filedump
>  
>  pg_filedump: pg_filedump.o
> -   ${CC} ${CFLAGS} -o pg_filedump pg_filedump.o
> +   ${CC} ${CFLAGS} -o pg_filedump pg_filedump.o -lpgport
> 
> Pavel

Hi,

thanks for the patches, I've pushed them to the git repo.

http://git.postgresql.org/gitweb/?p=pg_filedump.git

Christoph


-- 
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] pg_filedump patch for 9.5

[Pavel Raiskup]

On Saturday 08 of August 2015 20:38:38 Satoshi Nagayasu wrote:
> I have created a patch for pg_filedump to work with 9.5.
> Here is a list of changes.
> 
>  * Fix to rename CRC32 macros to work with 9.5.
>  * Fix to add missing DBState: DB_SHUTDOWNED_IN_RECOVERY.
>  * Fix to add missing page flags for Btree and GIN.
>  * Update copyright date.
> 
> Please take a look. Any comments are welcome.

Thanks for the patch;  it helps with building against 9.5.

Hints I can give ATM:

  * copyright is outdated now 
  * to allow the build with 'make
PGSQL_INCLUDE_DIR=/usr/include/pgsql/server' the following patch is
needed:

--- a/Makefile
+++ b/Makefile
@@ -18,7 +18,7 @@ DISTFILES= README.pg_filedump Makefile Makefile.contrib \
 all: pg_filedump
 
 pg_filedump: pg_filedump.o
-   ${CC} ${CFLAGS} -o pg_filedump pg_filedump.o
+   ${CC} ${CFLAGS} -o pg_filedump pg_filedump.o -lpgport

Pavel



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

[HACKERS] pg_filedump patch for 9.5

[Satoshi Nagayasu]

Hi,

I have created a patch for pg_filedump to work with 9.5.
Here is a list of changes.

 * Fix to rename CRC32 macros to work with 9.5.
 * Fix to add missing DBState: DB_SHUTDOWNED_IN_RECOVERY.
 * Fix to add missing page flags for Btree and GIN.
 * Update copyright date.

Please take a look. Any comments are welcome.

Regards,

-- 
NAGAYASU Satoshi sn...@uptime.jp
diff --git a/Makefile b/Makefile
index c64dfbf..d80a3a3 100644
--- a/Makefile
+++ b/Makefile
@@ -1,7 +1,7 @@
 # View README.pg_filedump first
 
 # note this must match version macros in pg_filedump.h
-FD_VERSION=9.3.0
+FD_VERSION=9.5.0
 
 CC=gcc
 CFLAGS=-g -O -Wall -Wmissing-prototypes -Wmissing-declarations
diff --git a/README.pg_filedump b/README.pg_filedump
index 3a04d59..13d51ff 100644
--- a/README.pg_filedump
+++ b/README.pg_filedump
@@ -2,7 +2,7 @@ pg_filedump - Display formatted contents of a PostgreSQL heap, 
index,
   or control file.
 
 Copyright (c) 2002-2010 Red Hat, Inc.
-Copyright (c) 2011-2014, PostgreSQL Global Development Group
+Copyright (c) 2011-2015, PostgreSQL Global Development Group
 
 This program is free software; you can redistribute it and/or modify
 it under the terms of the GNU General Public License as published by
diff --git a/pg_filedump.c b/pg_filedump.c
index 6cd02a9..1dfc524 100644
--- a/pg_filedump.c
+++ b/pg_filedump.c
@@ -3,7 +3,7 @@
  *formatting heap (data), index and control 
files.
  *
  * Copyright (c) 2002-2010 Red Hat, Inc.
- * Copyright (c) 2011-2014, PostgreSQL Global Development Group
+ * Copyright (c) 2011-2015, PostgreSQL Global Development Group
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -24,7 +24,7 @@
 
 #include pg_filedump.h
 
-#include utils/pg_crc_tables.h
+#include utils/pg_crc.h
 
 /* checksum_impl.h uses Assert, which doesn't work outside the server */
 #undef Assert
@@ -91,7 +91,7 @@ DisplayOptions(unsigned int validOptions)
printf
(\nVersion %s (for %s)
 \nCopyright (c) 2002-2010 Red Hat, Inc.
- \nCopyright (c) 2011-2014, PostgreSQL Global Development 
Group\n,
+ \nCopyright (c) 2011-2015, PostgreSQL Global Development 
Group\n,
 FD_VERSION, FD_PG_VERSION);
 
printf
@@ -1132,6 +1132,8 @@ FormatSpecial()
strcat(flagString, SPLITEND|);
if (btreeSection-btpo_flags  BTP_HAS_GARBAGE)
strcat(flagString, HASGARBAGE|);
+   if (btreeSection-btpo_flags  
BTP_INCOMPLETE_SPLIT)
+   strcat(flagString, INCOMPLETESPLIT|);
if (strlen(flagString))
flagString[strlen(flagString) - 1] = 
'\0';
 
@@ -1216,6 +1218,10 @@ FormatSpecial()
strcat(flagString, LIST|);
if (ginSection-flags  GIN_LIST_FULLROW)
strcat(flagString, FULLROW|);
+   if (ginSection-flags  GIN_INCOMPLETE_SPLIT)
+   strcat(flagString, INCOMPLETESPLIT|);
+   if (ginSection-flags  GIN_COMPRESSED)
+   strcat(flagString, COMPRESSED|);
if (strlen(flagString))
flagString[strlen(flagString) - 1] = 
'\0';
printf( GIN Index Section:\n
@@ -1340,9 +1346,9 @@ FormatControl()
char   *dbState;
 
/* Compute a local copy of the CRC to verify the one on disk */
-   INIT_CRC32(crcLocal);
-   COMP_CRC32(crcLocal, buffer, offsetof(ControlFileData, crc));
-   FIN_CRC32(crcLocal);
+   INIT_CRC32C(crcLocal);
+   COMP_CRC32C(crcLocal, buffer, offsetof(ControlFileData, crc));
+   FIN_CRC32C(crcLocal);
 
/* Grab a readable version of the database state */
switch (controlData-state)
@@ -1353,6 +1359,9 @@ FormatControl()
case DB_SHUTDOWNED:
dbState = SHUTDOWNED;
break;
+   case DB_SHUTDOWNED_IN_RECOVERY:
+   dbState = SHUTDOWNED_IN_RECOVERY;
+   break;
case DB_SHUTDOWNING:
dbState = SHUTDOWNING;
break;
@@ -1400,7 +1409,7 @@ FormatControl()
  Maximum Index Keys: %u\n
TOAST Chunk Size: %u\n
  Date and Time Type Storage: 

Re: [HACKERS] pg_filedump for 9.4?

[Christoph Berg]

Re: Fabrízio de Royes Mello 2014-06-25 
CAFcNs+oAb8h-0w2vLEWj6R-Gv=xizgdBya3K=SCd_9Tjyo=z...@mail.gmail.com
 On Wed, Jun 25, 2014 at 3:52 PM, Tom Lane t...@sss.pgh.pa.us wrote:
  Would like that, but I'm not sure what pgindent will do with the //
  comments.  It's been on my to-do list to switch all the comments to C89
  style and then pgindent it, but I don't see myself getting to that in this
  decade :-(
 
 
 I changed all // comments to /* */ and run pgindent.

I've pushed these patches to the git repository, thanks.

Christoph
-- 
c...@df7cb.de | http://www.df7cb.de/


-- 
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] pg_filedump for 9.4?

[Fabrízio de Royes Mello]

Em domingo, 31 de agosto de 2014, Christoph Berg c...@df7cb.de escreveu:

 Re: Fabrízio de Royes Mello 2014-06-25
 CAFcNs+oAb8h-0w2vLEWj6R-Gv=xizgdBya3K=SCd_9Tjyo=z...@mail.gmail.com
 javascript:;
  On Wed, Jun 25, 2014 at 3:52 PM, Tom Lane t...@sss.pgh.pa.us
 javascript:; wrote:
   Would like that, but I'm not sure what pgindent will do with the //
   comments.  It's been on my to-do list to switch all the comments to C89
   style and then pgindent it, but I don't see myself getting to that in
 this
   decade :-(
  
 
  I changed all // comments to /* */ and run pgindent.

 I've pushed these patches to the git repository, thanks.


Thanks!

Fabrízio de Royes Mello


-- 
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
 Timbira: http://www.timbira.com.br
 Blog: http://fabriziomello.github.io
 Linkedin: http://br.linkedin.com/in/fabriziomello
 Twitter: http://twitter.com/fabriziomello
 Github: http://github.com/fabriziomello

Re: [HACKERS] pg_filedump for 9.4?

[Stepan Rutz]

Hi community,
while I am currently investigating why a certain table with highly redundant 
and utterly verbose xml becomes worse storage wise when making the xml more 
compact. Since i am quite new to this, I believe its the lz compression in the 
text database. But thats irrelevant now, just mentioning because tools like 
pg_filedump allow people like me to help themselves and a basic understanding 
of things.

During checking I noticed pg_filedump (current from git.postgresql.org incl. 
the below mentioned commit) does not compile on Mac-OSX. Afaik it will not 
compile as soon as post.h comes into play and USE_REPL_SNPRINTF is defined. 
Then printf and sprintf (ouch particular but code path seems tolerable) in the 
source of pg_filedump become pg_printf and so on. These replacements are part 
of postgres and can’t be linked into the standalone pg_filedump. At least that 
is certainly not the intention.

Putting 

#undef sprintf
#undef print

after the includes in pg_filedump fixes the mac compile and imho all builds 
where the USE_REPL_SNPRINTF is defined as a side effect of include postgres.h 
effectively taking printf from me.

Not sure how to deal with this issue correctly so this is just for your 
consideration since the issue is a bit broader imho.

Regards,
Stepan

  
Am 31.08.2014 um 17:25 schrieb Fabrízio de Royes Mello 
fabriziome...@gmail.com:

 
 
 Em domingo, 31 de agosto de 2014, Christoph Berg c...@df7cb.de escreveu:
 Re: Fabrízio de Royes Mello 2014-06-25 
 CAFcNs+oAb8h-0w2vLEWj6R-Gv=xizgdBya3K=SCd_9Tjyo=z...@mail.gmail.com
  On Wed, Jun 25, 2014 at 3:52 PM, Tom Lane t...@sss.pgh.pa.us wrote:
   Would like that, but I'm not sure what pgindent will do with the //
   comments.  It's been on my to-do list to switch all the comments to C89
   style and then pgindent it, but I don't see myself getting to that in this
   decade :-(
  
 
  I changed all // comments to /* */ and run pgindent.
 
 I've pushed these patches to the git repository, thanks.
 
 
 Thanks!
 
 Fabrízio de Royes Mello
 
 
 -- 
 Fabrízio de Royes Mello
 Consultoria/Coaching PostgreSQL
  Timbira: http://www.timbira.com.br
  Blog: http://fabriziomello.github.io
  Linkedin: http://br.linkedin.com/in/fabriziomello
  Twitter: http://twitter.com/fabriziomello
  Github: http://github.com/fabriziomello
 



smime.p7s
Description: S/MIME cryptographic signature

[HACKERS] pg_filedump for 9.4?

[Devrim Gündüz]

Hi,

Will there be a pg_filedump for 9.4? I'd like to finish package tests
before we release 9.4.0.

Regards,
-- 
Devrim GÜNDÜZ
Principal Systems Engineer @ EnterpriseDB: http://www.enterprisedb.com
PostgreSQL Danışmanı/Consultant, Red Hat Certified Engineer
Twitter: @DevrimGunduz , @DevrimGunduzTR



signature.asc
Description: This is a digitally signed message part

Re: [HACKERS] pg_filedump for 9.4?

[Tom Lane]

Devrim =?ISO-8859-1?Q?G=FCnd=FCz?= dev...@gunduz.org writes:
 Will there be a pg_filedump for 9.4? I'd like to finish package tests
 before we release 9.4.0.

Probably, but I have no time for it right now.

FWIW, I believe the current 9.3 sources still work with HEAD/9.4.

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

Re: [HACKERS] pg_filedump for 9.4?

[Fabrízio de Royes Mello]

On Wed, Jun 25, 2014 at 12:31 PM, Tom Lane t...@sss.pgh.pa.us wrote:

 Devrim =?ISO-8859-1?Q?G=FCnd=FCz?= dev...@gunduz.org writes:
  Will there be a pg_filedump for 9.4? I'd like to finish package tests
  before we release 9.4.0.

 Probably, but I have no time for it right now.

 FWIW, I believe the current 9.3 sources still work with HEAD/9.4.


I'll check it and post the results.

Regards,

--
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
 Timbira: http://www.timbira.com.br
 Blog sobre TI: http://fabriziomello.blogspot.com
 Perfil Linkedin: http://br.linkedin.com/in/fabriziomello
 Twitter: http://twitter.com/fabriziomello

Re: [HACKERS] pg_filedump for 9.4?

[Tom Lane]

=?UTF-8?Q?Fabr=C3=ADzio_de_Royes_Mello?= fabriziome...@gmail.com writes:
 On Wed, Jun 25, 2014 at 12:31 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 Devrim =?ISO-8859-1?Q?G=FCnd=FCz?= dev...@gunduz.org writes:
 Will there be a pg_filedump for 9.4? I'd like to finish package tests
 before we release 9.4.0.

 Probably, but I have no time for it right now.
 FWIW, I believe the current 9.3 sources still work with HEAD/9.4.

 I'll check it and post the results.

I forgot to mention that thanks to Christoph Berg, the pg_filedump sources
have gotten off pgfoundry and onto git.postgresql.org.  If anyone's got
bandwidth for working on it (in particular, merging the checksum-testing
code that was posted awhile back), I'd be happy to grant commit access.

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

Re: [HACKERS] pg_filedump for 9.4?

[Fabrízio de Royes Mello]

On Wed, Jun 25, 2014 at 1:28 PM, Tom Lane t...@sss.pgh.pa.us wrote:

 =?UTF-8?Q?Fabr=C3=ADzio_de_Royes_Mello?= fabriziome...@gmail.com writes:
  On Wed, Jun 25, 2014 at 12:31 PM, Tom Lane t...@sss.pgh.pa.us wrote:
  Devrim =?ISO-8859-1?Q?G=FCnd=FCz?= dev...@gunduz.org writes:
  Will there be a pg_filedump for 9.4? I'd like to finish package tests
  before we release 9.4.0.

  Probably, but I have no time for it right now.
  FWIW, I believe the current 9.3 sources still work with HEAD/9.4.

  I'll check it and post the results.

 I forgot to mention that thanks to Christoph Berg, the pg_filedump sources
 have gotten off pgfoundry and onto git.postgresql.org.  If anyone's got
 bandwidth for working on it (in particular, merging the checksum-testing
 code that was posted awhile back), I'd be happy to grant commit access.


Merged...

I'm thinking in run pgindent to better organize the source code... What
do you think?

Regards,

--
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
 Timbira: http://www.timbira.com.br
 Blog sobre TI: http://fabriziomello.blogspot.com
 Perfil Linkedin: http://br.linkedin.com/in/fabriziomello
 Twitter: http://twitter.com/fabriziomello
diff --git a/README.pg_filedump b/README.pg_filedump
index c9104e4..3a04d59 100644
--- a/README.pg_filedump
+++ b/README.pg_filedump
@@ -59,7 +59,7 @@ not require any manual adjustments of the Makefile.
 
 Invocation:
 
-pg_filedump [-abcdfhixy] [-R startblock [endblock]] [-S blocksize] file
+pg_filedump [-abcdfhikxy] [-R startblock [endblock]] [-S blocksize] file
 
 Defaults are: relative addressing, range of the entire file, block size
   as listed on block 0 in the file
@@ -74,6 +74,7 @@ The following options are valid for heap and index files:
   -f  Display formatted block content dump along with interpretation
   -h  Display this information
   -i  Display interpreted item details
+  -k  Verify block checksums
   -R  Display specific block ranges within the file (Blocks are
   indexed from 0)
 [startblock]: block to start at
diff --git a/pg_filedump.c b/pg_filedump.c
index abcdb1e..34ce9d6 100644
--- a/pg_filedump.c
+++ b/pg_filedump.c
@@ -26,6 +26,13 @@
 
 #include utils/pg_crc_tables.h
 
+// checksum_impl.h uses Assert, which doesn't work outside the server
+#undef Assert
+#define Assert(X)
+
+#include storage/checksum.h
+#include storage/checksum_impl.h
+
 // Global variables for ease of use mostly
 static FILE *fp = NULL;		// File to dump or format
 static char *fileName = NULL;	// File name for display
@@ -40,12 +47,12 @@ static unsigned int blockVersion = 0;	// Block version number
 static void DisplayOptions (unsigned int validOptions);
 static unsigned int ConsumeOptions (int numOptions, char **options);
 static int GetOptionValue (char *optionString);
-static void FormatBlock ();
+static void FormatBlock (BlockNumber blkno);
 static unsigned int GetBlockSize ();
 static unsigned int GetSpecialSectionType (Page page);
 static bool IsBtreeMetaPage(Page page);
 static void CreateDumpFileHeader (int numOptions, char **options);
-static int FormatHeader (Page page);
+static int FormatHeader (Page page, BlockNumber blkno);
 static void FormatItemBlock (Page page);
 static void FormatItem (unsigned int numBytes, unsigned int startIndex,
 			unsigned int formatAs);
@@ -68,7 +75,7 @@ DisplayOptions (unsigned int validOptions)
FD_VERSION, FD_PG_VERSION);
 
   printf
-(\nUsage: pg_filedump [-abcdfhixy] [-R startblock [endblock]] [-S blocksize] file\n\n
+(\nUsage: pg_filedump [-abcdfhikxy] [-R startblock [endblock]] [-S blocksize] file\n\n
  Display formatted contents of a PostgreSQL heap/index/control file\n
  Defaults are: relative addressing, range of the entire file, block\n
 size as listed on block 0 in the file\n\n
@@ -82,6 +89,7 @@ DisplayOptions (unsigned int validOptions)
-f  Display formatted block content dump along with interpretation\n
-h  Display this information\n
-i  Display interpreted item details\n
+   -k  Verify block checksums\n
-R  Display specific block ranges within the file (Blocks are\n
indexed from 0)\n [startblock]: block to start at\n
  [endblock]: block to end at\n
@@ -288,6 +296,11 @@ ConsumeOptions (int numOptions, char **options)
 		  SET_OPTION (itemOptions, ITEM_DETAIL, 'i');
 		  break;
 
+		  // Verify block checksums
+		case 'k':
+		  SET_OPTION (blockOptions, BLOCK_CHECKSUMS, 'k');
+		  break;
+
 		  // Interpret items as standard index values
 		case 'x':
 		  SET_OPTION (itemOptions, ITEM_INDEX, 'x');
@@ -555,7 +568,7 @@ CreateDumpFileHeader (int numOptions, char **options)
 
 // Dump out a formatted block header for the requested block
 static int
-FormatHeader (Page page)
+FormatHeader (Page page, BlockNumber blkno)
 {
   int rc = 0;
   unsigned int headerBytes;
@@ -647,6 +660,14 @@ 

Re: [HACKERS] pg_filedump for 9.4?

[Tom Lane]

=?UTF-8?Q?Fabr=C3=ADzio_de_Royes_Mello?= fabriziome...@gmail.com writes:
 I'm thinking in run pgindent to better organize the source code... What
 do you think?

Would like that, but I'm not sure what pgindent will do with the //
comments.  It's been on my to-do list to switch all the comments to C89
style and then pgindent it, but I don't see myself getting to that in this
decade :-(

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

Re: [HACKERS] pg_filedump 9.3: checksums (and a few other fixes)

[Jeff Davis]

On Wed, 2013-07-17 at 13:43 -0400, Alvaro Herrera wrote:
 Tom Lane escribió:
 
  My feeling about this code is that the reason we print the infomask in
  hex is so you can see exactly which bits are set if you care, and that
  the rest of the line ought to be designed to interpret the bits in as
  reader-friendly a way as possible.  So I don't buy the notion that we
  should just print out a name for each bit that's set.  I'd rather
  replace individual bit names with items like LOCKED_FOR_KEY_SHARE,
  LOCKED_FOR_SHARE, etc in cases where you have to combine multiple
  bits to understand the meaning.
 
 Okay, that's what I've been saying all along so I cannot but agree.  I
 haven't reviewed Jeff's patch lately; Jeff, does Tom's suggestion need
 some more new code, and if so are you open to doing this work, or shall
 I?

At first glance it seems like a pretty trivial change. I'm going on
vacation tomorrow and unfortunately I haven't had a chance to look at
this. Pgfoundry CVS is down, so I can't see whether it's already been
committed or not.

Regards,
Jeff Davis




-- 
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] pg_filedump 9.3: checksums (and a few other fixes)

[Alvaro Herrera]

Tom Lane escribió:
 Alvaro Herrera alvhe...@2ndquadrant.com writes:
  Well, Tom opined in
  http://www.postgresql.org/message-id/23249.1370878...@sss.pgh.pa.us that
  the current patch is okay.  I have a mild opinion that it should instead
  print only SHR_LOCK when both bits are set, and one of the others when
  only one of them is set.  But I don't have a strong opinion about this,
  and since Tom disagrees with me, feel free to exercise your own (Jeff's)
  judgement.
 
 FWIW, I think that's exactly what I did in the preliminary 9.3 patch
 that I committed to pg_filedump a few weeks ago.  Could you take a look
 at what's there now and see if that's what you meant?

Here's sample output (-i) from the new code, i.e. this commit:
revision 1.7
date: 2013/06/06 18:33:17;  author: tgl;  state: Exp;  lines: +14 -10
Preliminary updates for Postgres 9.3.


Data -- 
 Item   1 -- Length:   28  Offset: 8160 (0x1fe0)  Flags: NORMAL
  XMIN: 692  XMAX: 693  CID|XVAC: 0
  Block Id: 0  linp Index: 1   Attributes: 1   Size: 24
  infomask: 0x0190 (XMAX_KEYSHR_LOCK|XMAX_LOCK_ONLY|XMIN_COMMITTED) 

 Item   2 -- Length:   28  Offset: 8128 (0x1fc0)  Flags: NORMAL
  XMIN: 692  XMAX: 694  CID|XVAC: 0
  Block Id: 0  linp Index: 2   Attributes: 1   Size: 24
  infomask: 0x01d0 
(XMAX_KEYSHR_LOCK|XMAX_EXCL_LOCK|XMAX_LOCK_ONLY|XMIN_COMMITTED) 

 Item   3 -- Length:   28  Offset: 8096 (0x1fa0)  Flags: NORMAL
  XMIN: 692  XMAX: 695  CID|XVAC: 0
  Block Id: 0  linp Index: 3   Attributes: 1   Size: 24
  infomask: 0x01c0 (XMAX_EXCL_LOCK|XMAX_LOCK_ONLY|XMIN_COMMITTED) 

 Item   4 -- Length:   28  Offset: 8064 (0x1f80)  Flags: NORMAL
  XMIN: 696  XMAX: 697  CID|XVAC: 0
  Block Id: 0  linp Index: 4   Attributes: 1   Size: 24
  infomask: 0x01c0 (XMAX_EXCL_LOCK|XMAX_LOCK_ONLY|XMIN_COMMITTED|KEYS_UPDATED) 

Item 1 has SELECT FOR KEY SHARE
Item 2 has SELECT FOR SHARE
Item 3 has SELECT FOR NO KEY UPDATE
Item 4 has SELECT FOR UPDATE

The one I was talking about is the second case, which prints
KEYSHR_LOCK|EXCL_LOCK to mean that there's a FOR SHARE lock.  I have no
problem reading it this way, but I fear that someone unfamiliar with
these bits might be confused.  On the other hand, trying to be nice and
interpret these bits (i.e. translate presence of both into something
like SHR_LOCK) might also be confusing, because that bit doesn't really
exist.  And one already needs to be careful while interpreting what do
KEYS_UPDATED and XMAX_LOCK_ONLY, or lack thereof, mean.

Perhaps it would be sensible to provide one more output line per tuple,
with interpretation of the flags, so it would tell you whether the tuple
has been locked or updated, and what kind of each it is.  I'd propose
something like

  status: locked (FOR {KEY SHARE,SHARE,NO KEY UPDATE,UPDATE}) [MultiXact: nnn]
  status: [HOT] updated (KEYS UPDATED/KEYS NOT UPDATED) [MultiXact: nnn]  To: 
blk/off
  status: deleted [MultiXact: nnn]

-- 
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training  Services


-- 
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] pg_filedump 9.3: checksums (and a few other fixes)

[Tom Lane]

Alvaro Herrera alvhe...@2ndquadrant.com writes:
 The one I was talking about is the second case, which prints
 KEYSHR_LOCK|EXCL_LOCK to mean that there's a FOR SHARE lock.  I have no
 problem reading it this way, but I fear that someone unfamiliar with
 these bits might be confused.  On the other hand, trying to be nice and
 interpret these bits (i.e. translate presence of both into something
 like SHR_LOCK) might also be confusing, because that bit doesn't really
 exist.  And one already needs to be careful while interpreting what do
 KEYS_UPDATED and XMAX_LOCK_ONLY, or lack thereof, mean.

 Perhaps it would be sensible to provide one more output line per tuple,
 with interpretation of the flags, so it would tell you whether the tuple
 has been locked or updated, and what kind of each it is.  I'd propose
 something like

   status: locked (FOR {KEY SHARE,SHARE,NO KEY UPDATE,UPDATE}) [MultiXact: nnn]
   status: [HOT] updated (KEYS UPDATED/KEYS NOT UPDATED) [MultiXact: nnn]  To: 
 blk/off
   status: deleted [MultiXact: nnn]

Hm.  I'm loath to add another output line per tuple, just for space
reasons.

My feeling about this code is that the reason we print the infomask in
hex is so you can see exactly which bits are set if you care, and that
the rest of the line ought to be designed to interpret the bits in as
reader-friendly a way as possible.  So I don't buy the notion that we
should just print out a name for each bit that's set.  I'd rather
replace individual bit names with items like LOCKED_FOR_KEY_SHARE,
LOCKED_FOR_SHARE, etc in cases where you have to combine multiple
bits to understand the meaning.

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

Re: [HACKERS] pg_filedump 9.3: checksums (and a few other fixes)

[Alvaro Herrera]

Tom Lane escribió:

 My feeling about this code is that the reason we print the infomask in
 hex is so you can see exactly which bits are set if you care, and that
 the rest of the line ought to be designed to interpret the bits in as
 reader-friendly a way as possible.  So I don't buy the notion that we
 should just print out a name for each bit that's set.  I'd rather
 replace individual bit names with items like LOCKED_FOR_KEY_SHARE,
 LOCKED_FOR_SHARE, etc in cases where you have to combine multiple
 bits to understand the meaning.

Okay, that's what I've been saying all along so I cannot but agree.  I
haven't reviewed Jeff's patch lately; Jeff, does Tom's suggestion need
some more new code, and if so are you open to doing this work, or shall
I?

-- 
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training  Services


-- 
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] pg_filedump 9.3: checksums (and a few other fixes)

[Josh Berkus]

On 07/08/2013 04:59 PM, Tom Lane wrote:
 Alvaro Herrera alvhe...@2ndquadrant.com writes:
 Well, Tom opined in
 http://www.postgresql.org/message-id/23249.1370878...@sss.pgh.pa.us that
 the current patch is okay.  I have a mild opinion that it should instead
 print only SHR_LOCK when both bits are set, and one of the others when
 only one of them is set.  But I don't have a strong opinion about this,
 and since Tom disagrees with me, feel free to exercise your own (Jeff's)
 judgement.
 
 FWIW, I think that's exactly what I did in the preliminary 9.3 patch
 that I committed to pg_filedump a few weeks ago.  Could you take a look
 at what's there now and see if that's what you meant?

So, is this getting committed today, or do we bounce it?

-- 
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com


-- 
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] pg_filedump 9.3: checksums (and a few other fixes)

[Tom Lane]

Josh Berkus j...@agliodbs.com writes:
 On 07/08/2013 04:59 PM, Tom Lane wrote:
 FWIW, I think that's exactly what I did in the preliminary 9.3 patch
 that I committed to pg_filedump a few weeks ago.  Could you take a look
 at what's there now and see if that's what you meant?

 So, is this getting committed today, or do we bounce it?

I was hoping for a comment from Alvaro, but wouldn't have gotten to
committing it today in any case.  IMO this patch doesn't really belong
in the commitfest queue, since pg_filedump isn't part of the community
distribution.

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

Re: [HACKERS] pg_filedump 9.3: checksums (and a few other fixes)

[Jeff Davis]

On Fri, 2013-07-05 at 22:43 -0700, Jeff Davis wrote:
 On Sat, 2013-07-06 at 10:30 +0900, Satoshi Nagayasu wrote:
  Hi,
  
  It looks fine, but I have one question here.
  
  When I run pg_filedump with -k against a database cluster which
  does not support checksums, pg_filedump produced checksum error as
  following. Is this expected or acceptable?
 
 Thank you for taking a look. That is expected, because there is not a
 good way to determine whether the file was created with checksums or
 not. So, we rely on the user to supply (or not) the -k flag.

I see this patch is still waiting on author in the CF. Is there
something else needed from me, or should we move this to ready for
committer?

Regards,
Jeff Davis




-- 
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] pg_filedump 9.3: checksums (and a few other fixes)

[Peter Geoghegan]

On Mon, Jul 8, 2013 at 10:28 AM, Jeff Davis pg...@j-davis.com wrote:
 I see this patch is still waiting on author in the CF. Is there
 something else needed from me, or should we move this to ready for
 committer?

Well, obviously someone still needs to think through the handling of
the infoMask bits.

Alvaro?

-- 
Peter Geoghegan


-- 
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] pg_filedump 9.3: checksums (and a few other fixes)

[Alvaro Herrera]

Peter Geoghegan escribió:
 On Mon, Jul 8, 2013 at 10:28 AM, Jeff Davis pg...@j-davis.com wrote:
  I see this patch is still waiting on author in the CF. Is there
  something else needed from me, or should we move this to ready for
  committer?
 
 Well, obviously someone still needs to think through the handling of
 the infoMask bits.

Well, Tom opined in
http://www.postgresql.org/message-id/23249.1370878...@sss.pgh.pa.us that
the current patch is okay.  I have a mild opinion that it should instead
print only SHR_LOCK when both bits are set, and one of the others when
only one of them is set.  But I don't have a strong opinion about this,
and since Tom disagrees with me, feel free to exercise your own (Jeff's)
judgement.

Tom's the only available committer in this case anyway.  [Actually,
pgfoundry.org says it's a zero-people team in the pgfiledump project
right now, but I'm hoping that's just a temporary glitch.]

-- 
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training  Services


-- 
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] pg_filedump 9.3: checksums (and a few other fixes)

[Peter Geoghegan]

On Mon, Jul 8, 2013 at 11:52 AM, Alvaro Herrera
alvhe...@2ndquadrant.com wrote:
 Well, Tom opined in
 http://www.postgresql.org/message-id/23249.1370878...@sss.pgh.pa.us that
 the current patch is okay.  I have a mild opinion that it should instead
 print only SHR_LOCK when both bits are set, and one of the others when
 only one of them is set.  But I don't have a strong opinion about this,
 and since Tom disagrees with me, feel free to exercise your own (Jeff's)
 judgement.

I'm inclined to agree with you here, but I suppose it isn't all that important.


-- 
Peter Geoghegan


-- 
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] pg_filedump 9.3: checksums (and a few other fixes)

[Tom Lane]

Alvaro Herrera alvhe...@2ndquadrant.com writes:
 Well, Tom opined in
 http://www.postgresql.org/message-id/23249.1370878...@sss.pgh.pa.us that
 the current patch is okay.  I have a mild opinion that it should instead
 print only SHR_LOCK when both bits are set, and one of the others when
 only one of them is set.  But I don't have a strong opinion about this,
 and since Tom disagrees with me, feel free to exercise your own (Jeff's)
 judgement.

FWIW, I think that's exactly what I did in the preliminary 9.3 patch
that I committed to pg_filedump a few weeks ago.  Could you take a look
at what's there now and see if that's what you meant?

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

Re: [HACKERS] pg_filedump 9.3: checksums (and a few other fixes)

[Satoshi Nagayasu]

Hi,

I have reviewed this patch as a CF reviewer.

(2013/06/27 4:07), Jeff Davis wrote:

On Mon, 2013-06-24 at 20:34 -0400, Josh Kupershmidt wrote:

This patch is in the current CommitFest, does it still need to be
reviewed? If so, I notice that the version in pgfoundry's CVS is
rather different than the version the patch seems to have been built
against (presumably the pg_filedump-9.2.0.tar.gz release), and
conflicts in several places with cvs tip.


Rebased against CVS tip; attached.


It looks fine, but I have one question here.

When I run pg_filedump with -k against a database cluster which
does not support checksums, pg_filedump produced checksum error as
following. Is this expected or acceptable?

-
***
* PostgreSQL File/Block Formatted Dump Utility - Version 9.3.0
*
* File: /tmp/pgsql/data/base/16384/16397
* Options used: -k
*
* Dump created on: Sat Jul  6 10:32:15 2013
***

Block0 
Header -
 Block Offset: 0x Offsets: Lower 268 (0x010c)
 Block: Size 8192  Version4Upper 384 (0x0180)
 LSN:  logid  0 recoff 0x  Special  8192 (0x2000)
 Items:   61  Free Space:  116
 Checksum: 0x  Prune XID: 0x  Flags: 0x ()
 Length (including item array): 268

 Error: checksum failure: calculated 0xf797.

Data --
 Item   1 -- Length:  121  Offset: 8064 (0x1f80)  Flags: NORMAL
 Item   2 -- Length:  121  Offset: 7936 (0x1f00)  Flags: NORMAL
 Item   3 -- Length:  121  Offset: 7808 (0x1e80)  Flags: NORMAL
-

Please check attached script to reproduce it.

Also, I have update the help message and README.
Please check attached patch.

Regards,
--
Satoshi Nagayasu sn...@uptime.jp
Uptime Technologies, LLC. http://www.uptime.jp
PGHOME=/tmp/pgsql
PGPORT=15433

function build_check {
echo make PGSQL_INCLUDE_DIR=...
make PGSQL_INCLUDE_DIR=../../src/include clean
make PGSQL_INCLUDE_DIR=../../src/include all
make PGSQL_INCLUDE_DIR=../../src/include install
make PGSQL_INCLUDE_DIR=../../src/include clean

echo make -f Makefile.contrib...
make -f Makefile.contrib clean
make -f Makefile.contrib all
make -f Makefile.contrib install
make -f Makefile.contrib clean

echo make -f Makefile.contrib USE_PGXS=1...
PATH=${PGHOME}/bin:$PATH
make -f Makefile.contrib USE_PGXS=1 clean
make -f Makefile.contrib USE_PGXS=1 all
make -f Makefile.contrib USE_PGXS=1 install
make -f Makefile.contrib USE_PGXS=1 clean
}

function do_builddb {
INITDB_OPTS=$1
killall -9 postmaster postgres
rm -rf ${PGHOME}/data

initdb ${INITDB_OPTS} --no-locale -D ${PGHOME}/data
pg_ctl -w -D ${PGHOME}/data start -o -p ${PGPORT}
createdb -p ${PGPORT} testdb
pgbench -p ${PGPORT} -i testdb

psql -A -t -p ${PGPORT} testdbEOF  file
select '${PGHOME}/data/' || pg_relation_filepath(oid) from pg_class where 
relname like 'pgbench%';
EOF
}

function builddb_checksum_enabled {
do_builddb -k
}

function builddb_checksum_disabled {
do_builddb 
}

function test_not_verify_checksum {
LOG=$1
sed 's/^/pg_filedump /'  file  _test.sh
sh _test.sh  $LOG
}

function test_verify_checksum {
LOG=$1
sed 's/^/pg_filedump -k /'  file  _test.sh
sh _test.sh  $LOG
}

build_check

builddb_checksum_enabled
test_not_verify_checksum test_enabled_not_verify.log
test_verify_checksum test_enabled_verify.log

builddb_checksum_disabled
test_not_verify_checksum test_disabled_not_verify.log
test_verify_checksum test_disabled_verify.log
diff --git a/contrib/pg_filedump/README.pg_filedump 
b/contrib/pg_filedump/README.pg_filedump
index 63d086f..b3050cd 100644
--- a/contrib/pg_filedump/README.pg_filedump
+++ b/contrib/pg_filedump/README.pg_filedump
@@ -2,7 +2,7 @@ pg_filedump - Display formatted contents of a PostgreSQL heap, 
index,
   or control file.
 
 Copyright (c) 2002-2010 Red Hat, Inc.
-Copyright (c) 2011-2012, PostgreSQL Global Development Group
+Copyright (c) 2011-2013, PostgreSQL Global Development Group
 
 This program is free software; you can redistribute it and/or modify
 it under the terms of the GNU General Public License as published by
@@ -59,10 +59,10 @@ not require any manual adjustments of the Makefile.
 
 Invocation:
 
-pg_filedump [-abcdfhixy] [-R startblock [endblock]] [-S blocksize] file
+pg_filedump [-abcdfhikxy] [-R startblock [endblock]] [-S blocksize] file
 
-Defaults are: relative addressing, range of the entire file, block size
-  as listed on block 0 in the file
+Defaults are: relative addressing, range of the entire file, block
+   

Re: [HACKERS] pg_filedump 9.3: checksums (and a few other fixes)

[Jeff Davis]

On Sat, 2013-07-06 at 10:30 +0900, Satoshi Nagayasu wrote:
 Hi,
 
 It looks fine, but I have one question here.
 
 When I run pg_filedump with -k against a database cluster which
 does not support checksums, pg_filedump produced checksum error as
 following. Is this expected or acceptable?

Thank you for taking a look. That is expected, because there is not a
good way to determine whether the file was created with checksums or
not. So, we rely on the user to supply (or not) the -k flag.

Regards,
Jeff Davis




-- 
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] pg_filedump 9.3: checksums (and a few other fixes)

[Jeff Davis]

On Thu, 2013-06-27 at 15:59 +0200, Andres Freund wrote:
 Maybe the trick is to add a recovery.conf option to make postgres replay
 to the first restartpoint and then shutdown. At that point you can be
 sure there aren't any torn pages anymore (bugs aside).
 In fact that sounds like a rather useful pg_basebackup extension...

+1

Jeff Davis




-- 
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] pg_filedump 9.3: checksums (and a few other fixes)

[Andres Freund]

On 2013-06-26 21:18:49 -0700, Peter Geoghegan wrote:
 On Wed, Jun 26, 2013 at 8:27 PM, Tom Lane t...@sss.pgh.pa.us wrote:
  TBH, I've always been annoyed that pg_filedump is GPL and so there's no
  way for us to just ship it in contrib.  (That stems from Red Hat
  corporate policy of a dozen years ago, but the conflict is real anyway.)
  If somebody is sufficiently excited about this topic to do something
  that's largely new anyway, I'd be in favor of starting from scratch so
  it could be put under the usual Postgres license.
 
 Heroku are interested in online verification of basebackups (i.e.
 using checksums to verify the integrity of heap files as they are
 backed up, with a view to relying less and less on logical backups). I
 am very glad that you made the page checksums stuff available to
 external utilities in commit f04216341dd1cc235e975f93ac806d9d3729a344.
 
 In the last couple of days, I haven't been able to figure out a way to
 solve the problem of torn pages in a way that isn't a complete kludge
 (with a hopefully-acceptable risk of false positives), so I've been
 operating under the assumption that anything I produce here won't be
 up to the standards of contrib.

Why not do this from a function/background worker in the backend where
you can go via the buffer manager to avoid torn pages et al. If you use
a buffer strategy the cache poisoning et al should be controlleable.

Greetings,

Andres Freund

-- 
 Andres Freund http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training  Services


-- 
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] pg_filedump 9.3: checksums (and a few other fixes)

[Peter Geoghegan]

On Wed, Jun 26, 2013 at 11:27 PM, Andres Freund and...@2ndquadrant.com wrote:
 Why not do this from a function/background worker in the backend where
 you can go via the buffer manager to avoid torn pages et al. If you use
 a buffer strategy the cache poisoning et al should be controlleable.

I had considered that, but thought it might be a little bit
aggressive, even with a strategy of BAS_BULKREAD. Maybe the kludge I
have in mind might not end up being that bad in practice, and would
certainly perform better than an approach that used the buffer
manager. But then, going through shared_buffers could be worth the
overhead, if only for the peace of mind of not relying on something
that is subtly broken.


-- 
Peter Geoghegan


-- 
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] pg_filedump 9.3: checksums (and a few other fixes)

[Andres Freund]

On 2013-06-26 23:42:55 -0700, Peter Geoghegan wrote:
 On Wed, Jun 26, 2013 at 11:27 PM, Andres Freund and...@2ndquadrant.com 
 wrote:
  Why not do this from a function/background worker in the backend where
  you can go via the buffer manager to avoid torn pages et al. If you use
  a buffer strategy the cache poisoning et al should be controlleable.
 
 I had considered that, but thought it might be a little bit
 aggressive, even with a strategy of BAS_BULKREAD.

Well, you can influence the pacing yourself, you don't need to rely on
the strategy for that. I'd only use it because of the ringbuffer logic
it has to avoid trashing the cache.

 Maybe the kludge I
 have in mind might not end up being that bad in practice, and would
 certainly perform better than an approach that used the buffer
 manager.

What do you have in mind then?

 But then, going through shared_buffers could be worth the
 overhead, if only for the peace of mind of not relying on something
 that is subtly broken.

Spurious alarms quickly lead to people ignoring them, consciously or
not, so trying to take care not to go there sounds like a good idea.

Greetings,

Andres Freund

-- 
 Andres Freund http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training  Services


-- 
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] pg_filedump 9.3: checksums (and a few other fixes)

[Peter Geoghegan]

On Tue, Jun 18, 2013 at 9:42 AM, Jeff Davis pg...@j-davis.com wrote:
 I'm not sure what the resolution of Alvaro's concern was, so I left the
 flag reporting the same as the previous patch.

Alvaro's concern was that the new flags added (those added by the
foreign key locks patch) do something cute with re-using multiple
other bits in an otherwise nonsensical combination to represent a
distinct state. So as written, the infoMask if statements will result
in spurious reporting of information stored in t_infomask. If you AND
some integer with HEAP_XMAX_SHR_LOCK and get something non-zero,
you'll surely also get a non-zero result with HEAP_LOCK_MASK, because
the latter flag has all the same bits set as the former (plus others,
obviously).


-- 
Peter Geoghegan


-- 
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] pg_filedump 9.3: checksums (and a few other fixes)

[Peter Geoghegan]

On Thu, Jun 27, 2013 at 12:07 AM, Peter Geoghegan p...@heroku.com wrote:
 I'm not sure what the resolution of Alvaro's concern was, so I left the
 flag reporting the same as the previous patch.

 Alvaro's concern was that the new flags added (those added by the
 foreign key locks patch) do something cute with re-using multiple
 other bits in an otherwise nonsensical combination to represent a
 distinct state.

I just realized that it wasn't that you didn't understand the nature
of the problem, but that you weren't sure of a resolution. Sorry for
the noise.

-- 
Peter Geoghegan


-- 
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] pg_filedump 9.3: checksums (and a few other fixes)

[Andres Freund]

On 2013-06-27 09:51:07 -0400, Tom Lane wrote:
 Andres Freund and...@2ndquadrant.com writes:
  On 2013-06-26 21:18:49 -0700, Peter Geoghegan wrote:
  Heroku are interested in online verification of basebackups (i.e.
  using checksums to verify the integrity of heap files as they are
  backed up, with a view to relying less and less on logical backups).
 
  Why not do this from a function/background worker in the backend where
  you can go via the buffer manager to avoid torn pages et al. If you use
  a buffer strategy the cache poisoning et al should be controlleable.
 
 That would require having a functioning postmaster environment, which
 seems like it would make such a tool much less flexible.

I personally wouldn't trust online backups that aren't proven to be
replayable into a runnable state very much. I have seen too many cases
where that failed.

Maybe the trick is to add a recovery.conf option to make postgres replay
to the first restartpoint and then shutdown. At that point you can be
sure there aren't any torn pages anymore (bugs aside).
In fact that sounds like a rather useful pg_basebackup extension...

Greetings,

Andres Freund

-- 
 Andres Freund http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training  Services


-- 
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] pg_filedump 9.3: checksums (and a few other fixes)

[Tom Lane]

Andres Freund and...@2ndquadrant.com writes:
 On 2013-06-26 21:18:49 -0700, Peter Geoghegan wrote:
 Heroku are interested in online verification of basebackups (i.e.
 using checksums to verify the integrity of heap files as they are
 backed up, with a view to relying less and less on logical backups).

 Why not do this from a function/background worker in the backend where
 you can go via the buffer manager to avoid torn pages et al. If you use
 a buffer strategy the cache poisoning et al should be controlleable.

That would require having a functioning postmaster environment, which
seems like it would make such a tool much less flexible.

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

Re: [HACKERS] pg_filedump 9.3: checksums (and a few other fixes)

[Jeff Davis]

On Mon, 2013-06-24 at 20:34 -0400, Josh Kupershmidt wrote:
 This patch is in the current CommitFest, does it still need to be
 reviewed? If so, I notice that the version in pgfoundry's CVS is
 rather different than the version the patch seems to have been built
 against (presumably the pg_filedump-9.2.0.tar.gz release), and
 conflicts in several places with cvs tip.

Rebased against CVS tip; attached.

 Also, would anyone be willing to convert this repository to git and
 post it on github or similar? pgfoundry is becoming increasingly
 difficult to use, for instance the 'Browse CVS Repository' link for
 pg_filedump and other projects is broken[1] and apparently has been
 for months[2], not to mention the general crumminess of using CVS [3].

Eventually, it would be nice to have a more full-featured offline
checker utility. Do we want to try to turn this utility into that, or
make a new one?

Regards,
Jeff Davis

Only in pg_filedump.checksums/: .deps
Only in pg_filedump.checksums/: pg_filedump
diff -rc pg_filedump/pg_filedump.c pg_filedump.checksums/pg_filedump.c
*** pg_filedump/pg_filedump.c	2013-06-06 11:33:17.0 -0700
--- pg_filedump.checksums/pg_filedump.c	2013-06-26 11:53:45.780117294 -0700
***
*** 26,31 
--- 26,38 
  
  #include utils/pg_crc_tables.h
  
+ // checksum_impl.h uses Assert, which doesn't work outside the server
+ #undef Assert
+ #define Assert(X)
+ 
+ #include storage/checksum.h
+ #include storage/checksum_impl.h
+ 
  // Global variables for ease of use mostly
  static FILE *fp = NULL;		// File to dump or format
  static char *fileName = NULL;	// File name for display
***
*** 40,51 
  static void DisplayOptions (unsigned int validOptions);
  static unsigned int ConsumeOptions (int numOptions, char **options);
  static int GetOptionValue (char *optionString);
! static void FormatBlock ();
  static unsigned int GetBlockSize ();
  static unsigned int GetSpecialSectionType (Page page);
  static bool IsBtreeMetaPage(Page page);
  static void CreateDumpFileHeader (int numOptions, char **options);
! static int FormatHeader (Page page);
  static void FormatItemBlock (Page page);
  static void FormatItem (unsigned int numBytes, unsigned int startIndex,
  			unsigned int formatAs);
--- 47,58 
  static void DisplayOptions (unsigned int validOptions);
  static unsigned int ConsumeOptions (int numOptions, char **options);
  static int GetOptionValue (char *optionString);
! static void FormatBlock (BlockNumber blkno);
  static unsigned int GetBlockSize ();
  static unsigned int GetSpecialSectionType (Page page);
  static bool IsBtreeMetaPage(Page page);
  static void CreateDumpFileHeader (int numOptions, char **options);
! static int FormatHeader (Page page, BlockNumber blkno);
  static void FormatItemBlock (Page page);
  static void FormatItem (unsigned int numBytes, unsigned int startIndex,
  			unsigned int formatAs);
***
*** 288,293 
--- 295,305 
  		  SET_OPTION (itemOptions, ITEM_DETAIL, 'i');
  		  break;
  
+ 		  // Verify block checksums
+ 		case 'k':
+ 		  SET_OPTION (blockOptions, BLOCK_CHECKSUMS, 'k');
+ 		  break;
+ 
  		  // Interpret items as standard index values
  		case 'x':
  		  SET_OPTION (itemOptions, ITEM_INDEX, 'x');
***
*** 555,561 
  
  // Dump out a formatted block header for the requested block
  static int
! FormatHeader (Page page)
  {
int rc = 0;
unsigned int headerBytes;
--- 567,573 
  
  // Dump out a formatted block header for the requested block
  static int
! FormatHeader (Page page, BlockNumber blkno)
  {
int rc = 0;
unsigned int headerBytes;
***
*** 647,652 
--- 659,672 
  	  || (pageHeader-pd_upper  pageHeader-pd_lower)
  	  || (pageHeader-pd_special  blockSize))
  	printf ( Error: Invalid header information.\n\n);
+ 
+   if (blockOptions  BLOCK_CHECKSUMS)
+ 	{
+ 	  uint16 calc_checksum = pg_checksum_page(page, blkno);
+ 	  if (calc_checksum != pageHeader-pd_checksum)
+ 	printf( Error: checksum failure: calculated 0x%04x.\n\n,
+ 		   calc_checksum);
+ 	}
  }
  
// If we have reached the end of file while interpreting the header, let
***
*** 1208,1214 
  
  // For each block, dump out formatted header and content information
  static void
! FormatBlock ()
  {
Page page = (Page) buffer;
pageOffset = blockSize * currentBlock;
--- 1228,1234 
  
  // For each block, dump out formatted header and content information
  static void
! FormatBlock (BlockNumber blkno)
  {
Page page = (Page) buffer;
pageOffset = blockSize * currentBlock;
***
*** 1228,1234 
int rc;
// Every block contains a header, items and possibly a special
// section.  Beware of partial block reads though
!   rc = FormatHeader (page);
  
// If we didn't encounter a partial read in the header, carry on...
if (rc != EOF_ENCOUNTERED)
--- 1248,1254 
int rc;
  

Re: [HACKERS] pg_filedump 9.3: checksums (and a few other fixes)

[Tom Lane]

Jeff Davis pg...@j-davis.com writes:
 On Mon, 2013-06-24 at 20:34 -0400, Josh Kupershmidt wrote:
 This patch is in the current CommitFest, does it still need to be
 reviewed? If so, I notice that the version in pgfoundry's CVS is
 rather different than the version the patch seems to have been built
 against (presumably the pg_filedump-9.2.0.tar.gz release), and
 conflicts in several places with cvs tip.

 Rebased against CVS tip; attached.

Thanks.  I'm feeling pretty overworked these days but will try to push
this into pgfoundry in a timely fashion.

 Eventually, it would be nice to have a more full-featured offline
 checker utility. Do we want to try to turn this utility into that, or
 make a new one?

TBH, I've always been annoyed that pg_filedump is GPL and so there's no
way for us to just ship it in contrib.  (That stems from Red Hat
corporate policy of a dozen years ago, but the conflict is real anyway.)
If somebody is sufficiently excited about this topic to do something
that's largely new anyway, I'd be in favor of starting from scratch so
it could be put under the usual Postgres license.

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

Re: [HACKERS] pg_filedump 9.3: checksums (and a few other fixes)

[Peter Geoghegan]

On Wed, Jun 26, 2013 at 8:27 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 TBH, I've always been annoyed that pg_filedump is GPL and so there's no
 way for us to just ship it in contrib.  (That stems from Red Hat
 corporate policy of a dozen years ago, but the conflict is real anyway.)
 If somebody is sufficiently excited about this topic to do something
 that's largely new anyway, I'd be in favor of starting from scratch so
 it could be put under the usual Postgres license.

Heroku are interested in online verification of basebackups (i.e.
using checksums to verify the integrity of heap files as they are
backed up, with a view to relying less and less on logical backups). I
am very glad that you made the page checksums stuff available to
external utilities in commit f04216341dd1cc235e975f93ac806d9d3729a344.

In the last couple of days, I haven't been able to figure out a way to
solve the problem of torn pages in a way that isn't a complete kludge
(with a hopefully-acceptable risk of false positives), so I've been
operating under the assumption that anything I produce here won't be
up to the standards of contrib. I had intended to release whatever
results as an open source project anyway. However, if we can figure
out a way to solve the torn pages problem, or at least produce
something acceptable, I think I'd certainly be able to find the time
to work on a contrib module that is mainly concerned with verifying
basebackups, but also offers some pg_filedump-like functionality.
That's something largely new.

-- 
Peter Geoghegan


-- 
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] pg_filedump 9.3: checksums (and a few other fixes)

[Jeff Davis]

On Mon, 2013-06-24 at 20:34 -0400, Josh Kupershmidt wrote:
 This patch is in the current CommitFest, does it still need to be
 reviewed? If so, I notice that the version in pgfoundry's CVS is
 rather different than the version the patch seems to have been built
 against (presumably the pg_filedump-9.2.0.tar.gz release), and
 conflicts in several places with cvs tip.

Oh, thank you. After browsing the CVS repo failed, I just made the diff
against 9.2.0. I'll rebase it.

Regards,
Jeff Davis




-- 
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] pg_filedump 9.3: checksums (and a few other fixes)

[Josh Kupershmidt]

On Tue, Jun 18, 2013 at 12:42 PM, Jeff Davis pg...@j-davis.com wrote:
 Attached a new diff for pg_filedump that makes use of the above change.

 I'm not sure what the resolution of Alvaro's concern was, so I left the
 flag reporting the same as the previous patch.

This patch is in the current CommitFest, does it still need to be
reviewed? If so, I notice that the version in pgfoundry's CVS is
rather different than the version the patch seems to have been built
against (presumably the pg_filedump-9.2.0.tar.gz release), and
conflicts in several places with cvs tip.

Also, would anyone be willing to convert this repository to git and
post it on github or similar? pgfoundry is becoming increasingly
difficult to use, for instance the 'Browse CVS Repository' link for
pg_filedump and other projects is broken[1] and apparently has been
for months[2], not to mention the general crumminess of using CVS [3].

Josh

[1] http://pgfoundry.org/scm/browser.php?group_id=1000541
[2] 
http://pgfoundry.org/forum/forum.php?thread_id=15554forum_id=44group_id=113
[3] Since the pgfoundry cvs server apparently doesn't support the 'ls'
command, you might need to know this to know which module name to
check out: cvs -d
:pserver:anonym...@cvs.pgfoundry.org:/cvsroot/pgfiledump checkout
pg_filedump


-- 
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] pg_filedump 9.3: checksums (and a few other fixes)

[Tom Lane]

Josh Kupershmidt schmi...@gmail.com writes:
 This patch is in the current CommitFest, does it still need to be
 reviewed? If so, I notice that the version in pgfoundry's CVS is
 rather different than the version the patch seems to have been built
 against (presumably the pg_filedump-9.2.0.tar.gz release), and
 conflicts in several places with cvs tip.

Yeah, I pushed some basic 9.3 compatibility fixes into pg_filedump CVS
a few weeks back.  If someone could rebase the patch against that,
I'd appreciate it.

 Also, would anyone be willing to convert this repository to git and
 post it on github or similar?

I know it's time to get off of pgfoundry, but doing so for pg_filedump
is way down the priority list ...

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

Re: [HACKERS] pg_filedump 9.3: checksums (and a few other fixes)

[Jeff Davis]

On Thu, 2013-06-13 at 20:09 -0400, Tom Lane wrote:
 What I propose we do about this is reduce backend/storage/page/checksum.c
 to something like
 
 #include postgres.h
 #include storage/checksum.h
 #include storage/checksum_impl.h

Attached a new diff for pg_filedump that makes use of the above change.

I'm not sure what the resolution of Alvaro's concern was, so I left the
flag reporting the same as the previous patch.

Regards,
Jeff Davis

Common subdirectories: pg_filedump-9.2.0/.deps and pg_filedump-9.3.0j/.deps
diff -Nc pg_filedump-9.2.0/Makefile pg_filedump-9.3.0j/Makefile
*** pg_filedump-9.2.0/Makefile	2012-03-12 09:02:44.0 -0700
--- pg_filedump-9.3.0j/Makefile	2013-06-18 09:14:42.442220848 -0700
***
*** 1,7 
  # View README.pg_filedump first
  
  # note this must match version macros in pg_filedump.h
! FD_VERSION=9.2.0
  
  CC=gcc
  CFLAGS=-g -O -Wall -Wmissing-prototypes -Wmissing-declarations
--- 1,7 
  # View README.pg_filedump first
  
  # note this must match version macros in pg_filedump.h
! FD_VERSION=9.3.0
  
  CC=gcc
  CFLAGS=-g -O -Wall -Wmissing-prototypes -Wmissing-declarations
diff -Nc pg_filedump-9.2.0/pg_filedump.c pg_filedump-9.3.0j/pg_filedump.c
*** pg_filedump-9.2.0/pg_filedump.c	2012-03-12 08:58:31.0 -0700
--- pg_filedump-9.3.0j/pg_filedump.c	2013-06-18 09:25:42.438208300 -0700
***
*** 26,31 
--- 26,37 
  
  #include utils/pg_crc_tables.h
  
+ // checksum_impl.h uses Assert, which doesn't work outside the server
+ #undef Assert
+ #define Assert(X)
+ 
+ #include storage/checksum_impl.h
+ 
  // Global variables for ease of use mostly
  static FILE *fp = NULL;		// File to dump or format
  static char *fileName = NULL;	// File name for display
***
*** 40,51 
  static void DisplayOptions (unsigned int validOptions);
  static unsigned int ConsumeOptions (int numOptions, char **options);
  static int GetOptionValue (char *optionString);
! static void FormatBlock ();
  static unsigned int GetBlockSize ();
  static unsigned int GetSpecialSectionType (Page page);
  static bool IsBtreeMetaPage(Page page);
  static void CreateDumpFileHeader (int numOptions, char **options);
! static int FormatHeader (Page page);
  static void FormatItemBlock (Page page);
  static void FormatItem (unsigned int numBytes, unsigned int startIndex,
  			unsigned int formatAs);
--- 46,57 
  static void DisplayOptions (unsigned int validOptions);
  static unsigned int ConsumeOptions (int numOptions, char **options);
  static int GetOptionValue (char *optionString);
! static void FormatBlock (BlockNumber blkno);
  static unsigned int GetBlockSize ();
  static unsigned int GetSpecialSectionType (Page page);
  static bool IsBtreeMetaPage(Page page);
  static void CreateDumpFileHeader (int numOptions, char **options);
! static int FormatHeader (Page page, BlockNumber blkno);
  static void FormatItemBlock (Page page);
  static void FormatItem (unsigned int numBytes, unsigned int startIndex,
  			unsigned int formatAs);
***
*** 288,293 
--- 294,304 
  		  SET_OPTION (itemOptions, ITEM_DETAIL, 'i');
  		  break;
  
+ 		  // Verify block checksums
+ 		case 'k':
+ 		  SET_OPTION (blockOptions, BLOCK_CHECKSUMS, 'k');
+ 		  break;
+ 
  		  // Interpret items as standard index values
  		case 'x':
  		  SET_OPTION (itemOptions, ITEM_INDEX, 'x');
***
*** 555,561 
  
  // Dump out a formatted block header for the requested block
  static int
! FormatHeader (Page page)
  {
int rc = 0;
unsigned int headerBytes;
--- 566,572 
  
  // Dump out a formatted block header for the requested block
  static int
! FormatHeader (Page page, BlockNumber blkno)
  {
int rc = 0;
unsigned int headerBytes;
***
*** 609,623 
  	  Block: Size %4d  Version %4uUpper%4u (0x%04hx)\n
  	  LSN:  logid %6d recoff 0x%08x  Special  %4u (0x%04hx)\n
  	  Items: %4d  Free Space: %4u\n
! 	  TLI: 0x%04x  Prune XID: 0x%08x  Flags: 0x%04x (%s)\n
  	  Length (including item array): %u\n\n,
  	 pageOffset, pageHeader-pd_lower, pageHeader-pd_lower,
  	 (int) PageGetPageSize (page), blockVersion,
  	 pageHeader-pd_upper, pageHeader-pd_upper,
! 	 pageLSN.xlogid, pageLSN.xrecoff,
  	 pageHeader-pd_special, pageHeader-pd_special,
  	 maxOffset, pageHeader-pd_upper - pageHeader-pd_lower,
! 	 pageHeader-pd_tli, pageHeader-pd_prune_xid,
  	 pageHeader-pd_flags, flagString,
  	 headerBytes);
  
--- 620,634 
  	  Block: Size %4d  Version %4uUpper%4u (0x%04hx)\n
  	  LSN:  logid %6d recoff 0x%08x  Special  %4u (0x%04hx)\n
  	  Items: %4d  Free Space: %4u\n
! 	  Checksum: %05hu  Prune XID: 0x%08x  Flags: 0x%04x (%s)\n
  	  Length (including item array): %u\n\n,
  	 pageOffset, pageHeader-pd_lower, pageHeader-pd_lower,
  	 (int) PageGetPageSize (page), blockVersion,
  	 pageHeader-pd_upper, pageHeader-pd_upper,
! 	 (uint32) (pageLSN  32), 

Re: [HACKERS] pg_filedump 9.3: checksums (and a few other fixes)

[Jeff Davis]

On Thu, 2013-06-13 at 20:09 -0400, Tom Lane wrote:
 What I propose we do about this is reduce backend/storage/page/checksum.c
 to something like
 
 #include postgres.h
 #include storage/checksum.h
 #include storage/checksum_impl.h
 
 moving all the code currently in the file into a new .h file.  Then,
 any external programs such as pg_filedump can use the checksum code
 by including checksum_impl.h.  This is essentially the same thing we
 did with the CRC support functionality some time ago.

Thank you for taking care of that. After seeing that it needed to be in
a header file, I was going to try doing it all as macros.

I have a question about the commit though: shouldn't both functions be
static if they are in a .h file? Otherwise, it could lead to naming
conflicts. I suppose it's wrong to include the implementation file
twice, but it still might be confusing if someone tries. Two ideas that
come to mind are:
  * make both static and then have a trivial wrapper in checksum.c
  * export one or both functions, but use #ifndef CHECKSUM_IMPL_H to
prevent redefinition

 Also, we have the cut-point between checksum.c and bufpage.c at the
 wrong place.  IMO we should move PageCalcChecksum16 in toto into
 checksum.c (or really now into checksum_impl.h), because that and not
 just checksum_block() is the functionality that is wanted.

Agreed.

Regards,
Jeff Davis




-- 
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] pg_filedump 9.3: checksums (and a few other fixes)

[Tom Lane]

Jeff Davis pg...@j-davis.com writes:
 I have a question about the commit though: shouldn't both functions be
 static if they are in a .h file? Otherwise, it could lead to naming
 conflicts. I suppose it's wrong to include the implementation file
 twice, but it still might be confusing if someone tries. Two ideas that
 come to mind are:
   * make both static and then have a trivial wrapper in checksum.c
   * export one or both functions, but use #ifndef CHECKSUM_IMPL_H to
 prevent redefinition

Ah, you are right, I forgot the #ifndef CHECKSUM_IMPL_H dance.  Will fix
in a bit.

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

Re: [HACKERS] pg_filedump 9.3: checksums (and a few other fixes)

[Andres Freund]

On 2013-06-14 11:59:04 -0400, Tom Lane wrote:
 Jeff Davis pg...@j-davis.com writes:
  I have a question about the commit though: shouldn't both functions be
  static if they are in a .h file? Otherwise, it could lead to naming
  conflicts. I suppose it's wrong to include the implementation file
  twice, but it still might be confusing if someone tries. Two ideas that
  come to mind are:
* make both static and then have a trivial wrapper in checksum.c
* export one or both functions, but use #ifndef CHECKSUM_IMPL_H to
  prevent redefinition
 
 Ah, you are right, I forgot the #ifndef CHECKSUM_IMPL_H dance.  Will fix
 in a bit.

That won't help against errors if it's included in two different
files/translation units though. I don't really see a valid case where it
could be validly be included multiple times in one TU?
If anything we should #error in that case, but I am not sure it's worth
bothering.
E.g. in rmgrlist.h we have the following comment:
/* there is deliberately not an #ifndef RMGRLIST_H here */
and I think the reasoning behind that comment applies here as well.

Greetings,

Andres Freund

-- 
 Andres Freund http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training  Services


-- 
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] pg_filedump 9.3: checksums (and a few other fixes)

[Tom Lane]

Andres Freund and...@2ndquadrant.com writes:
 On 2013-06-14 11:59:04 -0400, Tom Lane wrote:
 Ah, you are right, I forgot the #ifndef CHECKSUM_IMPL_H dance.  Will fix
 in a bit.

 That won't help against errors if it's included in two different
 files/translation units though.

Good point, but there's not any real reason to do that --- only
checksum.h should ever be #include'd in more than one file.  Any program
using this stuff is expected to #include checksum_impl.h in exactly one
place.  So maybe it's fine as-is.

 E.g. in rmgrlist.h we have the following comment:
 /* there is deliberately not an #ifndef RMGRLIST_H here */
 and I think the reasoning behind that comment applies here as well.

Well, that's a different case: there, and also in kwlist.h, there's an
idea that it could actually be useful to #include the file more than
once, redefining the PG_RMGR() macro each time.  There's no such use
case that I can see for checksum_impl.h.

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

Re: [HACKERS] pg_filedump 9.3: checksums (and a few other fixes)

[Tom Lane]

Jeff Davis pg...@j-davis.com writes:
 The patch is a bit ugly: I had to copy some code, and copy the entire
 checksum.c file (minus some Asserts, which don't work in an external
 program). Suggestions welcome.

What I propose we do about this is reduce backend/storage/page/checksum.c
to something like

#include postgres.h
#include storage/checksum.h
#include storage/checksum_impl.h

moving all the code currently in the file into a new .h file.  Then,
any external programs such as pg_filedump can use the checksum code
by including checksum_impl.h.  This is essentially the same thing we
did with the CRC support functionality some time ago.

Also, we have the cut-point between checksum.c and bufpage.c at the
wrong place.  IMO we should move PageCalcChecksum16 in toto into
checksum.c (or really now into checksum_impl.h), because that and not
just checksum_block() is the functionality that is wanted.

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

Re: [HACKERS] pg_filedump 9.3: checksums (and a few other fixes)

[Jeff Davis]

On Mon, 2013-06-10 at 01:28 -0400, Alvaro Herrera wrote:
 Hm, note that XMAX_SHR_LOCK is two bits, so when that flag is present
 you will get the three lock modes displayed with the above code, which is
 probably going to be misleading.  htup_details.h does this:
 
 /*
  * Use these to test whether a particular lock is applied to a tuple
  */
 #define HEAP_XMAX_IS_SHR_LOCKED(infomask) \
   (((infomask)  HEAP_LOCK_MASK) == HEAP_XMAX_SHR_LOCK)
 #define HEAP_XMAX_IS_EXCL_LOCKED(infomask) \
   (((infomask)  HEAP_LOCK_MASK) == HEAP_XMAX_EXCL_LOCK)
 #define HEAP_XMAX_IS_KEYSHR_LOCKED(infomask) \
   (((infomask)  HEAP_LOCK_MASK) == HEAP_XMAX_KEYSHR_LOCK)
 
 Presumably it'd be better to do something similar.

I was hesitant to do too much interpretation of the bits. Do you think
it would be better to just remove the test for XMAX_SHR_LOCK?

Regards,
Jeff Davis




-- 
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] pg_filedump 9.3: checksums (and a few other fixes)

[Alvaro Herrera]

Jeff Davis wrote:

 I was hesitant to do too much interpretation of the bits. Do you think
 it would be better to just remove the test for XMAX_SHR_LOCK?

I don't know, but then I'm biased because I know what that specific bit
combination means.  I guess someone that doesn't know is going to be
surprised by seeing both lock strength bits together .. but maybe
they're just going to have a look at htup_details.h and instantly
understand what's going on.  Not sure how likely that is.

-- 
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training  Services


-- 
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] pg_filedump 9.3: checksums (and a few other fixes)

[Tom Lane]

Alvaro Herrera alvhe...@2ndquadrant.com writes:
 Jeff Davis wrote:
 I was hesitant to do too much interpretation of the bits. Do you think
 it would be better to just remove the test for XMAX_SHR_LOCK?

 I don't know, but then I'm biased because I know what that specific bit
 combination means.  I guess someone that doesn't know is going to be
 surprised by seeing both lock strength bits together .. but maybe
 they're just going to have a look at htup_details.h and instantly
 understand what's going on.  Not sure how likely that is.

I think it's all right because there are only 4 combinations of the two
bits and all 4 will be printed sensibly if we do it this way.  It would
be bad if pg_filedump could print invalid flag combinations in a
misleading way --- but I don't see such a risk here.  So we might as
well go for readability.

The thing I'm not too happy about is having to copy the checksum code
into pg_filedump.  We just got rid of the need to do that for the CRC
code, and here it is coming back again.  Can't we rearrange the core
checksum code similarly to what we did for the CRC stuff recently,
so that you only need access to a .h file for it?

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

Re: [HACKERS] pg_filedump 9.3: checksums (and a few other fixes)

[Jeff Davis]

On Mon, 2013-06-10 at 11:38 -0400, Tom Lane wrote:
 The thing I'm not too happy about is having to copy the checksum code
 into pg_filedump.  We just got rid of the need to do that for the CRC
 code, and here it is coming back again.  Can't we rearrange the core
 checksum code similarly to what we did for the CRC stuff recently,
 so that you only need access to a .h file for it?

The CRC implementation is entirely in header files. Do you think we need
to go that far, or is it fine to just put it in libpgport and link that
to pg_filedump?

Regards,
Jeff Davis




-- 
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] pg_filedump 9.3: checksums (and a few other fixes)

[Alvaro Herrera]

Jeff Davis wrote:
 On Mon, 2013-06-10 at 11:38 -0400, Tom Lane wrote:
  The thing I'm not too happy about is having to copy the checksum code
  into pg_filedump.  We just got rid of the need to do that for the CRC
  code, and here it is coming back again.  Can't we rearrange the core
  checksum code similarly to what we did for the CRC stuff recently,
  so that you only need access to a .h file for it?
 
 The CRC implementation is entirely in header files. Do you think we need
 to go that far, or is it fine to just put it in libpgport and link that
 to pg_filedump?

If a lib is okay, use libpgcommon please, not libpgport.  But I think a
.h would be better, because there'd be no need to have a built source
tree to build pg_filedump, only the headers installed.

-- 
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training  Services


-- 
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] pg_filedump 9.3: checksums (and a few other fixes)

[Tom Lane]

Alvaro Herrera alvhe...@2ndquadrant.com writes:
 Jeff Davis wrote:
 The CRC implementation is entirely in header files. Do you think we need
 to go that far, or is it fine to just put it in libpgport and link that
 to pg_filedump?

 If a lib is okay, use libpgcommon please, not libpgport.  But I think a
 .h would be better, because there'd be no need to have a built source
 tree to build pg_filedump, only the headers installed.

Neither lib would provide a useful solution so far as Red Hat's
packaging is concerned, because those libs are not exposed to other
packages (and never will be, unless we start supplying them as .so's)

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] pg_filedump 9.3: checksums (and a few other fixes)

[Jeff Davis]

Attached is a first draft of an update to pg_filedump for 9.3. I know
pg_filedump is a pgfoundry project, but that seems like it's just there
to host the download; so please excuse the slightly off-topic post here
on -hackers.

I made a few changes to support 9.3, which were mostly fixes related two
things:

 * new htup_details.h and changes related to FK concurrency improvements
 * XLogRecPtr is now a uint64

And, of course, I added support for checksums. They are always displayed
and calculated, but it only throws an error if you pass -k. Only the
user knows whether checksums are enabled, because we removed page-level
bits indicating the presence of a checksum.

The patch is a bit ugly: I had to copy some code, and copy the entire
checksum.c file (minus some Asserts, which don't work in an external
program). Suggestions welcome.

Regards,
Jeff Davis

diff -Nc pg_filedump-9.2.0/checksum.c pg_filedump-9.3.0j/checksum.c
*** pg_filedump-9.2.0/checksum.c	1969-12-31 16:00:00.0 -0800
--- pg_filedump-9.3.0j/checksum.c	2013-06-09 21:20:34.036176831 -0700
***
*** 0 
--- 1,157 
+ /*-
+  *
+  * checksum.c
+  *	  Checksum implementation for data pages.
+  *
+  * Portions Copyright (c) 1996-2013, PostgreSQL Global Development Group
+  * Portions Copyright (c) 1994, Regents of the University of California
+  *
+  *
+  * IDENTIFICATION
+  *	  src/backend/storage/page/checksum.c
+  *
+  *-
+  *
+  * Checksum algorithm
+  *
+  * The algorithm used to checksum pages is chosen for very fast calculation.
+  * Workloads where the database working set fits into OS file cache but not
+  * into shared buffers can read in pages at a very fast pace and the checksum
+  * algorithm itself can become the largest bottleneck.
+  *
+  * The checksum algorithm itself is based on the FNV-1a hash (FNV is shorthand
+  * for Fowler/Noll/Vo) The primitive of a plain FNV-1a hash folds in data 1
+  * byte at a time according to the formula:
+  *
+  * hash = (hash ^ value) * FNV_PRIME
+  *
+  * FNV-1a algorithm is described at http://www.isthe.com/chongo/tech/comp/fnv/
+  *
+  * PostgreSQL doesn't use FNV-1a hash directly because it has bad mixing of
+  * high bits - high order bits in input data only affect high order bits in
+  * output data. To resolve this we xor in the value prior to multiplication
+  * shifted right by 17 bits. The number 17 was chosen because it doesn't
+  * have common denominator with set bit positions in FNV_PRIME and empirically
+  * provides the fastest mixing for high order bits of final iterations quickly
+  * avalanche into lower positions. For performance reasons we choose to combine
+  * 4 bytes at a time. The actual hash formula used as the basis is:
+  *
+  * hash = (hash ^ value) * FNV_PRIME ^ ((hash ^ value)  17)
+  *
+  * The main bottleneck in this calculation is the multiplication latency. To
+  * hide the latency and to make use of SIMD parallelism multiple hash values
+  * are calculated in parallel. The page is treated as a 32 column two
+  * dimensional array of 32 bit values. Each column is aggregated separately
+  * into a partial checksum. Each partial checksum uses a different initial
+  * value (offset basis in FNV terminology). The initial values actually used
+  * were chosen randomly, as the values themselves don't matter as much as that
+  * they are different and don't match anything in real data. After initializing
+  * partial checksums each value in the column is aggregated according to the
+  * above formula. Finally two more iterations of the formula are performed with
+  * value 0 to mix the bits of the last value added.
+  *
+  * The partial checksums are then folded together using xor to form a single
+  * 32-bit checksum. The caller can safely reduce the value to 16 bits
+  * using modulo 2^16-1. That will cause a very slight bias towards lower
+  * values but this is not significant for the performance of the
+  * checksum.
+  *
+  * The algorithm choice was based on what instructions are available in SIMD
+  * instruction sets. This meant that a fast and good algorithm needed to use
+  * multiplication as the main mixing operator. The simplest multiplication
+  * based checksum primitive is the one used by FNV. The prime used is chosen
+  * for good dispersion of values. It has no known simple patterns that result
+  * in collisions. Test of 5-bit differentials of the primitive over 64bit keys
+  * reveals no differentials with 3 or more values out of 10 random keys
+  * colliding. Avalanche test shows that only high order bits of the last word
+  * have a bias. Tests of 1-4 uncorrelated bit errors, stray 0 and 0xFF bytes,
+  * overwriting page from random position to end with 0 bytes, and overwriting
+  * random segments of page with 0x00, 0xFF and random data all show optimal
+  * 2e-16 

Re: [HACKERS] pg_filedump 9.3: checksums (and a few other fixes)

[Alvaro Herrera]

Jeff Davis wrote:

 --- 1000,1015 
   strcat (flagString, HASEXTERNAL|);
 if (infoMask  HEAP_HASOID)
   strcat (flagString, HASOID|);
 +   if (infoMask  HEAP_XMAX_KEYSHR_LOCK)
 + strcat (flagString, XMAX_KEYSHR_LOCK|);
 if (infoMask  HEAP_COMBOCID)
   strcat (flagString, COMBOCID|);
 if (infoMask  HEAP_XMAX_EXCL_LOCK)
   strcat (flagString, XMAX_EXCL_LOCK|);
 !   if (infoMask  HEAP_XMAX_SHR_LOCK)
 ! strcat (flagString, XMAX_SHR_LOCK|);
 !   if (infoMask  HEAP_XMAX_LOCK_ONLY)
 ! strcat (flagString, XMAX_LOCK_ONLY|);
 if (infoMask  HEAP_XMIN_COMMITTED)
   strcat (flagString, XMIN_COMMITTED|);
 if (infoMask  HEAP_XMIN_INVALID)

Hm, note that XMAX_SHR_LOCK is two bits, so when that flag is present
you will get the three lock modes displayed with the above code, which is
probably going to be misleading.  htup_details.h does this:

/*
 * Use these to test whether a particular lock is applied to a tuple
 */
#define HEAP_XMAX_IS_SHR_LOCKED(infomask) \
(((infomask)  HEAP_LOCK_MASK) == HEAP_XMAX_SHR_LOCK)
#define HEAP_XMAX_IS_EXCL_LOCKED(infomask) \
(((infomask)  HEAP_LOCK_MASK) == HEAP_XMAX_EXCL_LOCK)
#define HEAP_XMAX_IS_KEYSHR_LOCKED(infomask) \
(((infomask)  HEAP_LOCK_MASK) == HEAP_XMAX_KEYSHR_LOCK)

Presumably it'd be better to do something similar.

-- 
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training  Services


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

[HACKERS] pg_filedump improvements

[Tom Lane]

Alvaro Herrera alvhe...@commandprompt.com writes:
 Also, what do you think about adding the ability to dump pg_filenode.map
 files?  Do you think it belongs in pg_filedump, or should we look at
 doing that elsewhere?

Not sure.  It does already contain the ability to dump pg_control, but
that seems like rather a wart (not to mention redundant with
pg_controldata) because you have to explicitly say -c to get it to
realize that's what you want.  Map files would have to be another
special switch I think.

 (Hmm, I just realized we might also conceivably
 want to dump VM and FSM forks, too)

Yeah, that's been on the to-do list for awhile.  There's not a way that
pg_filedump could automatically recognize such files, is there?

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

Re: [HACKERS] pg_filedump moved to pgfoundry

[Mark Kirkwood]

On 18/01/11 18:04, Tom Lane wrote:

David Fetterda...@fetter.org  writes:

Who's the copyright holder(s)?  If it's all individual contributors,
Red Hat policy is not in play.

Sorry David, it was written on the company's dime.



However, I doubt that Red Hat derives any value from this useful product 
being excluded from contrib by the choice of license - would they be 
receptive to the idea that it would be free marketing to have it in the 
main tarball/rpm/deb (etc) with merely a decision to change it GPL-BSD?


regards

Mark

--
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] pg_filedump moved to pgfoundry

[David Fetter]

On Tue, Jan 18, 2011 at 09:14:41PM +1300, Mark Kirkwood wrote:
 On 18/01/11 18:04, Tom Lane wrote:
 David Fetterda...@fetter.org  writes:
 Who's the copyright holder(s)?  If it's all individual
 contributors, Red Hat policy is not in play.
 Sorry David, it was written on the company's dime.
 
 However, I doubt that Red Hat derives any value from this useful
 product being excluded from contrib by the choice of license - would
 they be receptive to the idea that it would be free marketing to
 have it in the main tarball/rpm/deb (etc) with merely a decision to
 change it GPL-BSD?

I'm guessing there's a Policy® at Red Hat that software made on its
dime be GPL (v2, I'd guess), and that getting an exception would
involve convening its board or similarly drastic action.

Is that about right?

Cheers,
David.
-- 
David Fetter da...@fetter.org http://fetter.org/
Phone: +1 415 235 3778  AIM: dfetter666  Yahoo!: dfetter
Skype: davidfetter  XMPP: david.fet...@gmail.com
iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate

-- 
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] pg_filedump moved to pgfoundry

[Tom Lane]

David Fetter da...@fetter.org writes:
 I'm guessing there's a Policy® at Red Hat that software made on its
 dime be GPL (v2, I'd guess), and that getting an exception would
 involve convening its board or similarly drastic action.

It's company policy, and while it *might* be possible to get an
exception, the effort involved would far exceed the benefit we'd get out
of it.  Moreover, despite Mark's creative argument, I really doubt that
Red Hat would perceive any benefit to themselves in making an exception.

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

Re: [HACKERS] pg_filedump moved to pgfoundry

[Robert Haas]

On Tue, Jan 18, 2011 at 10:53 AM, Tom Lane t...@sss.pgh.pa.us wrote:
 David Fetter da...@fetter.org writes:
 I'm guessing there's a PolicyŽ at Red Hat that software made on its
 dime be GPL (v2, I'd guess), and that getting an exception would
 involve convening its board or similarly drastic action.

 It's company policy, and while it *might* be possible to get an
 exception, the effort involved would far exceed the benefit we'd get out
 of it.  Moreover, despite Mark's creative argument, I really doubt that
 Red Hat would perceive any benefit to themselves in making an exception.

I'm not sure why they'd care, but it certainly doesn't seem worth
spending the amount of time arguing about it that we are.  David and
Mark are, of course, free to spend their time petitioning Red Hat for
relicensing if they are so inclined, but they aren't entitled to tell
you how to spend yours.  And even if they were, I would hope that
they'd want you to spend it committing patches rather than arguing
with your employer about relicensing of a utility that's freely
available anyway and of use to 0.1% of our user base.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

-- 
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] pg_filedump moved to pgfoundry

[Cédric Villemain]

2011/1/18 Robert Haas robertmh...@gmail.com:
 On Tue, Jan 18, 2011 at 10:53 AM, Tom Lane t...@sss.pgh.pa.us wrote:
 David Fetter da...@fetter.org writes:
 I'm guessing there's a PolicyŽ at Red Hat that software made on its
 dime be GPL (v2, I'd guess), and that getting an exception would
 involve convening its board or similarly drastic action.

 It's company policy, and while it *might* be possible to get an
 exception, the effort involved would far exceed the benefit we'd get out
 of it.  Moreover, despite Mark's creative argument, I really doubt that
 Red Hat would perceive any benefit to themselves in making an exception.

 I'm not sure why they'd care, but it certainly doesn't seem worth
 spending the amount of time arguing about it that we are.  David and
 Mark are, of course, free to spend their time petitioning Red Hat for
 relicensing if they are so inclined, but they aren't entitled to tell
 you how to spend yours.  And even if they were, I would hope that
 they'd want you to spend it committing patches rather than arguing
 with your employer about relicensing of a utility that's freely
 available anyway and of use to 0.1% of our user base.


still good, thanks Tom and RH to have push it nearest other PostgreSQL. tools.



-- 
Cédric Villemain               2ndQuadrant
http://2ndQuadrant.fr/     PostgreSQL : Expertise, Formation et Support

-- 
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] pg_filedump moved to pgfoundry

[Mark Kirkwood]

On 19/01/11 05:51, Robert Haas wrote:


I'm not sure why they'd care, but it certainly doesn't seem worth
spending the amount of time arguing about it that we are.  David and
Mark are, of course, free to spend their time petitioning Red Hat for
relicensing if they are so inclined, but they aren't entitled to tell
you how to spend yours.  And even if they were, I would hope that
they'd want you to spend it committing patches rather than arguing
with your employer about relicensing of a utility that's freely
available anyway and of use to 0.1% of our user base.



Funny how people can read an email and derive a completely different 
meaning from what you intended - just to be clear: it certainly wasn't 
my intention to tell anyone how to spend their time.


Anyway, good to have pg_filedump on pgfoundry, thanks Tom and RH.

regards

Mark




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

[HACKERS] pg_filedump moved to pgfoundry

[Tom Lane]

I've gotten permission to move pg_filedump from its former home at
sources.redhat.com to pgfoundry.  You can find the historical release
tarballs as well as current sources at
http://pgfoundry.org/projects/pgfiledump/

One advantage of doing this is it will be a lot easier to let other
folks join in the fun of hacking it.  If anyone has been harboring
a yen to improve pg_filedump, please join that pgfoundry project.

(Before someone suggests folding it into contrib/: we can't because
of license issues.  pg_filedump is GPL, per Red Hat company policy,
and that's not going to change.)

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

Re: [HACKERS] pg_filedump moved to pgfoundry

[David Fetter]

On Mon, Jan 17, 2011 at 09:48:58PM -0500, Tom Lane wrote:
 I've gotten permission to move pg_filedump from its former home at
 sources.redhat.com to pgfoundry.  You can find the historical release
 tarballs as well as current sources at
   http://pgfoundry.org/projects/pgfiledump/
 
 One advantage of doing this is it will be a lot easier to let other
 folks join in the fun of hacking it.  If anyone has been harboring
 a yen to improve pg_filedump, please join that pgfoundry project.
 
 (Before someone suggests folding it into contrib/: we can't because
 of license issues.  pg_filedump is GPL, per Red Hat company policy,
 and that's not going to change.)

Who's the copyright holder(s)?  If it's all individual contributors,
Red Hat policy is not in play.

Cheers,
David.
-- 
David Fetter da...@fetter.org http://fetter.org/
Phone: +1 415 235 3778  AIM: dfetter666  Yahoo!: dfetter
Skype: davidfetter  XMPP: david.fet...@gmail.com
iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate

-- 
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] pg_filedump moved to pgfoundry

[Tom Lane]

David Fetter da...@fetter.org writes:
 On Mon, Jan 17, 2011 at 09:48:58PM -0500, Tom Lane wrote:
 (Before someone suggests folding it into contrib/: we can't because
 of license issues.  pg_filedump is GPL, per Red Hat company policy,
 and that's not going to change.)

 Who's the copyright holder(s)?  If it's all individual contributors,
 Red Hat policy is not in play.

Sorry David, it was written on the company's dime.

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] pg_filedump for 9.0?

[Tatsuo Ishii]

Hi,

Does anybody know if pg_filedump for PostgreSQL 9.0 already exists?
--
Tatsuo Ishii
SRA OSS, Inc. Japan
English: http://www.sraoss.co.jp/index_en.php
Japanese: http://www.sraoss.co.jp

-- 
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] pg_filedump for 9.0?

[Tom Lane]

Tatsuo Ishii is...@postgresql.org writes:
 Does anybody know if pg_filedump for PostgreSQL 9.0 already exists?

It's on my todo list to look at that, but right now I would think
that it doesn't need any changes since 8.4.

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

Re: [HACKERS] pg_filedump for 9.0?

[Devrim GÜNDÜZ]

On Tue, 2010-10-05 at 10:12 -0400, Tom Lane wrote:
  Does anybody know if pg_filedump for PostgreSQL 9.0 already exists?
 
 It's on my todo list to look at that, but right now I would think
 that it doesn't need any changes since 8.4.

Is there a test suite or so ? I can give it a try while building RPMs.
-- 
Devrim GÜNDÜZ
PostgreSQL Danışmanı/Consultant, Red Hat Certified Engineer
PostgreSQL RPM Repository: http://yum.pgrpms.org
Community: devrim~PostgreSQL.org, devrim.gunduz~linux.org.tr
http://www.gunduz.org  Twitter: http://twitter.com/devrimgunduz


signature.asc
Description: This is a digitally signed message part

Re: [HACKERS] pg_filedump for 9.0?

[Alvaro Herrera]

Excerpts from Devrim GÜNDÜZ's message of mar oct 05 10:16:45 -0400 2010:
 On Tue, 2010-10-05 at 10:12 -0400, Tom Lane wrote:
   Does anybody know if pg_filedump for PostgreSQL 9.0 already exists?
  
  It's on my todo list to look at that, but right now I would think
  that it doesn't need any changes since 8.4.
 
 Is there a test suite or so ? I can give it a try while building RPMs.

I don't think there's a test suite.  Just run it over some file and see
if it likes it :-)
 
-- 
Álvaro Herrera alvhe...@commandprompt.com
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

-- 
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] pg_filedump for 9.0?

[Alvaro Herrera]

Excerpts from Alvaro Herrera's message of mar oct 05 12:15:45 -0400 2010:
 Excerpts from Devrim GÜNDÜZ's message of mar oct 05 10:16:45 -0400 2010:
  On Tue, 2010-10-05 at 10:12 -0400, Tom Lane wrote:
Does anybody know if pg_filedump for PostgreSQL 9.0 already exists?
   
   It's on my todo list to look at that, but right now I would think
   that it doesn't need any changes since 8.4.
  
  Is there a test suite or so ? I can give it a try while building RPMs.
 
 I don't think there's a test suite.  Just run it over some file and see
 if it likes it :-)

I just tried it (the 8.4 version) and it works fine, at least for a heap
file.  Didn't try an index.

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

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

[HACKERS] pg_filedump strangeness

[Alvaro Herrera]

Hi,

I'm chasing an apparent index corruption problem, and I came across
something I can't quite explain in pg_filedump.  Say I dump a non-leaf
btree index page:

***
* PostgreSQL File/Block Formatted Dump Utility - Version 8.3.0
*
* File: 31141
* Options used: -fi -x -R 6246 
*
* Dump created on: Tue Apr  6 17:40:28 2010
***

Block 6246 
Header -
 Block Offset: 0x030cc000 Offsets: Lower  36 (0x0024)
 Block: Size 8192  Version4Upper8120 (0x1fb8)
 LSN:  logid   1077 recoff 0x45c8b660  Special  8176 (0x1ff0)
 Items:3  Free Space: 8084
 TLI: 0x0001  Prune XID: 0x  Flags: 0x ()
 Length (including item array): 36

  : 3504 60b6c845 0100 2400b81f  5...`..E$...
  0010: f01f0420  d89f3000 d09f1000  ... ..0.
  0020: b89f3000 ..0.

Data -- 
 Item   1 -- Length:   24  Offset: 8152 (0x1fd8)  Flags: NORMAL
  Block Id: 6232  linp Index: 1  Size: 24
  Has Nulls: 32768  Has Varwidths: 0

  1fd8: 5818 01001880 0100   ..X.
  1fe8: 80bcc57d 74230100...}t#..

 Item   2 -- Length:8  Offset: 8144 (0x1fd0)  Flags: NORMAL
  Block Id: 2756  linp Index: 1  Size: 8
  Has Nulls: 0  Has Varwidths: 0

  1fd0: c40a 01000800

 Item   3 -- Length:   24  Offset: 8120 (0x1fb8)  Flags: NORMAL
  Block Id: 6231  linp Index: 1  Size: 24
  Has Nulls: 32768  Has Varwidths: 0

  1fb8: 5718 01001880 0100   ..W.
  1fc8: 4009cc7f 73230100@...s#..


Special Section -
 BTree Index Section:
  Flags: 0x ()
  Blocks: Previous (6109)  Next (6305)  Level (1)  CycleId (0)

  1ff0: dd17 a118 0100   


*** End of Requested Range Encountered. Last Block Read: 6246 ***


Notice how item 2 has size 8, but regular entries have size 24.  I know
this is related to the high key of this page, but I can't quite figure
out why the short entry is 2 not 1.  Is item 2 just assumed to be
greater than the previous' page high key?

Page's 6109 high key is:

 Item   1 -- Length:   24  Offset: 8152 (0x1fd8)  Flags: NORMAL
  Block Id: 6101  linp Index: 1  Size: 24
  Has Nulls: 32768  Has Varwidths: 0

  1fd8: d517 01001880 0100   
  1fe8: 8004f17d 6f230100...}o#..


Note that the data values are integer timestamp without time zone in
little endian byte order.


(The Has Nulls bit is somewhat bogus -- it displays 32768 when the
0x8000 bit is on, which is rather surprising.  I'd expect it to display
1).


-- 
Alvaro Herrerahttp://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

-- 
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] pg_filedump strangeness

[Tom Lane]

Alvaro Herrera alvhe...@commandprompt.com writes:
 I'm chasing an apparent index corruption problem, and I came across
 something I can't quite explain in pg_filedump.  Say I dump a non-leaf
 btree index page:

I think this is actually OK.  Remember that in a non-rightmost page,
item 1 is the high key not a data entry.  On the other hand, in a
non-leaf page, we don't bother to store the key for the first downlink
entry, since the associated key is really minus infinity.  Cf
nbtree/README:

On a non-leaf page, the data items are down-links to child pages with
bounding keys.  The key in each data item is the *lower* bound for
keys on that child page, so logically the key is to the left of that
downlink.  The high key (if present) is the upper bound for the last
downlink.  The first data item on each such page has no lower bound
--- or lower bound of minus infinity, if you prefer.  The comparison
routines must treat it accordingly.  The actual key stored in the
item is irrelevant, and need not be stored at all.  This arrangement
corresponds to the fact that an LY non-leaf page has one more pointer
than key.

So item 2 doesn't have a key in it.  The other two items have null
keys, which means they need a null bitmap.  I don't however understand
why there seems to be data as well as a null bitmap in there --- is
this perhaps a two-column index?

 (The Has Nulls bit is somewhat bogus -- it displays 32768 when the
 0x8000 bit is on, which is rather surprising.  I'd expect it to display
 1).

Yeah, I noticed that too.  Made a note to myself to fix it in the next
revision of pg_filedump, which I suppose I'd better get on with
producing...

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

Re: [HACKERS] pg_filedump strangeness

[Alvaro Herrera]

Tom Lane wrote:
 Alvaro Herrera alvhe...@commandprompt.com writes:
  I'm chasing an apparent index corruption problem, and I came across
  something I can't quite explain in pg_filedump.  Say I dump a non-leaf
  btree index page:
 
 I think this is actually OK.  Remember that in a non-rightmost page,
 item 1 is the high key not a data entry.  On the other hand, in a
 non-leaf page, we don't bother to store the key for the first downlink
 entry, since the associated key is really minus infinity.  Cf
 nbtree/README:
 
   On a non-leaf page, the data items are down-links to child pages with
   bounding keys.  The key in each data item is the *lower* bound for
   keys on that child page, so logically the key is to the left of that
   downlink.  The high key (if present) is the upper bound for the last
   downlink.  The first data item on each such page has no lower bound
   --- or lower bound of minus infinity, if you prefer.  The comparison
   routines must treat it accordingly.  The actual key stored in the
   item is irrelevant, and need not be stored at all.  This arrangement
   corresponds to the fact that an LY non-leaf page has one more pointer
   than key.

Ahh, I had forgotten that bit completely.  Thanks.


 So item 2 doesn't have a key in it.  The other two items have null
 keys, which means they need a null bitmap.

Correct.

 I don't however understand
 why there seems to be data as well as a null bitmap in there --- is
 this perhaps a two-column index?

Eh, yeah, it's a two column index, so it's OK.

-- 
Alvaro Herrerahttp://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

-- 
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] pg_filedump strangeness

[Alvaro Herrera]

Tom Lane wrote:
 Alvaro Herrera alvhe...@commandprompt.com writes:
  I'm chasing an apparent index corruption problem, and I came across
  something I can't quite explain in pg_filedump.  Say I dump a non-leaf
  btree index page:
 
 I think this is actually OK.  Remember that in a non-rightmost page,
 item 1 is the high key not a data entry.

Oh, BTW, this is not what's corrupted about this index -- I just had
trouble following what pg_filedump was reporting.  The corruption is
more subtle: vacuum cannot find the parent page when trying to mark a
page for deletion.

-- 
Alvaro Herrerahttp://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.

-- 
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] pg_filedump for CVS HEAD

[Tom Lane]

Alvaro Herrera [EMAIL PROTECTED] writes:
 Who is in charge of pg_filedump now?

It's usually me that fixes it for new PG versions.  I don't normally
try to track CVS HEAD, just update it at release time.

 I noticed that the latest version (for 8.3) does not play nice with
 HEAD, because of changes in ControlFileData.  The attached patch fixes
 that, allowing it to compile.

Thanks, appreciate the patch.

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] pg_filedump for CVS HEAD

[Alvaro Herrera]

Hi,

Who is in charge of pg_filedump now?

I noticed that the latest version (for 8.3) does not play nice with
HEAD, because of changes in ControlFileData.  The attached patch fixes
that, allowing it to compile.

I didn't look if there were other changes needed for it to actually
work; any clues?

-- 
Alvaro Herrerahttp://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support
*** pg_filedump.c.orig	2008-02-28 11:12:21.0 -0300
--- pg_filedump.c	2008-11-13 13:55:11.0 -0300
***
*** 1170,1178 
  	 Maximum Index Keys: %u\n
  	   TOAST Chunk Size: %u\n
  	 Date and Time Type Storage: %s\n
! 	   Locale Buffer Length: %u\n
! 	 lc_collate: %s\n
! 	   lc_ctype: %s\n\n,
  	  EQ_CRC32 (crcLocal,
  			controlData-crc) ? Correct : Not Correct,
  	  controlData-pg_control_version,
--- 1170,1177 
  	 Maximum Index Keys: %u\n
  	   TOAST Chunk Size: %u\n
  	 Date and Time Type Storage: %s\n
! 	   float4 parameter passing: %s\n
! 	   float8 parameter passing: %s\n\n,
  	  EQ_CRC32 (crcLocal,
  			controlData-crc) ? Correct : Not Correct,
  	  controlData-pg_control_version,
***
*** 1204,1212 
  	  controlData-toast_max_chunk_size,
  	  (controlData-enableIntTimes ?
  	   64 bit Integers : Floating Point),
! 	  controlData-localeBuflen,
! 	  controlData-lc_collate,
! 	  controlData-lc_ctype);
  }
else
  {
--- 1203,1210 
  	  controlData-toast_max_chunk_size,
  	  (controlData-enableIntTimes ?
  	   64 bit Integers : Floating Point),
! 	  controlData-float4ByVal ? by value : by reference,
! 	  controlData-float8ByVal ? by value : by reference);
  }
else
  {

-- 
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] pg_filedump

[Tom Lane]

Alvaro Herrera [EMAIL PROTECTED] writes:
 I'm trying to get something from pg_filedump.  However, the version
 published in sources.redhat.com/rhdb doesn't grok a lot of changes in
 current CVS.  I changed all those and made it compile... but looks like
 that's only the easy part.  I get bogus values everywhere (block sizes,
 item numbers, etc).
 Does somebody know whether it's mantained for current versions?

AFAIK, no one has yet updated it for 7.3's changes in tuple header
format.  That needs to get done sometime soon ...

regards, tom lane

---(end of broadcast)---
TIP 4: Don't 'kill -9' the postmaster

[HACKERS] pg_filedump

[Alvaro Herrera]

Hello hackers,

I'm trying to get something from pg_filedump.  However, the version
published in sources.redhat.com/rhdb doesn't grok a lot of changes in
current CVS.  I changed all those and made it compile... but looks like
that's only the easy part.  I get bogus values everywhere (block sizes,
item numbers, etc).

Does somebody know whether it's mantained for current versions?

-- 
Alvaro Herrera (alvherre[a]dcc.uchile.cl)
Cuando miro a alguien, mas me atrae como cambia que quien es (J. Binoche)

---(end of broadcast)---
TIP 3: if posting/reading through Usenet, please send an appropriate
subscribe-nomail command to [EMAIL PROTECTED] so that your
message can get through to the mailing list cleanly