Thread

1. Ignore invalid indexes in pg_dump

   Michael Paquier <michael.paquier@gmail.com> — 2013-03-20T02:51:05Z

   Hi,
   
   If failures happen with CREATE INDEX CONCURRENTLY, the system will be let
   with invalid indexes. I don't think that the user would like to see invalid
   indexes of
   an existing system being recreated as valid after a restore.
   So why not removing from a dump invalid indexes with something like the
   patch
   attached?
   This should perhaps be applied in pg_dump for versions down to 8.2 where
   CREATE
   INDEX CONCURRENTLY has been implemented?
   
   I noticed some recent discussions about that:
   http://www.postgresql.org/message-id/20121207141236.GB4699@alvh.no-ip.org
   In this case the problem has been fixed in pg_upgrade directly.
   
   -- 
   Michael
   

2. Re: Ignore invalid indexes in pg_dump

   Simon Riggs <simon@2ndquadrant.com> — 2013-03-20T09:00:05Z

   On 20 March 2013 02:51, Michael Paquier <michael.paquier@gmail.com> wrote:
   
   > If failures happen with CREATE INDEX CONCURRENTLY, the system will be let
   > with invalid indexes. I don't think that the user would like to see invalid
   > indexes of
   > an existing system being recreated as valid after a restore.
   > So why not removing from a dump invalid indexes with something like the
   > patch
   > attached?
   > This should perhaps be applied in pg_dump for versions down to 8.2 where
   > CREATE
   > INDEX CONCURRENTLY has been implemented?
   
   Invalid also means currently-in-progress, so it would be better to keep them in.
   
   > I noticed some recent discussions about that:
   > http://www.postgresql.org/message-id/20121207141236.GB4699@alvh.no-ip.org
   > In this case the problem has been fixed in pg_upgrade directly.
   
   That is valid because the index build is clearly not in progress.
   
   -- 
    Simon Riggs                   http://www.2ndQuadrant.com/
    PostgreSQL Development, 24x7 Support, Training & Services
   
   
   

3. Re: Ignore invalid indexes in pg_dump

   Josh Kupershmidt <schmiddy@gmail.com> — 2013-03-20T15:38:58Z

   On Wed, Mar 20, 2013 at 2:00 AM, Simon Riggs <simon@2ndquadrant.com> wrote:
   > On 20 March 2013 02:51, Michael Paquier <michael.paquier@gmail.com> wrote:
   >
   >> If failures happen with CREATE INDEX CONCURRENTLY, the system will be let
   >> with invalid indexes. I don't think that the user would like to see invalid
   >> indexes of
   >> an existing system being recreated as valid after a restore.
   >> So why not removing from a dump invalid indexes with something like the
   >> patch
   >> attached?
   >> This should perhaps be applied in pg_dump for versions down to 8.2 where
   >> CREATE
   >> INDEX CONCURRENTLY has been implemented?
   >
   > Invalid also means currently-in-progress, so it would be better to keep them in.
   
   For invalid indexes which are left hanging around in the database, if
   the index definition is included by pg_dump, it will likely cause pain
   during the restore. If the index build failed the first time and
   hasn't been manually dropped and recreated since then, it's a good bet
   it will fail the next time. Errors during restore can be more than
   just a nuisance; consider restores with --single-transaction.
   
   And if the index is simply currently-in-progress, it seems like the
   expected behavior would be for pg_dump to ignore it anyway. We don't
   include other DDL objects which are not yet committed while pg_dump is
   running.
   
   Josh
   
   
   

4. Re: Ignore invalid indexes in pg_dump

   Tom Lane <tgl@sss.pgh.pa.us> — 2013-03-20T15:58:26Z

   Josh Kupershmidt <schmiddy@gmail.com> writes:
   > On Wed, Mar 20, 2013 at 2:00 AM, Simon Riggs <simon@2ndquadrant.com> wrote:
   >> Invalid also means currently-in-progress, so it would be better to keep them in.
   
   > For invalid indexes which are left hanging around in the database, if
   > the index definition is included by pg_dump, it will likely cause pain
   > during the restore. If the index build failed the first time and
   > hasn't been manually dropped and recreated since then, it's a good bet
   > it will fail the next time. Errors during restore can be more than
   > just a nuisance; consider restores with --single-transaction.
   
   > And if the index is simply currently-in-progress, it seems like the
   > expected behavior would be for pg_dump to ignore it anyway. We don't
   > include other DDL objects which are not yet committed while pg_dump is
   > running.
   
   I had been on the fence about what to do here, but I find Josh's
   arguments persuasive, particularly the second one.  Why shouldn't we
   consider an in-progress index to be an uncommitted DDL change?
   
   (Now admittedly, there won't *be* any uncommitted ordinary DDL on tables
   while pg_dump is running, because it takes AccessShareLock on all
   tables.  But there could easily be uncommitted DDL against other types
   of database objects, which pg_dump won't even see.)
   
   			regards, tom lane
   
   
   

5. Re: Ignore invalid indexes in pg_dump

   Michael Paquier <michael.paquier@gmail.com> — 2013-03-21T00:51:39Z

   On Thu, Mar 21, 2013 at 12:58 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
   
   > I had been on the fence about what to do here, but I find Josh's
   > arguments persuasive, particularly the second one.  Why shouldn't we
   > consider an in-progress index to be an uncommitted DDL change?
   >
   > (Now admittedly, there won't *be* any uncommitted ordinary DDL on tables
   > while pg_dump is running, because it takes AccessShareLock on all
   > tables.  But there could easily be uncommitted DDL against other types
   > of database objects, which pg_dump won't even see.)
   >
   +1. Playing it safe is a better thing to do for sure, especially if a
   restore would
   fail. I didn't think about that first...
   
   On top of checking indisvalid, I think that some additional checks on
   indislive
   and indisready are also necessary. As indisready has been introduced in 8.3
   and
   indislive has been added in 9.3, the attached patch is good I think.
   I also added a note in the documentation about invalid indexes not being
   dumped.
   Perhaps this patch should be backpatched to previous versions in order to
   have
   the same consistent behavior.
   
   Regards,
   -- 
   Michael
   

6. Re: Ignore invalid indexes in pg_dump

   Tom Lane <tgl@sss.pgh.pa.us> — 2013-03-26T21:47:30Z

   Michael Paquier <michael.paquier@gmail.com> writes:
   > On top of checking indisvalid, I think that some additional checks on
   > indislive and indisready are also necessary.
   
   Those are not necessary, as an index that is marked indisvalid should
   certainly also have those flags set.  If it didn't require making two
   new version distinctions in getIndexes(), I'd be okay with the extra
   checks; but as-is I think the maintenance pain this would add greatly
   outweighs any likely value.
   
   I've committed this in the simpler form that just adds indisvalid
   checks to the appropriate version cases.
   
   			regards, tom lane
   
   
   

7. Re: Ignore invalid indexes in pg_dump

   Michael Paquier <michael.paquier@gmail.com> — 2013-03-26T23:19:22Z

   On Wed, Mar 27, 2013 at 6:47 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
   
   > Michael Paquier <michael.paquier@gmail.com> writes:
   > > On top of checking indisvalid, I think that some additional checks on
   > > indislive and indisready are also necessary.
   >
   > Those are not necessary, as an index that is marked indisvalid should
   > certainly also have those flags set.  If it didn't require making two
   > new version distinctions in getIndexes(), I'd be okay with the extra
   > checks; but as-is I think the maintenance pain this would add greatly
   > outweighs any likely value.
   >
   > I've committed this in the simpler form that just adds indisvalid
   > checks to the appropriate version cases.
   >
   Thanks.
   -- 
   Michael