Re: [HACKERS] [PATCHES] Writing WAL for relcache invalidation:pg_internal.init
On Wed, 2006-11-01 at 12:05 -0500, Tom Lane wrote:
I think we're probably better off to just forcibly remove the init file
during post-recovery cleanup. The easiest place to do this might be
BuildFlatFiles, which has to scan pg_database anyway ...
Patch enclosed.
Clean apply to HEAD, make check OK, plus restart check.
No specific PITR test, since same code path.
--
Simon Riggs
EnterpriseDB http://www.enterprisedb.com
Index: src/backend/utils/cache/inval.c
===
RCS file: /projects/cvsroot/pgsql/src/backend/utils/cache/inval.c,v
retrieving revision 1.78
diff -c -r1.78 inval.c
*** src/backend/utils/cache/inval.c 4 Oct 2006 00:30:00 - 1.78
--- src/backend/utils/cache/inval.c 5 Nov 2006 22:00:03 -
***
*** 767,776
SendSharedInvalidMessage(msg);
break;
case TWOPHASE_INFO_FILE_BEFORE:
! RelationCacheInitFileInvalidate(true);
break;
case TWOPHASE_INFO_FILE_AFTER:
! RelationCacheInitFileInvalidate(false);
break;
default:
Assert(false);
--- 767,776
SendSharedInvalidMessage(msg);
break;
case TWOPHASE_INFO_FILE_BEFORE:
! RelationCacheInitFileInvalidate(true, DatabasePath);
break;
case TWOPHASE_INFO_FILE_AFTER:
! RelationCacheInitFileInvalidate(false, DatabasePath);
break;
default:
Assert(false);
***
*** 817,823
* unless we committed.
*/
if (transInvalInfo-RelcacheInitFileInval)
! RelationCacheInitFileInvalidate(true);
AppendInvalidationMessages(transInvalInfo-PriorCmdInvalidMsgs,
transInvalInfo-CurrentCmdInvalidMsgs);
--- 817,823
* unless we committed.
*/
if (transInvalInfo-RelcacheInitFileInval)
! RelationCacheInitFileInvalidate(true, DatabasePath);
AppendInvalidationMessages(transInvalInfo-PriorCmdInvalidMsgs,
transInvalInfo-CurrentCmdInvalidMsgs);
***
*** 826,832
SendSharedInvalidMessage);
if (transInvalInfo-RelcacheInitFileInval)
! RelationCacheInitFileInvalidate(false);
}
else if (transInvalInfo != NULL)
{
--- 826,832
SendSharedInvalidMessage);
if (transInvalInfo-RelcacheInitFileInval)
! RelationCacheInitFileInvalidate(false, DatabasePath);
}
else if (transInvalInfo != NULL)
{
Index: src/backend/utils/cache/relcache.c
===
RCS file: /projects/cvsroot/pgsql/src/backend/utils/cache/relcache.c,v
retrieving revision 1.249
diff -c -r1.249 relcache.c
*** src/backend/utils/cache/relcache.c 4 Oct 2006 00:30:00 - 1.249
--- src/backend/utils/cache/relcache.c 5 Nov 2006 22:00:06 -
***
*** 3559,3570
* no backend has been started since the last removal.
*/
void
! RelationCacheInitFileInvalidate(bool beforeSend)
{
char initfilename[MAXPGPATH];
snprintf(initfilename, sizeof(initfilename), %s/%s,
! DatabasePath, RELCACHE_INIT_FILENAME);
if (beforeSend)
{
--- 3559,3570
* no backend has been started since the last removal.
*/
void
! RelationCacheInitFileInvalidate(bool beforeSend, char *ForDatabasePath)
{
char initfilename[MAXPGPATH];
snprintf(initfilename, sizeof(initfilename), %s/%s,
! ForDatabasePath, RELCACHE_INIT_FILENAME);
if (beforeSend)
{
Index: src/backend/utils/init/flatfiles.c
===
RCS file: /projects/cvsroot/pgsql/src/backend/utils/init/flatfiles.c,v
retrieving revision 1.21
diff -c -r1.21 flatfiles.c
*** src/backend/utils/init/flatfiles.c 14 Jul 2006 14:52:25 - 1.21
--- src/backend/utils/init/flatfiles.c 5 Nov 2006 22:00:07 -
***
*** 36,41
--- 36,42
#include access/transam.h
#include access/twophase_rmgr.h
#include access/xact.h
+ #include catalog/catalog.h
#include catalog/pg_auth_members.h
#include catalog/pg_authid.h
#include catalog/pg_database.h
***
*** 47,52
--- 48,54
#include storage/pmsignal.h
#include utils/builtins.h
#include utils/flatfiles.h
+ #include utils/relcache.h
#include utils/resowner.h
***
*** 165,173
*
* A side effect is to determine the oldest database's datminxid
* so we can set or update the XID wrap limit.
*/
static void
! write_database_file(Relation drel)
{
char *filename,
*tempname;
--- 167,178
*
* A side effect is to determine the oldest database's datminxid
* so we can set or update the XID wrap limit.
+ *
+ * removeDBRelInitFile is true only at startup, to allow us to
+ * remove the relcache init file as we read each database
*/
static void
! write_database_file(Relation drel, bool removeDBRelInitFile)
{
char *filename,
*tempname;
***
*** 252,257
--- 257,278
fputs_quote(datname, fp);
fprintf(fp, %u
Re: [HACKERS] [PATCHES] Writing WAL for relcache invalidation:pg_internal.init
Simon Riggs [EMAIL PROTECTED] writes: On Wed, 2006-11-01 at 12:05 -0500, Tom Lane wrote: I think we're probably better off to just forcibly remove the init file during post-recovery cleanup. The easiest place to do this might be BuildFlatFiles, which has to scan pg_database anyway ... Patch enclosed. Applied to HEAD and 8.1. It doesn't work in 8.0 though, because flatfiles.c doesn't exist. AFAIR there isn't any startup-time scan of pg_database at all before 8.1. Not sure if it's worth coming up with a whole new patch for that branch. regards, tom lane ---(end of broadcast)--- TIP 9: In versions below 8.0, the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
Re: [HACKERS] [PATCHES] Writing WAL for relcache invalidation:pg_internal.init
On Wed, 2006-11-01 at 12:05 -0500, Tom Lane wrote: Simon Riggs [EMAIL PROTECTED] writes: Enclose a patch for new WAL records for relcache invalidation. I don't think this works. RelationCacheInitFileInvalidate is executed post-commit, which means that there's a window between commit and where you propose to write the WAL entry. A crash and restart in that interval would leave the catalog changes committed, but not reflected into pg_internal.init. Surely you are pointing out a bug, no? If a backend did crash, the init file would be wrong and we'd get exactly the same wrong relfilenode errors we got after that PITR. The issue must surely be that the patch isn't wrong per se, just that RelationCacheInitFileInvalidate is called too late and that requires an additional fix. Are we certain that a crash between commit and invalidation will cause a PANIC that takes down the server? Doesn't look like its in a critical section to me. I think we're probably better off to just forcibly remove the init file during post-recovery cleanup. The easiest place to do this might be BuildFlatFiles, which has to scan pg_database anyway ... I can do this - I don't have a problem there, but the above issue just occurred to me so I wonder now if its the right thing to do. PITR will be always-safe but normal operation might not be. -- Simon Riggs EnterpriseDB http://www.enterprisedb.com ---(end of broadcast)--- TIP 1: 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
Re: [HACKERS] [PATCHES] Writing WAL for relcache invalidation:pg_internal.init
Tom, Simon et al; Please clarify. PostgreSQL 8.1.5 on sparc-sun-solaris2.9, compiled by GCC gcc (GCC) 3.4.2 We're getting ready to init a new warm standby instance based on last night's snapshot of running prod server. I see a few of these pg_internal.init files in the cluster as it's being unpacked. Same warm standby instance may sit for weeks gobbling up WALs from the prod server then be finally brought live. Question; Is it safe to delete the .init files now (before starting recovery), or perhaps unconditionally right after going live? In other words, is there any sure fire preventative measure that I can apply to guarantee against the fault that started this threadd? Tom wrote: Meanwhile, if you're trying to recover from a PITR backup and it's not working, try removing any pg_internal.init files you can find. Comment above seems to suggest not touching existing pg_internal.init files unless a problem is seen. Thanks Simon Riggs [EMAIL PROTECTED] writes: On Wed, 2006-11-01 at 12:05 -0500, Tom Lane wrote: Simon Riggs [EMAIL PROTECTED] writes: Enclose a patch for new WAL records for relcache invalidation. I don't think this works. RelationCacheInitFileInvalidate is executed post-commit, which means that there's a window between commit and where you propose to write the WAL entry. A crash and restart in that interval would leave the catalog changes committed, but not reflected into pg_internal.init. Surely you are pointing out a bug, no? If a backend did crash, the init file would be wrong and we'd get exactly the same wrong relfilenode errors we got after that PITR. The issue must surely be that the patch isn't wrong per se, just that RelationCacheInitFileInvalidate is called too late and that requires an additional fix. Are we certain that a crash between commit and invalidation will cause a PANIC that takes down the server? Doesn't look like its in a critical section to me. I think we're probably better off to just forcibly remove the init file during post-recovery cleanup. The easiest place to do this might be BuildFlatFiles, which has to scan pg_database anyway ... I can do this - I don't have a problem there, but the above issue just occurred to me so I wonder now if its the right thing to do. PITR will be always-safe but normal operation might not be. -- Simon Riggs EnterpriseDB http://www.enterprisedb.com ---(end of broadcast)--- TIP 1: 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 -- --- Jerry Sievers 305 854-3001 (home) WWW ECommerce Consultant 305 321-1144 (mobilehttp://www.JerrySievers.com/ ---(end of broadcast)--- TIP 9: In versions below 8.0, the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
Re: [HACKERS] [PATCHES] Writing WAL for relcache invalidation:pg_internal.init
Simon Riggs [EMAIL PROTECTED] writes: On Wed, 2006-11-01 at 12:05 -0500, Tom Lane wrote: I don't think this works. Surely you are pointing out a bug, no? If a backend did crash, the init file would be wrong and we'd get exactly the same wrong relfilenode errors we got after that PITR. Yeah, which is the same bug we've got now. They're both WAL-replay- doesn't-fix-the-init-file cases. The issue must surely be that the patch isn't wrong per se, just that RelationCacheInitFileInvalidate is called too late and that requires an additional fix. No. In the non-crash situation there is sufficient interlocking to avoid a problem, and I feel no desire to redesign that mechanism. Trying to do it before commit would create its own issues, anyway: someone could install a new init file before you finish committing. regards, tom lane ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [HACKERS] [PATCHES] Writing WAL for relcache invalidation:pg_internal.init
Jerry Sievers [EMAIL PROTECTED] writes: Is it safe to delete the .init files now (before starting recovery), Should be OK as far as I can see. regards, tom lane ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [HACKERS] [PATCHES] Writing WAL for relcache invalidation:pg_internal.init
On Wed, 2006-11-01 at 12:05 -0500, Tom Lane wrote: I think we're probably better off to just forcibly remove the init file during post-recovery cleanup. The easiest place to do this might be BuildFlatFiles, which has to scan pg_database anyway ... Presumably not rebuilding it, since we can let that occur naturally. I'll submit a patch tomorrow. -- Simon Riggs EnterpriseDB http://www.enterprisedb.com ---(end of broadcast)--- TIP 1: 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
