Patch to allow pg_filedump to support reading of pg_filenode.map

Started by Richard Yenover 4 years ago7 messages
#1Richard Yen
richyen3@gmail.com
1 attachment(s)

Hello Hackers,

I recently had to work on a case where some catalog files were
corrupt and/or missing. One of the things we sought to inspect was
pg_filenode.map, but there was no tooling available to do so.

With the help of Álvaro H.. I've put together a patch to allow pg_filedump
to do some rudimentary decoding of pg_filenode.map, so that it's at least
human-readable. I had the idea to try to map the OIDs to relnames, but
that might get hairy, with TOAST table OIDs possibly changing (for pg_proc,
etc.)

It seems that Christoph Berg is the primary committer for the pg_filedump
project these days so he is cc'd here, but if there's some other place I
should be sending this kind of contribution to, please let me know.
Hopefully this will be helpful to others in the future.

Much appreciated,
--Richard

Attachments:

add_filenode_support.patchapplication/octet-stream; name=add_filenode_support.patchDownload
diff --git a/pg_filedump.c b/pg_filedump.c
index f3650da..0bee1e3 100644
--- a/pg_filedump.c
+++ b/pg_filedump.c
@@ -83,9 +83,34 @@ static unsigned int bytesToFormat = 0;
 /* Block version number */
 static unsigned int blockVersion = 0;
 
+/* Flag to indicate filemap file */
+static unsigned int isMapperFile = 0;
+
 /* Program exit code */
 static int	exitCode = 0;
 
+/* Relmapper structs */
+/* Maybe ask community to put this into utils/relmapper.h? */
+#define RELMAPPER_FILEMAGIC   0x592717
+char magic_buffer[8];
+char relmap_file[512];
+typedef struct RelMapping
+{
+  Oid     mapoid;     /* OID of a catalog */
+  Oid     mapfilenode;  /* its filenode number */
+} RelMapping;
+
+/* crc and pad are ignored here, even though they are
+ * present in the backend code.  We assume that anyone
+ * seeking to inspect the contents of pg_filenode.map
+ * probably have a corrupted or non-functional cluster */
+typedef struct RelMapFile
+{
+  int32   magic;      /* always RELMAPPER_FILEMAGIC */
+  int32   num_mappings; /* number of valid RelMapping entries */
+  RelMapping  mappings[FLEXIBLE_ARRAY_MEMBER];
+} RelMapFile;
+
 /*
  * Function Prototypes
  */
@@ -127,6 +152,8 @@ static void FormatControl(char *buffer);
 static void FormatBinary(char *buffer,
 		unsigned int numBytes, unsigned int startIndex);
 static void DumpBinaryBlock(char *buffer);
+static int CheckForRelmap(FILE *fp);
+static int PrintMappings(RelMapFile *map);
 
 
 /* Send properly formed usage information to the user. */
@@ -179,6 +206,7 @@ DisplayOptions(unsigned int validOptions)
 		 "  -c  Interpret the file listed as a control file\n"
 		 "  -f  Display formatted content dump along with interpretation\n"
 		 "  -S  Force block size to [blocksize]\n"
+     "If a pg_filenode.map file is passed in, all options will be ignored\n"
 		 "\nReport bugs to <pgsql-bugs@postgresql.org>\n");
 }
 
@@ -430,6 +458,7 @@ ConsumeOptions(int numOptions, char **options)
 					fileName = options[x];
 					if (!(segmentOptions & SEGMENT_NUMBER_FORCED))
 						segmentNumber = GetSegmentNumberFromFileName(fileName);
+                                        isMapperFile = CheckForRelmap(fp);
 				}
 				else
 				{
@@ -2000,6 +2029,44 @@ DumpFileContents(unsigned int blockOptions,
 	return result;
 }
 
+int
+CheckForRelmap(FILE *fp)
+{
+  // Get the first 8 bytes for comparison
+  fread(magic_buffer,1,8,fp);
+  // Put things back where we found them
+  rewind(fp);
+
+  // If it's a relmapper file, then ingest the whole thing
+  if ( (int) magic_buffer & RELMAPPER_FILEMAGIC ) {
+    fread(relmap_file,1,512,fp);
+    return 1;
+  }
+  return 0;
+}
+
+int
+PrintMappings(RelMapFile *map)
+{
+	RelMapping *mappings;
+	RelMapping m;
+
+	// Print Metadata
+	printf("Magic Number: %x\n",map->magic);
+	printf("Num Mappings: %d\n",map->num_mappings);
+
+	// Print Mappings
+	printf("Detailed Mappings list:\n");
+	mappings = map->mappings;
+	for (int i=0; i < map->num_mappings; i++) {
+		m = mappings[i];
+		printf("OID: %u\tFilenode: %u\n",
+			m.mapoid,
+			m.mapfilenode);
+	}
+	return 1;
+}
+
 /* Consume the options and iterate through the given file, formatting as
  * requested. */
 int
@@ -2030,17 +2097,21 @@ main(int argv, char **argc)
 		else if (!(blockOptions & BLOCK_FORCED))
 			blockSize = GetBlockSize(fp);
 
-		exitCode = DumpFileContents(blockOptions,
-				controlOptions,
-				fp,
-				blockSize,
-				blockStart,
-				blockEnd,
-				false /* is toast realtion */,
-				0,    /* no toast Oid */
-				0,    /* no toast external size */
-				NULL  /* no out toast value */
-				);
+                if (isMapperFile) {
+			exitCode = PrintMappings((RelMapFile *) relmap_file);
+                } else {
+			exitCode = DumpFileContents(blockOptions,
+					controlOptions,
+					fp,
+					blockSize,
+					blockStart,
+					blockEnd,
+					false /* is toast realtion */,
+					0,    /* no toast Oid */
+					0,    /* no toast external size */
+					NULL  /* no out toast value */
+					);
+                }
 	}
 
 	if (fp)
#2Justin Pryzby
pryzby@telsasoft.com
In reply to: Richard Yen (#1)
Re: Patch to allow pg_filedump to support reading of pg_filenode.map

This is separate from the postgresql server repo.
https://git.postgresql.org/gitweb/?p=pg_filedump.git

+#define RELMAPPER_FILEMAGIC 0x592717
+char magic_buffer[8];

...

+ if ( (int) magic_buffer & RELMAPPER_FILEMAGIC ) {

This is doing bitwise arithmetic on a pointer, which seems badly wrong.
I think it breaks normal use of pg_filedump - unless you happen to get a
magic_buffer without those bits set. The segfault seems to confirm that, as
does gcc:

pg_filedump.c:2041:8: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
2041 | if ( (int) magic_buffer & RELMAPPER_FILEMAGIC ) {

I think it probably means to do memcmp, instead ??

--
Justin

#3Richard Yen
richyen3@gmail.com
In reply to: Justin Pryzby (#2)
1 attachment(s)
Re: Patch to allow pg_filedump to support reading of pg_filenode.map

Thanks for the feedback, Justin. I've gone ahead and switched to use
memcmp. I also refactored it to:

1. Don't assume that any file with first 4 bytes matching the
relmapper magic number is a pg_relnode.map file
2. Don't assume the pg_relnode.map file is uncorrupted and intact; perform
a check of the first 4 bytes against the reference magic number
3. Provide a flag (-m) for users to have their file interpreted as a
pg_relnode.map file

I hope this is more palatable to everyone :)

--Richard

On Wed, Apr 28, 2021 at 9:42 PM Justin Pryzby <pryzby@telsasoft.com> wrote:

Show quoted text

This is separate from the postgresql server repo.
https://git.postgresql.org/gitweb/?p=pg_filedump.git

+#define RELMAPPER_FILEMAGIC 0x592717
+char magic_buffer[8];

...

+ if ( (int) magic_buffer & RELMAPPER_FILEMAGIC ) {

This is doing bitwise arithmetic on a pointer, which seems badly wrong.
I think it breaks normal use of pg_filedump - unless you happen to get a
magic_buffer without those bits set. The segfault seems to confirm that,
as
does gcc:

pg_filedump.c:2041:8: warning: cast from pointer to integer of different
size [-Wpointer-to-int-cast]
2041 | if ( (int) magic_buffer & RELMAPPER_FILEMAGIC ) {

I think it probably means to do memcmp, instead ??

--
Justin

Attachments:

add_filenode_support_v2.patchapplication/octet-stream; name=add_filenode_support_v2.patchDownload
diff --git a/pg_filedump.c b/pg_filedump.c
index f3650da..50a5e2f 100644
--- a/pg_filedump.c
+++ b/pg_filedump.c
@@ -83,9 +83,30 @@ static unsigned int bytesToFormat = 0;
 /* Block version number */
 static unsigned int blockVersion = 0;
 
+/* Flag to indicate pg_relnode.map file */
+static bool isRelMapFile = false;
+
 /* Program exit code */
 static int	exitCode = 0;
 
+/* Relmapper structs */
+typedef struct RelMapping
+{
+  Oid     mapoid;     /* OID of a catalog */
+  Oid     mapfilenode;  /* its filenode number */
+} RelMapping;
+
+/* crc and pad are ignored here, even though they are
+ * present in the backend code.  We assume that anyone
+ * seeking to inspect the contents of pg_filenode.map
+ * probably have a corrupted or non-functional cluster */
+typedef struct RelMapFile
+{
+  int32   magic;      /* always RELMAPPER_FILEMAGIC */
+  int32   num_mappings; /* number of valid RelMapping entries */
+  RelMapping  mappings[FLEXIBLE_ARRAY_MEMBER];
+} RelMapFile;
+
 /*
  * Function Prototypes
  */
@@ -127,6 +148,7 @@ static void FormatControl(char *buffer);
 static void FormatBinary(char *buffer,
 		unsigned int numBytes, unsigned int startIndex);
 static void DumpBinaryBlock(char *buffer);
+static int PrintRelMappings(void);
 
 
 /* Send properly formed usage information to the user. */
@@ -179,6 +201,9 @@ DisplayOptions(unsigned int validOptions)
 		 "  -c  Interpret the file listed as a control file\n"
 		 "  -f  Display formatted content dump along with interpretation\n"
 		 "  -S  Force block size to [blocksize]\n"
+		 "Additional functions:\n"
+		 "  -m  Interpret file as pg_relnode.map file and print contents (all\n"
+		 "      other options will be ignored)\n" 
 		 "\nReport bugs to <pgsql-bugs@postgresql.org>\n");
 }
 
@@ -515,6 +540,11 @@ ConsumeOptions(int numOptions, char **options)
 						SET_OPTION(blockOptions, BLOCK_CHECKSUMS, 'k');
 						break;
 
+						/* Treat file as pg_relnode.map file */
+					case 'm':
+						isRelMapFile = true;
+						break;
+
 						/* Display old values. Ignore Xmax */
 					case 'o':
 						SET_OPTION(blockOptions, BLOCK_IGNORE_OLD, 'o');
@@ -2000,6 +2030,59 @@ DumpFileContents(unsigned int blockOptions,
 	return result;
 }
 
+int
+PrintRelMappings(void)
+{
+	// For storing ingested data
+	char charbuf[RELMAPPER_FILESIZE];
+	RelMapFile *map;
+	RelMapping *mappings;
+	RelMapping m;
+	int bytesRead;
+
+	// For confirming Magic Number correctness
+	char m1[RELMAPPER_MAGICSIZE];
+	char m2[RELMAPPER_MAGICSIZE];
+	int magic_ref = RELMAPPER_FILEMAGIC;
+	int magic_val;
+
+	// Read in the file
+	rewind(fp); // Make sure to start from the beginning
+	bytesRead = fread(charbuf,1,RELMAPPER_FILESIZE,fp);
+	if ( bytesRead != RELMAPPER_FILESIZE ) {
+		printf("Read %d bytes, expected %d\n", bytesRead, RELMAPPER_FILESIZE);
+		return 0;
+	}
+
+	// Convert to RelMapFile type for usability
+	map = (RelMapFile *) charbuf;
+
+
+	// Check and print Magic Number correctness
+	printf("Magic Number: 0x%x",map->magic);
+	magic_val = map->magic;
+
+	memcpy(m1,&magic_ref,RELMAPPER_MAGICSIZE);
+	memcpy(m2,&magic_val,RELMAPPER_MAGICSIZE);
+	if ( memcmp(m1,m2,RELMAPPER_MAGICSIZE) == 0 ) {
+		printf(" (CORRECT)\n");
+	} else {
+		printf(" (INCORRECT)\n");
+	}
+
+	// Print Mappings
+	printf("Num Mappings: %d\n",map->num_mappings);
+	printf("Detailed Mappings list:\n");
+	mappings = map->mappings;
+	for (int i=0; i < map->num_mappings; i++) {
+		m = mappings[i];
+		printf("OID: %u\tFilenode: %u\n",
+			m.mapoid,
+			m.mapfilenode);
+	}
+	return 1;
+}
+
 /* Consume the options and iterate through the given file, formatting as
  * requested. */
 int
@@ -2014,6 +2097,11 @@ main(int argv, char **argc)
 	 * where encountered */
 	if (validOptions != OPT_RC_VALID)
 		DisplayOptions(validOptions);
+	else if (isRelMapFile)
+	{
+		CreateDumpFileHeader(argv, argc);
+		exitCode = PrintRelMappings();
+	}
 	else
 	{
 		/* Don't dump the header if we're dumping binary pages */
diff --git a/pg_filedump.h b/pg_filedump.h
index fbb8792..0596505 100644
--- a/pg_filedump.h
+++ b/pg_filedump.h
@@ -137,6 +137,12 @@ typedef enum optionReturnCodes
 #define EOF_ENCOUNTERED (-1)	/* Indicator for partial read */
 #define BYTES_PER_LINE 16		/* Format the binary 16 bytes per line */
 
+/* Constants for pg_relnode.map decoding */
+#define RELMAPPER_MAGICSIZE   4
+#define RELMAPPER_FILESIZE    512
+/* Maybe ask community to put this into utils/relmapper.h? */
+#define RELMAPPER_FILEMAGIC   0x592717
+
 extern char *fileName;
 
 /*
#4Justin Pryzby
pryzby@telsasoft.com
In reply to: Richard Yen (#3)
Re: Patch to allow pg_filedump to support reading of pg_filenode.map

I think you should be able to avoid crashing if passed a non-relmapper file.
Make sure not to loop over more mappings than exist in the relmapper file of
the given size.

I guess you should warn if the number of mappings is too large for the file's
size. And then "cap" the number of mappings to the maximum possible number.

--
Justin

#5Richard Yen
richyen3@gmail.com
In reply to: Justin Pryzby (#4)
1 attachment(s)
Re: Patch to allow pg_filedump to support reading of pg_filenode.map

On Thu, Apr 29, 2021 at 12:05 PM Justin Pryzby <pryzby@telsasoft.com> wrote:

I think you should be able to avoid crashing if passed a non-relmapper
file.
Make sure not to loop over more mappings than exist in the relmapper file
of
the given size.

I guess you should warn if the number of mappings is too large for the
file's
size. And then "cap" the number of mappings to the maximum possible
number.

Ah, thanks for the tip. That's right -- I can't assume the user's input is
a valid file. Updated patch here.

--Richard

Show quoted text

--
Justin

Attachments:

add_filenode_support_v3.patchapplication/octet-stream; name=add_filenode_support_v3.patchDownload
diff --git a/pg_filedump.c b/pg_filedump.c
index f3650da..d1609c0 100644
--- a/pg_filedump.c
+++ b/pg_filedump.c
@@ -83,9 +83,30 @@ static unsigned int bytesToFormat = 0;
 /* Block version number */
 static unsigned int blockVersion = 0;
 
+/* Flag to indicate pg_filenode.map file */
+static bool isRelMapFile = false;
+
 /* Program exit code */
 static int	exitCode = 0;
 
+/* Relmapper structs */
+typedef struct RelMapping
+{
+  Oid     mapoid;     /* OID of a catalog */
+  Oid     mapfilenode;  /* its filenode number */
+} RelMapping;
+
+/* crc and pad are ignored here, even though they are
+ * present in the backend code.  We assume that anyone
+ * seeking to inspect the contents of pg_filenode.map
+ * probably have a corrupted or non-functional cluster */
+typedef struct RelMapFile
+{
+  int32   magic;      /* always RELMAPPER_FILEMAGIC */
+  int32   num_mappings; /* number of valid RelMapping entries */
+  RelMapping  mappings[FLEXIBLE_ARRAY_MEMBER];
+} RelMapFile;
+
 /*
  * Function Prototypes
  */
@@ -127,6 +148,7 @@ static void FormatControl(char *buffer);
 static void FormatBinary(char *buffer,
 		unsigned int numBytes, unsigned int startIndex);
 static void DumpBinaryBlock(char *buffer);
+static int PrintRelMappings(void);
 
 
 /* Send properly formed usage information to the user. */
@@ -179,6 +201,9 @@ DisplayOptions(unsigned int validOptions)
 		 "  -c  Interpret the file listed as a control file\n"
 		 "  -f  Display formatted content dump along with interpretation\n"
 		 "  -S  Force block size to [blocksize]\n"
+		 "Additional functions:\n"
+		 "  -m  Interpret file as pg_filenode.map file and print contents (all\n"
+		 "      other options will be ignored)\n" 
 		 "\nReport bugs to <pgsql-bugs@postgresql.org>\n");
 }
 
@@ -515,6 +540,11 @@ ConsumeOptions(int numOptions, char **options)
 						SET_OPTION(blockOptions, BLOCK_CHECKSUMS, 'k');
 						break;
 
+						/* Treat file as pg_filenode.map file */
+					case 'm':
+						isRelMapFile = true;
+						break;
+
 						/* Display old values. Ignore Xmax */
 					case 'o':
 						SET_OPTION(blockOptions, BLOCK_IGNORE_OLD, 'o');
@@ -2000,6 +2030,69 @@ DumpFileContents(unsigned int blockOptions,
 	return result;
 }
 
+int
+PrintRelMappings(void)
+{
+	// For storing ingested data
+	char charbuf[RELMAPPER_FILESIZE];
+	RelMapFile *map;
+	RelMapping *mappings;
+	RelMapping m;
+	int bytesRead;
+
+	// For confirming Magic Number correctness
+	char m1[RELMAPPER_MAGICSIZE];
+	char m2[RELMAPPER_MAGICSIZE];
+	int magic_ref = RELMAPPER_FILEMAGIC;
+	int magic_val;
+	int num_loops;
+
+	// Read in the file
+	rewind(fp); // Make sure to start from the beginning
+	bytesRead = fread(charbuf,1,RELMAPPER_FILESIZE,fp);
+	if ( bytesRead != RELMAPPER_FILESIZE ) {
+		printf("Read %d bytes, expected %d\n", bytesRead, RELMAPPER_FILESIZE);
+		return 0;
+	}
+
+	// Convert to RelMapFile type for usability
+	map = (RelMapFile *) charbuf;
+
+
+	// Check and print Magic Number correctness
+	printf("Magic Number: 0x%x",map->magic);
+	magic_val = map->magic;
+
+	memcpy(m1,&magic_ref,RELMAPPER_MAGICSIZE);
+	memcpy(m2,&magic_val,RELMAPPER_MAGICSIZE);
+	if ( memcmp(m1,m2,RELMAPPER_MAGICSIZE) == 0 ) {
+		printf(" (CORRECT)\n");
+	} else {
+		printf(" (INCORRECT)\n");
+	}
+
+	// Print Mappings
+	printf("Num Mappings: %d\n",map->num_mappings);
+	printf("Detailed Mappings list:\n");
+	mappings = map->mappings;
+
+	// Limit number of mappings as per MAX_MAPPINGS
+	num_loops = map->num_mappings;
+	if ( map->num_mappings > MAX_MAPPINGS ) {
+		num_loops = MAX_MAPPINGS;
+		printf("  NOTE: listing has been limited to the first %d mappings\n", MAX_MAPPINGS);
+		printf("        (perhaps your file is not a valid pg_filenode.map file?)\n");
+	}
+
+	for (int i=0; i < num_loops; i++) {
+		m = mappings[i];
+		printf("OID: %u\tFilenode: %u\n",
+			m.mapoid,
+			m.mapfilenode);
+	}
+	return 1;
+}
+
 /* Consume the options and iterate through the given file, formatting as
  * requested. */
 int
@@ -2014,6 +2107,11 @@ main(int argv, char **argc)
 	 * where encountered */
 	if (validOptions != OPT_RC_VALID)
 		DisplayOptions(validOptions);
+	else if (isRelMapFile)
+	{
+		CreateDumpFileHeader(argv, argc);
+		exitCode = PrintRelMappings();
+	}
 	else
 	{
 		/* Don't dump the header if we're dumping binary pages */
diff --git a/pg_filedump.h b/pg_filedump.h
index fbb8792..eaafde1 100644
--- a/pg_filedump.h
+++ b/pg_filedump.h
@@ -137,6 +137,14 @@ typedef enum optionReturnCodes
 #define EOF_ENCOUNTERED (-1)	/* Indicator for partial read */
 #define BYTES_PER_LINE 16		/* Format the binary 16 bytes per line */
 
+/* Constants for pg_relnode.map decoding */
+#define RELMAPPER_MAGICSIZE   4
+#define RELMAPPER_FILESIZE    512
+/* From utils/cache/relmapper.c -- Maybe ask community to put
+ * these into utils/cache/relmapper.h? */
+#define RELMAPPER_FILEMAGIC   0x592717
+#define MAX_MAPPINGS          62
+
 extern char *fileName;
 
 /*
#6Christoph Berg
myon@debian.org
In reply to: Richard Yen (#5)
Re: Patch to allow pg_filedump to support reading of pg_filenode.map

Re: Richard Yen

Ah, thanks for the tip. That's right -- I can't assume the user's input is
a valid file. Updated patch here.

Hi Richard,

sorry for the very late response here.

Thanks for the patch which I just merged, and thanks Justin for the
reviews!

Christoph

#7Richard Yen
richyen3@gmail.com
In reply to: Christoph Berg (#6)
Re: Patch to allow pg_filedump to support reading of pg_filenode.map

On Sep 29, 2021, at 9:01 AM, Christoph Berg <myon@debian.org> wrote:

Re: Richard Yen

Ah, thanks for the tip. That's right -- I can't assume the user's input is
a valid file. Updated patch here.

Hi Richard,

sorry for the very late response here.

Thanks for the patch which I just merged, and thanks Justin for the
reviews!

Thank you! Was a great coding exercise :)

-Richard