Thread: Re: [PATCHES] Writing WAL for relcache invalidation: pg_internal.init
"Simon Riggs" <simon@2ndquadrant.com> 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.
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 ...
regards, tom lane
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
On Wed, 2006-11-01 at 12:05 -0500, Tom Lane wrote: > "Simon Riggs" <simon@2ndquadrant.com> 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
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" <simon@2ndquadrant.com> writes: > On Wed, 2006-11-01 at 12:05 -0500, Tom Lane wrote: > > > "Simon Riggs" <simon@2ndquadrant.com> 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 majordomo@postgresql.org so that your > message can get through to the mailing list cleanly > -- ------------------------------------------------------------------------------- Jerry Sievers 305 854-3001 (home) WWW ECommerce Consultant 305 321-1144 (mobile http://www.JerrySievers.com/
On Thu, 2006-11-02 at 09:36 -0500, Jerry Sievers wrote: > Is it safe to delete the .init files now (before starting recovery), > or perhaps unconditionally right after going live? You're safe to delete those files from the restored version of your base backup prior to commencing startup with a recovery.conf. When they are missing they will be recreated automatically. When we agree on how to fix it, which should be soon, I would imagine that the fix can be back patched to 8.0 and 8.1 and fixed prior to release in 8.2. -- Simon Riggs EnterpriseDB http://www.enterprisedb.com
"Simon Riggs" <simon@2ndquadrant.com> 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
Jerry Sievers <jerry@jerrysievers.com> 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
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
Attachment
"Simon Riggs" <simon@2ndquadrant.com> 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