Re: [HACKERS] pg_upgrade and toasted pg_largeobject
Alvaro Herrera writes: > Robert Haas wrote: >> On Tue, May 3, 2016 at 2:09 PM, Alvaro Herrera >> wrote: >>> How about backpatching patch 1 all the way back, and putting the others >>> in 9.6? >> Why would we do that? It seems very odd to back-patch a pure >> refactoring - isn't that taking a risk for no benefit? Yeah, I don't see the point of that either. > My inclination is actually to put the whole series back to 9.2, but if > we don't want to do that, then backpatching just the first one seems to > make pg_upgrade more amenable to future bugfixes. I checked, and found that patch 1 doesn't apply cleanly before 9.5. I've not looked into exactly why not, but it would possibly take some work to adapt these patches to older branches. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_upgrade and toasted pg_largeobject
Robert Haas wrote: > On Tue, May 3, 2016 at 2:09 PM, Alvaro Herrera > wrote: > > Tom Lane wrote: > >> Any thoughts what to do with this? We could decide that it's a bug fix > >> and backpatch, or decide that it's a new feature and delay till 9.7, > >> or decide that it's a minor bug fix and add it to 9.6 only. I kinda lean > >> towards the last alternative. > > > > How about backpatching patch 1 all the way back, and putting the others > > in 9.6? > > Why would we do that? It seems very odd to back-patch a pure > refactoring - isn't that taking a risk for no benefit? >From Tom's description, what is there works by chance only, and maybe not even in all cases. The rest of the patches are to fix one particular problem, which perhaps is not overly serious, but maybe some other future problem will be discovered and we will want to have patch 1 installed. My inclination is actually to put the whole series back to 9.2, but if we don't want to do that, then backpatching just the first one seems to make pg_upgrade more amenable to future bugfixes. (I say "back to 9.2" instead of "back to 9.1" because surely we don't care all that much about upgrades *to* 9.1, since it's going unsupported soon.) -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_upgrade and toasted pg_largeobject
On Tue, May 3, 2016 at 2:09 PM, Alvaro Herrera wrote: > Tom Lane wrote: >> Any thoughts what to do with this? We could decide that it's a bug fix >> and backpatch, or decide that it's a new feature and delay till 9.7, >> or decide that it's a minor bug fix and add it to 9.6 only. I kinda lean >> towards the last alternative. > > How about backpatching patch 1 all the way back, and putting the others > in 9.6? Why would we do that? It seems very odd to back-patch a pure refactoring - isn't that taking a risk for no benefit? -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_upgrade and toasted pg_largeobject
Tom Lane wrote: > Any thoughts what to do with this? We could decide that it's a bug fix > and backpatch, or decide that it's a new feature and delay till 9.7, > or decide that it's a minor bug fix and add it to 9.6 only. I kinda lean > towards the last alternative. How about backpatching patch 1 all the way back, and putting the others in 9.6? -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_upgrade and toasted pg_largeobject
On Tue, May 3, 2016 at 1:38 PM, Tom Lane wrote: > Any thoughts what to do with this? We could decide that it's a bug fix > and backpatch, or decide that it's a new feature and delay till 9.7, > or decide that it's a minor bug fix and add it to 9.6 only. I kinda lean > towards the last alternative. I'm fine with that. (But I haven't reviewed the code.) -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_upgrade and toasted pg_largeobject
I wrote: > Alvaro Herrera writes: >> I'm happy with the solution that pg_upgrade has a step in the check >> stage that says "catalog XYZ has a toast table but shouldn't, aborting >> the upgrade". (Well, not _happy_, but at least it's a lot easier to >> diagnose). > I think though that you're defining the problem too narrowly by putting > forward a solution that would result in an error message like that. > If we're going to do anything here at all, I think the code should emit > a list of the names of relations that it was unable to match up, rather > than trying (and likely failing) to be smart about why. To take just > one reason why, the issue might be that there were too many rels in > the new installation, not too few. I took a break from release-note writing and hacked up something for this; see attached patch series. pg_upgrade-fix-0001.patch attempts to turn get_rel_infos()'s data collection query into less of an unmaintainable mess. In the current code, the "regular_heap" CTE doesn't fetch heap OIDs: it collects some index OIDs as well. The "all_indexes" CTE doesn't fetch all index OIDs: it only fetches OIDs for toast-table indexes (although this is far from obvious to the naked eye, since it reads both the regular_heap and toast_heap CTEs; it's only when you notice that it's subsequently ignoring tables with zero reltoastrelid, and therefore that having read the toast_heap CTE was totally useless, that you realize this). Also it forgets to check indisready, so it's fortunate that it doesn't fetch anything but toast-table indexes, which are unlikely to be in !indisready state. Only the toast_heap CTE actually does what it says, though rather inefficiently since it's uselessly looking for toast tables attached to indexes. The comments that aren't outright wrong are variously misleading, repetitive, badly placed, and/or ungrammatical. I'd like to commit this even if we don't use the rest, because what's there now is not a fit basis for further development. pg_upgrade-fix-0002.patch expands the data collection query to collect the OID of the table associated with each index, and the OID of the base table for each toast table. This isn't needed for pg_upgrade's other processing but we need it in order to usefully identify mismatched tables. pg_upgrade-fix-0003.patch actually changes gen_db_file_maps() so that it prints identifying information about each unmatched relation. pg_upgrade-fix-0004.patch isn't meant to get committed; it's for testing the previous patches. It hacks the pg_upgrade test script to cause pg_largeobject in the source DB to acquire a toast table, thereby replicating the situation Alvaro complained of. With that hack in place, I get ... Copying user relation files No match found in new cluster for old relation with OID 13270 in database "postgres": "pg_toast.pg_toast_2613", toast table for "pg_catalog.pg_largeobject" No match found in new cluster for old relation with OID 13272 in database "postgres": "pg_toast.pg_toast_2613_index", index on "pg_toast.pg_toast_2613", toast table for "pg_catalog.pg_largeobject" Failed to match up old and new tables in database "postgres" Failure, exiting + rm -rf /tmp/pg_upgrade_check-Lui8zH make: *** [check] Error 1 which illustrates the output this will produce for a mismatch. (If anyone wants to bikeshed the output wording, feel free.) Any thoughts what to do with this? We could decide that it's a bug fix and backpatch, or decide that it's a new feature and delay till 9.7, or decide that it's a minor bug fix and add it to 9.6 only. I kinda lean towards the last alternative. regards, tom lane diff --git a/src/bin/pg_upgrade/info.c b/src/bin/pg_upgrade/info.c index 5d83455..0653ff1 100644 *** a/src/bin/pg_upgrade/info.c --- b/src/bin/pg_upgrade/info.c *** get_db_infos(ClusterInfo *cluster) *** 301,311 /* * get_rel_infos() * ! * gets the relinfos for all the user tables of the database referred ! * by "db". * ! * NOTE: we assume that relations/entities with oids greater than ! * FirstNormalObjectId belongs to the user */ static void get_rel_infos(ClusterInfo *cluster, DbInfo *dbinfo) --- 301,311 /* * get_rel_infos() * ! * gets the relinfos for all the user tables and indexes of the database ! * referred to by "dbinfo". * ! * Note: the resulting RelInfo array is assumed to be sorted by OID. ! * This allows later processing to match up old and new databases efficiently. */ static void get_rel_infos(ClusterInfo *cluster, DbInfo *dbinfo) *** get_rel_infos(ClusterInfo *cluster, DbIn *** 330,415 char *last_namespace = NULL, *last_tablespace = NULL; /* * pg_largeobject contains user data that does not appear in pg_dump ! * --schema-only output, so we have to copy that system table heap and ! * index. We could grab the pg_largeobject oids from template1, but it is ! * easy to treat it as a n
Re: [HACKERS] pg_upgrade and toasted pg_largeobject
Alvaro Herrera writes: > Robert Haas wrote: >> On Mon, May 2, 2016 at 12:30 PM, Alvaro Herrera >> wrote: >>> A customer of ours was unable to pg_upgrade a database, with this error: old and new databases "postgres" have a mismatched number of relations >>> After some research, it turned out that pg_largeobject had acquired a >>> toast table. >> I think that if you use -O, and it breaks something, you get to keep >> both pieces. > I'm happy with the solution that pg_upgrade has a step in the check > stage that says "catalog XYZ has a toast table but shouldn't, aborting > the upgrade". (Well, not _happy_, but at least it's a lot easier to > diagnose). I agree with Robert that this is not something we should expend a huge amount of effort on, but I also agree that that error message is inadequate if the situation is not a nobody-could-ever-hit-this case. I think though that you're defining the problem too narrowly by putting forward a solution that would result in an error message like that. If we're going to do anything here at all, I think the code should emit a list of the names of relations that it was unable to match up, rather than trying (and likely failing) to be smart about why. To take just one reason why, the issue might be that there were too many rels in the new installation, not too few. The matching probably does need to be smart about toast tables to the extent of regarding them as "toast table belonging to relation FOO", since just printing their actual names would be unhelpful in most cases. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_upgrade and toasted pg_largeobject
Robert Haas wrote: > On Mon, May 2, 2016 at 12:30 PM, Alvaro Herrera > wrote: > > A customer of ours was unable to pg_upgrade a database, with this error: > > > > old and new databases "postgres" have a mismatched number of relations > > Failure, exiting > > > > After some research, it turned out that pg_largeobject had acquired a > > toast table. After some more research, we determined that it was > > because right after initdb of the old database (months or years prior) > > they moved pg_largeobject to another, slower tablespace, because for > > their case it is very bulky and not used as much as the other data. > > (This requires restarting postmaster with the -O parameter). > I think that if you use -O, and it breaks something, you get to keep > both pieces. pg_largeobject is a big problem, and we should replace > it with something better. And maybe in the meantime we should support > moving it to a different tablespace. But if it's not officially > supported and you do it anyway, I don't think it's pg_upgrade's job to > cope. I'm happy with the solution that pg_upgrade has a step in the check stage that says "catalog XYZ has a toast table but shouldn't, aborting the upgrade". (Well, not _happy_, but at least it's a lot easier to diagnose). -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_upgrade and toasted pg_largeobject
On Mon, May 2, 2016 at 12:30 PM, Alvaro Herrera wrote: > A customer of ours was unable to pg_upgrade a database, with this error: > > old and new databases "postgres" have a mismatched number of relations > Failure, exiting > > After some research, it turned out that pg_largeobject had acquired a > toast table. After some more research, we determined that it was > because right after initdb of the old database (months or years prior) > they moved pg_largeobject to another, slower tablespace, because for > their case it is very bulky and not used as much as the other data. > (This requires restarting postmaster with the -O parameter). > > While I understand that manual system catalog modifications are frowned > upon, it seems to me that we should handle this better. The failure is > very confusing and hard to diagnose, and hard to fix also. I think that if you use -O, and it breaks something, you get to keep both pieces. pg_largeobject is a big problem, and we should replace it with something better. And maybe in the meantime we should support moving it to a different tablespace. But if it's not officially supported and you do it anyway, I don't think it's pg_upgrade's job to cope. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] pg_upgrade and toasted pg_largeobject
Hi, A customer of ours was unable to pg_upgrade a database, with this error: old and new databases "postgres" have a mismatched number of relations Failure, exiting After some research, it turned out that pg_largeobject had acquired a toast table. After some more research, we determined that it was because right after initdb of the old database (months or years prior) they moved pg_largeobject to another, slower tablespace, because for their case it is very bulky and not used as much as the other data. (This requires restarting postmaster with the -O parameter). While I understand that manual system catalog modifications are frowned upon, it seems to me that we should handle this better. The failure is very confusing and hard to diagnose, and hard to fix also. -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
