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
*******************************************************************
Block 0 ********************************************************
<Header> -----
Block Offset: 0x00000000 Offsets: Lower 268 (0x010c)
Block: Size 8192 Version 4 Upper 384 (0x0180)
LSN: logid 0 recoff 0x00000000 Special 8192 (0x2000)
Items: 61 Free Space: 116
Checksum: 0x0000 Prune XID: 0x00000000 Flags: 0x0000 ()
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} testdb<<EOF > 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
+ size as listed on block 0 in the file
The following options are valid for heap and index files:
-a Display absolute addresses when formatting (Block header
@@ -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/contrib/pg_filedump/pg_filedump.c
b/contrib/pg_filedump/pg_filedump.c
index e5686ea..7d1aa9a 100644
--- a/contrib/pg_filedump/pg_filedump.c
+++ b/contrib/pg_filedump/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);
@@ -64,11 +71,11 @@ DisplayOptions (unsigned int validOptions)
printf
("\nVersion %s (for %s)"
"\nCopyright (c) 2002-2010 Red Hat, Inc."
- "\nCopyright (c) 2011-2012, PostgreSQL Global Development Group\n",
+ "\nCopyright (c) 2011-2013, PostgreSQL Global Development Group\n",
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 @@ FormatHeader (Page page)
|| (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,7 +1229,7 @@ FormatSpecial ()
// For each block, dump out formatted header and content information
static void
-FormatBlock ()
+FormatBlock (BlockNumber blkno)
{
Page page = (Page) buffer;
pageOffset = blockSize * currentBlock;
@@ -1228,7 +1249,7 @@ FormatBlock ()
int rc;
// Every block contains a header, items and possibly a special
// section. Beware of partial block reads though
- rc = FormatHeader (page);
+ rc = FormatHeader (page, blkno);
// If we didn't encounter a partial read in the header, carry on...
if (rc != EOF_ENCOUNTERED)
@@ -1498,7 +1519,7 @@ DumpFileContents ()
contentsToDump = false;
}
else
- FormatBlock ();
+ FormatBlock (currentBlock);
}
}
diff --git a/contrib/pg_filedump/pg_filedump.h
b/contrib/pg_filedump/pg_filedump.h
index e0c61be..1953144 100644
--- a/contrib/pg_filedump/pg_filedump.h
+++ b/contrib/pg_filedump/pg_filedump.h
@@ -50,7 +50,8 @@ typedef enum
BLOCK_FORMAT = 0x00000004, // -f: Formatted dump of blocks / control file
BLOCK_FORCED = 0x00000008, // -S: Block size forced
BLOCK_NO_INTR = 0x00000010, // -d: Dump straight blocks
- BLOCK_RANGE = 0x00000020 // -R: Specific block range to dump
+ BLOCK_RANGE = 0x00000020, // -R: Specific block range to dump
+ BLOCK_CHECKSUMS = 0x00000040 // -k: verify block checksums
}
blockSwitches;
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
