Re: [HACKERS] Fix for pg_upgrade and invalid indexes
On Fri, Mar 29, 2013 at 07:03:05PM -0400, Tom Lane wrote:
> Andres Freund writes:
> > Those columns cannot be NULL, so using IS DISTINCT FROM seems a bit
> > clumsy.
>
> That was what I started to write, too, but actually I think the IS
> DISTINCT is correct and the RIGHT JOIN should be a LEFT JOIN. Note
> that the query appears to be intended to collect regular tables as
> well as indexes. (As patched, that's totally broken, so I infer
> Bruce hasn't tested it yet.)
Yes, I only ran my simple tests so far --- I wanted to at least get some
eyes on it. I was wondering if we ever need to use parentheses for
queries that mix normal and outer joins? I am unclear on that.
Attached is a fixed patch that uses LEFT JOIN. I went back and looked
at the patch that added this test and I think the patch is now complete.
I would like to apply it tomorrow/Saturday so it will be ready for
Monday's packaging, and get some buildfarm time on it.
--
Bruce Momjian http://momjian.us
EnterpriseDB http://enterprisedb.com
+ It's impossible for everything to be true. +
diff --git a/contrib/pg_upgrade/check.c b/contrib/pg_upgrade/check.c
new file mode 100644
index 65fb548..35783d0
*** a/contrib/pg_upgrade/check.c
--- b/contrib/pg_upgrade/check.c
*** static void check_is_super_user(ClusterI
*** 20,26
static void check_for_prepared_transactions(ClusterInfo *cluster);
static void check_for_isn_and_int8_passing_mismatch(ClusterInfo *cluster);
static void check_for_reg_data_type_usage(ClusterInfo *cluster);
- static void check_for_invalid_indexes(ClusterInfo *cluster);
static void get_bin_version(ClusterInfo *cluster);
static char *get_canonical_locale_name(int category, const char *locale);
--- 20,25
*** check_and_dump_old_cluster(bool live_che
*** 97,103
check_is_super_user(&old_cluster);
check_for_prepared_transactions(&old_cluster);
check_for_reg_data_type_usage(&old_cluster);
- check_for_invalid_indexes(&old_cluster);
check_for_isn_and_int8_passing_mismatch(&old_cluster);
/* old = PG 8.3 checks? */
--- 96,101
*** check_for_reg_data_type_usage(ClusterInf
*** 952,1046
"%s\n\n", output_path);
}
else
- check_ok();
- }
-
-
- /*
- * check_for_invalid_indexes()
- *
- * CREATE INDEX CONCURRENTLY can create invalid indexes if the index build
- * fails. These are dumped as valid indexes by pg_dump, but the
- * underlying files are still invalid indexes. This checks to make sure
- * no invalid indexes exist, either failed index builds or concurrent
- * indexes in the process of being created.
- */
- static void
- check_for_invalid_indexes(ClusterInfo *cluster)
- {
- int dbnum;
- FILE *script = NULL;
- bool found = false;
- char output_path[MAXPGPATH];
-
- prep_status("Checking for invalid indexes from concurrent index builds");
-
- snprintf(output_path, sizeof(output_path), "invalid_indexes.txt");
-
- for (dbnum = 0; dbnum < cluster->dbarr.ndbs; dbnum++)
- {
- PGresult *res;
- bool db_used = false;
- int ntups;
- int rowno;
- int i_nspname,
- i_relname;
- DbInfo *active_db = &cluster->dbarr.dbs[dbnum];
- PGconn *conn = connectToServer(cluster, active_db->db_name);
-
- res = executeQueryOrDie(conn,
- "SELECT n.nspname, c.relname "
- "FROM pg_catalog.pg_class c, "
- " pg_catalog.pg_namespace n, "
- " pg_catalog.pg_index i "
- "WHERE (i.indisvalid = false OR "
- " i.indisready = false) AND "
- " i.indexrelid = c.oid AND "
- " c.relnamespace = n.oid AND "
- /* we do not migrate these, so skip them */
- " n.nspname != 'pg_catalog' AND "
- " n.nspname != 'information_schema' AND "
- /* indexes do not have toast tables */
- " n.nspname != 'pg_toast'");
-
- ntups = PQntuples(res);
- i_nspname = PQfnumber(res, "nspname");
- i_relname = PQfnumber(res, "relname");
- for (rowno = 0; rowno < ntups; rowno++)
- {
- found = true;
- if (script == NULL && (script = fopen_priv(output_path, "w")) == NULL)
- pg_log(PG_FATAL, "Could not open file \"%s\": %s\n",
- output_path, getErrorText(errno));
- if (!db_used)
- {
- fprintf(script, "Database: %s\n", active_db->db_name);
- db_used = true;
- }
- fprintf(script, " %s.%s\n",
- PQgetvalue(res, rowno, i_nspname),
- PQgetvalue(res, rowno, i_relname));
- }
-
- PQclear(res);
-
- PQfinish(conn);
- }
-
- if (script)
- fclose(script);
-
- if (found)
- {
- pg_log(PG_REPORT, "fatal\n");
- pg_log(PG_FATAL,
- "Your installation contains invalid indexes due to failed or\n"
- "currently running CREATE INDEX CONCURRENTLY operations. You\n"
- "cannot upgrade until these indexes are valid or removed. A\n"
- "list of the problem indexes is in the file:\n"
- "%s\n\n", output_
Re: [HACKERS] Getting to 9.3 beta
On Friday, March 29, 2013 11:04 PM Andres Freund wrote: > On 2013-03-29 12:28:59 -0400, Tom Lane wrote: > > Andres Freund writes: > > > On 2013-03-29 10:15:42 -0400, Bruce Momjian wrote: > > >> What is a reasonable timeframe to target for completion of these > items? > > > > > Here's my take on it: > > > > Thanks for annotating these! I've commented on the ones where I > > come to a different conclusion: > > > > > - replace plugins directory with GUC > > > Peter has agreed to boot it to the next fest afaics > > > (515357f4.6000...@gmx.net) > > > => move (done) > > > > It looks to me more like a RWF situation, since Peter has evidently > lost > > interest in committing this feature at all. So I marked it that way. > > Yea, after a second reading of Peter's email I agree. > > > > - Patch to compute Max LSN of Data Pages: > > > I *personally* don't really see a point in including it in > postgres, > > > there doesn't really seem to be any real demand met by the tool > > > => reject? > > > > Not sure. It certainly hasn't drawn that much enthusiasm on the > list, > > but it strikes me as the sort of thing that when you need it you > really > > need it. It would be interesting to hear opinions about it from > people > > who deal with data recovery situations on a regular basis. > > I only have done so infrequently, but I guess my real problem is that I > can't see it being all that helpful in such a situation. If you've lost > pg_xlog - otherwise there doesn't seem to be much use in this - you can > just use > pg_resetxlog to set an insanely high lsn and then dump & restore the > database. This utility can give user value that is okay to proceed. > The only reason to use something more complex than this is if > you want to continue using the cluster which seems like a really bad > idea. I agree that mostly user doesn't need to use cluster further, but incase if he is aware that only some part of the database (particular tablespace) has problem and he can take dump of that part of database and redo some of lost operations on other tablespaces. With Regards, Amit Kapila. -- 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] Hash Join cost estimates
On Fri, 2013-03-29 at 16:37 -0400, Tom Lane wrote: > Jeff Davis writes: > > Yes, I have run into this issue (or something very similar). I don't > > understand why the bucketsize even matters much -- assuming few hash > > collisions, we are not actually evaluating the quals any more times than > > necessary. So why all of the hashjoin-specific logic in determining the > > number of qual evaluations? The only reason I can think of is to model > > the cost of comparing the hashes themselves. > > I think the point is that there may *not* be few hash collisions ... In Stephen's case the table was only 41KB, so something still seems off. Maybe we should model the likelihood of a collision based on the cardinalities (assuming a reasonably good hash function)? Also, I think I found an important assumption that seems dubious (in comment for estimate_hash_bucketsize()): "If the other relation in the join has a key distribution similar to this one's, then the most-loaded buckets are exactly those that will be probed most often. Therefore, the "average" bucket size for costing purposes should really be taken as something close to the "worst case" bucket size. We try to estimate this by adjusting the fraction if there are too few distinct data values, and then scaling up by the ratio of the most common value's frequency to the average frequency." But the key distribution is not necessarily similar at all... the large table might have many more distinct values. Stephen, do you think this could explain your problem? Regards, Jeff Davis -- 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] [COMMITTERS] pgsql: Add parallel pg_dump option.
On 03/29/2013 03:12 PM, David Fetter wrote: On Sun, Mar 24, 2013 at 03:39:44PM +, Andrew Dunstan wrote: Add parallel pg_dump option. This is great! While testing, I noticed that the only supported -F option when -j is specified is directory, which is fine as far as it goes, but I think it would be easier on users if there were some default like ./pg_dump when -j is specified. It could of course be overridden by -Fd. What say? What you really want is to supply a default for --file in this case. It would seem very odd to do so when --jobs is specified but not when it isn't. You already need to specify a --file value when --format=directory is in use, and the addition of --jobs has not changed that; neither has the fact that --jobs requires --format=directory to be specified. Given that, this can be seen as a feature request that could be considered for 9.4 (although I'd be skeptical, TBH), but it surely isn't something that's broken and needs to be fixed, and we're long past the point where we should be adding new design for 9.3. cheers andrew -- 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] [COMMITTERS] pgsql: Add sql_drop event for event triggers
Alvaro Herrera writes: > Add sql_drop event for event triggers The buildfarm members that use -DCLOBBER_CACHE_ALWAYS don't like this patch: *** *** 760,771 FROM generate_series(1, 50) a; BEGIN; SET TRANSACTION ISOLATION LEVEL SERIALIZABLE; UPDATE serializable_update_tab SET description = 'no no', id = 1 WHERE id = 1; COMMIT; SELECT description FROM serializable_update_tab WHERE id = 1; ! description ! ! updated in trigger (1 row) DROP TABLE serializable_update_tab; --- 760,773 FROM generate_series(1, 50) a; BEGIN; SET TRANSACTION ISOLATION LEVEL SERIALIZABLE; + ERROR: SET TRANSACTION ISOLATION LEVEL must be called before any query UPDATE serializable_update_tab SET description = 'no no', id = 1 WHERE id = 1; + ERROR: current transaction is aborted, commands ignored until end of transaction block COMMIT; SELECT description FROM serializable_update_tab WHERE id = 1; ! description ! - ! new (1 row) DROP TABLE serializable_update_tab; I suspect you have inserted a snapshot-capturing operation into someplace it mustn't go during transaction startup, but only in a path that is triggered by an immediately preceding cache flush. 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] Getting to 9.3 beta
On Fri, Mar 29, 2013 at 8:22 AM, Andres Freund wrote: > - pg_stat_statements: query, session, and eviction identification: > Seems to need at least docs > => wait for author, seems to be easy enough? I would have responded by now, but recent events have unfortunately made me put a lot of things that take longer than fifteen minutes on hold. Still, I am watching. Like you say, it's not terribly invasive. The under-estimation error propagation is perhaps a bit too weird to justify (even though it's a simple implementation). -- fdr -- 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] Fix for pg_upgrade and invalid indexes
Andres Freund writes: > Those columns cannot be NULL, so using IS DISTINCT FROM seems a bit > clumsy. That was what I started to write, too, but actually I think the IS DISTINCT is correct and the RIGHT JOIN should be a LEFT JOIN. Note that the query appears to be intended to collect regular tables as well as indexes. (As patched, that's totally broken, so I infer Bruce hasn't tested it yet.) 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] Fix for pg_upgrade and invalid indexes
On 2013-03-29 16:57:06 -0400, Bruce Momjian wrote:
> On Thu, Mar 28, 2013 at 05:27:28PM -0400, Tom Lane wrote:
> > Bruce Momjian writes:
> > > Should I just patch pg_upgrade to remove the "indisvalid", skip
> > > "indisvalid" indexes, and backpatch it? Users should be using the
> > > version of pg_upgrade to match new pg_dump. Is there any case where
> > > they don't match? Do I still need to check for "indisready"?
> >
> > Yeah, if you can just ignore !indisvalid indexes that should work fine.
> > I see no need to look at indisready if you're doing that.
>
> Attached is a patch that implements the suggested pg_upgrade changes of
> not copying invalid indexes now that pg_dump doesn't dump them. This
> should be backpatched back to 8.4 to match pg_dump. It might require
> release note updates; not sure. Previously pg_upgrade threw an error
> if invalid indexes exist, but only since February, when we released the
> pg_upgrade fix to do this. You can see the majority of this patch is
> removing that check.
> + "RIGHT OUTER JOIN pg_catalog.pg_index i "
> + " ON (c.oid = i.indexrelid) "
>"WHERE relkind IN ('r', 'm', 'i'%s) AND "
> + /* pg_dump only dumps valid indexes; testing
> indisready is
> + * necessary in 9.2, and harmless in earlier/later
> versions. */
> + " i.indisvalid IS DISTINCT FROM false AND "
> + " i.indisready IS DISTINCT FROM false AND "
> /* exclude possible orphaned temp tables */
>" ((n.nspname !~ '^pg_temp_' AND "
>"n.nspname !~ '^pg_toast_temp_' AND "
Those columns cannot be NULL, so using IS DISTINCT FROM seems a bit
clumsy.
Greetings,
Andres Freund
--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, 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] Fix for pg_upgrade and invalid indexes
Bruce Momjian writes: > Attached is a patch that implements the suggested pg_upgrade changes of > not copying invalid indexes now that pg_dump doesn't dump them. This > should be backpatched back to 8.4 to match pg_dump. It might require > release note updates; not sure. Previously pg_upgrade threw an error > if invalid indexes exist, but only since February, when we released the > pg_upgrade fix to do this. You can see the majority of this patch is > removing that check. Surely that should be LEFT JOIN not RIGHT JOIN? 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] Getting to 9.3 beta
On 2013/03/30, at 2:33, Andres Freund wrote: > On 2013-03-29 12:28:59 -0400, Tom Lane > >>> - REINDEX CONCURRENTLY: >>> Imo pretty close to being comittable and pretty useful, but it got >>> redesigned pretty late and it mostly had review from me and fujii and >>> it could use a bit more input >>> => unclear >> >> I think this really has no hope of being bulletproof until we have >> MVCC-based catalog scans. The work up to now has been good exploratory >> effort, but it's not going to be committable without that >> infrastructure, IMHO anyway. > > I think its fair not to commit it in 9.3, the patch came late and has > undergone significant issues since then. And there are some things in it > I am not sure about yet. > But I am not sure why its only acceptable with MVCC scans? Sure, it > takes an exclusive lock for a very short time when switching the > relations, but thats still a huge step up over the status quo: > * toast indexes currently cannot be created/dropped at all. So you have > to reindex the table which will need to be locked over the whole time. > * you currently cannot replace indexes that are referenced by foreign > keys without doing manual catalog hackery which is definitely unsafe > * you cannot create constraints that use an index concurrently. Parts of > that is emulatable by creating the constraint with an existing index, > but that doesn't help e.g. for exclusion constraints. > * there's no convenient way to issue CREATE INDEX CONCURRENLTY new_index; DROP > INDEX CONCURRENTLY old_index; RENAME new_index old_index; for all indexes. Thanks for the detailed explanation! Just adding that this patch needs more review and attention especially for the toast part where multiple indexes are handled. As it looks that it is too late for 9.3, well let's move it to the next commit fest and work on that in the next release cycle. Thanks, -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Fix for pg_upgrade and invalid indexes
On Thu, Mar 28, 2013 at 05:27:28PM -0400, Tom Lane wrote:
> Bruce Momjian writes:
> > Should I just patch pg_upgrade to remove the "indisvalid", skip
> > "indisvalid" indexes, and backpatch it? Users should be using the
> > version of pg_upgrade to match new pg_dump. Is there any case where
> > they don't match? Do I still need to check for "indisready"?
>
> Yeah, if you can just ignore !indisvalid indexes that should work fine.
> I see no need to look at indisready if you're doing that.
Attached is a patch that implements the suggested pg_upgrade changes of
not copying invalid indexes now that pg_dump doesn't dump them. This
should be backpatched back to 8.4 to match pg_dump. It might require
release note updates; not sure. Previously pg_upgrade threw an error
if invalid indexes exist, but only since February, when we released the
pg_upgrade fix to do this. You can see the majority of this patch is
removing that check.
--
Bruce Momjian http://momjian.us
EnterpriseDB http://enterprisedb.com
+ It's impossible for everything to be true. +
diff --git a/contrib/pg_upgrade/check.c b/contrib/pg_upgrade/check.c
new file mode 100644
index 65fb548..35783d0
*** a/contrib/pg_upgrade/check.c
--- b/contrib/pg_upgrade/check.c
*** static void check_is_super_user(ClusterI
*** 20,26
static void check_for_prepared_transactions(ClusterInfo *cluster);
static void check_for_isn_and_int8_passing_mismatch(ClusterInfo *cluster);
static void check_for_reg_data_type_usage(ClusterInfo *cluster);
- static void check_for_invalid_indexes(ClusterInfo *cluster);
static void get_bin_version(ClusterInfo *cluster);
static char *get_canonical_locale_name(int category, const char *locale);
--- 20,25
*** check_and_dump_old_cluster(bool live_che
*** 97,103
check_is_super_user(&old_cluster);
check_for_prepared_transactions(&old_cluster);
check_for_reg_data_type_usage(&old_cluster);
- check_for_invalid_indexes(&old_cluster);
check_for_isn_and_int8_passing_mismatch(&old_cluster);
/* old = PG 8.3 checks? */
--- 96,101
*** check_for_reg_data_type_usage(ClusterInf
*** 952,1046
"%s\n\n", output_path);
}
else
- check_ok();
- }
-
-
- /*
- * check_for_invalid_indexes()
- *
- * CREATE INDEX CONCURRENTLY can create invalid indexes if the index build
- * fails. These are dumped as valid indexes by pg_dump, but the
- * underlying files are still invalid indexes. This checks to make sure
- * no invalid indexes exist, either failed index builds or concurrent
- * indexes in the process of being created.
- */
- static void
- check_for_invalid_indexes(ClusterInfo *cluster)
- {
- int dbnum;
- FILE *script = NULL;
- bool found = false;
- char output_path[MAXPGPATH];
-
- prep_status("Checking for invalid indexes from concurrent index builds");
-
- snprintf(output_path, sizeof(output_path), "invalid_indexes.txt");
-
- for (dbnum = 0; dbnum < cluster->dbarr.ndbs; dbnum++)
- {
- PGresult *res;
- bool db_used = false;
- int ntups;
- int rowno;
- int i_nspname,
- i_relname;
- DbInfo *active_db = &cluster->dbarr.dbs[dbnum];
- PGconn *conn = connectToServer(cluster, active_db->db_name);
-
- res = executeQueryOrDie(conn,
- "SELECT n.nspname, c.relname "
- "FROM pg_catalog.pg_class c, "
- " pg_catalog.pg_namespace n, "
- " pg_catalog.pg_index i "
- "WHERE (i.indisvalid = false OR "
- " i.indisready = false) AND "
- " i.indexrelid = c.oid AND "
- " c.relnamespace = n.oid AND "
- /* we do not migrate these, so skip them */
- " n.nspname != 'pg_catalog' AND "
- " n.nspname != 'information_schema' AND "
- /* indexes do not have toast tables */
- " n.nspname != 'pg_toast'");
-
- ntups = PQntuples(res);
- i_nspname = PQfnumber(res, "nspname");
- i_relname = PQfnumber(res, "relname");
- for (rowno = 0; rowno < ntups; rowno++)
- {
- found = true;
- if (script == NULL && (script = fopen_priv(output_path, "w")) == NULL)
- pg_log(PG_FATAL, "Could not open file \"%s\": %s\n",
- output_path, getErrorText(errno));
- if (!db_used)
- {
- fprintf(script, "Database: %s\n", active_db->db_name);
- db_used = true;
- }
- fprintf(script, " %s.%s\n",
- PQgetvalue(res, rowno, i_nspname),
- PQgetvalue(res, rowno, i_relname));
- }
-
- PQclear(res);
-
- PQfinish(conn);
- }
-
- if (script)
- fclose(script);
-
- if (found)
- {
- pg_log(PG_REPORT, "fatal\n");
- pg_log(PG_FATAL,
- "Your installation contains invalid indexes due to failed or\n"
- "currently running CREATE INDEX CONCURRENTLY operations. You\n"
- "cannot upgrade until these indexes are valid or removed. A\n"
- "list of the problem indexes is in the file:\n"
- "%s\n\n", output_path
Re: [HACKERS] Hash Join cost estimates
Jeff Davis writes: > Yes, I have run into this issue (or something very similar). I don't > understand why the bucketsize even matters much -- assuming few hash > collisions, we are not actually evaluating the quals any more times than > necessary. So why all of the hashjoin-specific logic in determining the > number of qual evaluations? The only reason I can think of is to model > the cost of comparing the hashes themselves. I think the point is that there may *not* be few hash collisions ... 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] Hash Join cost estimates
On Thu, 2013-03-28 at 19:56 -0400, Stephen Frost wrote: > 41K hashed, seqscan 4M: 115030.10 + 1229.46 = 116259.56 > 4M hashed, seqscan 41K: 1229.46 + 211156.20 = 212385.66 I think those are backwards -- typo? > In the end, I think the problem here is that we are charging far too > much for these bucket costs (particularly when we're getting them so > far wrong) and not nearly enough for the cost of building the hash > table in the first place. > > Thoughts? Ideas about how we can 'fix' this? Have others run into > similar issues? Yes, I have run into this issue (or something very similar). I don't understand why the bucketsize even matters much -- assuming few hash collisions, we are not actually evaluating the quals any more times than necessary. So why all of the hashjoin-specific logic in determining the number of qual evaluations? The only reason I can think of is to model the cost of comparing the hashes themselves. Also, searching the archives turns up at least one other, but I think I've seen more: http://www.postgresql.org/message-id/a82128a6-4e3b-43bd-858d-21b129f7b...@richrelevance.com Regards, Jeff Davis -- 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] Getting to 9.3 beta
Hello > - plpgsql_check_function: > Tom says (27661.1364267...@sss.pgh.pa.us) that even if the approach > can be aggreed uppon it needs quite a bit more work > => move > > Can we talk about this patch little bit more before moving to next commitfest? I would to have some plan to next commitfest. We have to remove some wanted functionality - possibility to identify more issues in one run, remove warnings, ... or we have significantly refactor plpgsql parser (two stages) Next possibility - introduce some new API and move plpgsql_check_function to external module, although I am thinking so this is important and very missing functionality (and should be in core) still. There is no reply, comments to my last update - where I rewrote output, what was mayor Tom's objection (well specified). Regards Pavel
Re: [HACKERS] [COMMITTERS] pgsql: Add parallel pg_dump option.
On Sun, Mar 24, 2013 at 03:39:44PM +, Andrew Dunstan wrote: > Add parallel pg_dump option. This is great! While testing, I noticed that the only supported -F option when -j is specified is directory, which is fine as far as it goes, but I think it would be easier on users if there were some default like ./pg_dump when -j is specified. It could of course be overridden by -Fd. What say? Cheers, David. -- David Fetter http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david.fet...@gmail.com iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate -- 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] Enabling Checksums
On 3/25/13 8:25 AM, Bruce Momjian wrote: On Fri, Mar 22, 2013 at 11:35:35PM -0500, Jim Nasby wrote: >On 3/20/13 8:41 AM, Bruce Momjian wrote: > >Also, if a users uses checksums in 9.3, could they initdb without > >checksums in 9.4 and use pg_upgrade? As coded, the pg_controldata > >checksum settings would not match and pg_upgrade would throw an > >error, but it might be possible to allow this, i.e. you could go from > >checksum to no checksum initdb clusters, but not from no checksum to > >checksum. I am wondering if the patch should reflect this. > >If the docs don't warn about this, they should, but I don't think it's >the responsibility of this patch to deal with that problem. The reason >I don't believe this patch should deal with it is because that is a >known, rather serious, limitation of pg_upgrade. It's something about >pg_upgrade that just needs to be fixed, regardless of what patches >might make the situation worse. Huh? It wasn't a "serious limitation" of pg_upgrade until this patch. What limitation does pg_upgrade have regardless of this patch? The limitation that it depends on binary compatibility. I suppose it's unfair to say that's a pg_upgrade limitation, but it's a certainly a limitation of Postgres upgrade capability. So far we've been able to skirt the issue but at some point we need to address this. -- 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] Getting to 9.3 beta
On 2013-03-29 12:28:59 -0400, Tom Lane wrote: > Andres Freund writes: > > On 2013-03-29 10:15:42 -0400, Bruce Momjian wrote: > >> What is a reasonable timeframe to target for completion of these items? > > > Here's my take on it: > > Thanks for annotating these! I've commented on the ones where I > come to a different conclusion: > > > - replace plugins directory with GUC > > Peter has agreed to boot it to the next fest afaics > > (515357f4.6000...@gmx.net) > > => move (done) > > It looks to me more like a RWF situation, since Peter has evidently lost > interest in committing this feature at all. So I marked it that way. Yea, after a second reading of Peter's email I agree. > > - Patch to compute Max LSN of Data Pages: > > I *personally* don't really see a point in including it in postgres, > > there doesn't really seem to be any real demand met by the tool > > => reject? > > Not sure. It certainly hasn't drawn that much enthusiasm on the list, > but it strikes me as the sort of thing that when you need it you really > need it. It would be interesting to hear opinions about it from people > who deal with data recovery situations on a regular basis. I only have done so infrequently, but I guess my real problem is that I can't see it being all that helpful in such a situation. If you've lost pg_xlog - otherwise there doesn't seem to be much use in this - you can just use pg_resetxlog to set an insanely high lsn and then dump & restore the database. The only reason to use something more complex than this is if you want to continue using the cluster which seems like a really bad idea. Am I missing a usecase here? I guess it can be useful as a template utility to start hacking on for more complex scenarios, but I don't think that warrants /contrib. > > - REINDEX CONCURRENTLY: > > Imo pretty close to being comittable and pretty useful, but it got > > redesigned pretty late and it mostly had review from me and fujii and > > it could use a bit more input > > => unclear > > I think this really has no hope of being bulletproof until we have > MVCC-based catalog scans. The work up to now has been good exploratory > effort, but it's not going to be committable without that > infrastructure, IMHO anyway. I think its fair not to commit it in 9.3, the patch came late and has undergone significant issues since then. And there are some things in it I am not sure about yet. But I am not sure why its only acceptable with MVCC scans? Sure, it takes an exclusive lock for a very short time when switching the relations, but thats still a huge step up over the status quo: * toast indexes currently cannot be created/dropped at all. So you have to reindex the table which will need to be locked over the whole time. * you currently cannot replace indexes that are referenced by foreign keys without doing manual catalog hackery which is definitely unsafe * you cannot create constraints that use an index concurrently. Parts of that is emulatable by creating the constraint with an existing index, but that doesn't help e.g. for exclusion constraints. * there's no convenient way to issue CREATE INDEX CONCURRENLTY new_index; DROP INDEX CONCURRENTLY old_index; RENAME new_index old_index; for all indexes. The last alone imnsho is already a good enough usecase for the patch. Also, the part that deals with the (locked) switch of the relfilenodes is a really small part of the patch, improving that later is not going to throw away much. Imo having this would have made e.g. beb850e1d873f8920a78b9b9ee27e9f87c95592f way much less painful. We (as in 2ndq) had to write scripts for customers to deal with this in a remotely sane manner. And those were *ugly*. > > - psql watch > > Waiting on author since 4 days > > => wait or boot? > > If there is agreement on the basic design of the feature, ie > "\watch [n]" as a repeating version of "\g", it seems to me that > this could be fixed and committed in a day or less. I'd be willing > to look at it sometime next week. Cool. > > - transforms: > > I am not sure what solution is proposed for the dependency issue > > between shared objects. > > There also hasn't been code level review for a while... > > => unclear > > Needs to be moved; no way this is ready in the next week or two. Good. I haven't looked at it in a long time and nobody else did, so I just wasn't sure. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, 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] Getting to 9.3 beta
On Fri, Mar 29, 2013 at 11:05:46AM -0400, Tom Lane wrote: > Bruce Momjian writes: > > Our final 9.3 commit-fest has has exceeded the two-month mark, so it is > > time to start targeting a date to close it and get to 9.3 beta. I see > > 25 items will needing attention before we can close it: > > > https://commitfest.postgresql.org/action/commitfest_view?id=17 > > > What is a reasonable timeframe to target for completion of these items? > > TBH, once Andrew commits the JSON patch, I wouldn't have a problem with > moving all the rest to Returned With Feedback or the next CF. None of > the others seem to me to be close-to-committable (with the possible > exception of the Max LSN patch, which I've not looked at), and April is > not the time to be doing development. > > Next week is going to be tied up with the back-branch releases, but > maybe we could target beta for the week after? The main gating factor > at this point really would be how quickly we could write some draft > release notes, so people know what to test. I have been clearing my calendar with plans to do the 9.3 release notes, so I am ready to start. -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + -- 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] Getting to 9.3 beta
Andres Freund writes: > On 2013-03-29 10:15:42 -0400, Bruce Momjian wrote: >> What is a reasonable timeframe to target for completion of these items? > Here's my take on it: Thanks for annotating these! I've commented on the ones where I come to a different conclusion: > - replace plugins directory with GUC > Peter has agreed to boot it to the next fest afaics > (515357f4.6000...@gmx.net) > => move (done) It looks to me more like a RWF situation, since Peter has evidently lost interest in committing this feature at all. So I marked it that way. > - Patch to compute Max LSN of Data Pages: > I *personally* don't really see a point in including it in postgres, > there doesn't really seem to be any real demand met by the tool > => reject? Not sure. It certainly hasn't drawn that much enthusiasm on the list, but it strikes me as the sort of thing that when you need it you really need it. It would be interesting to hear opinions about it from people who deal with data recovery situations on a regular basis. > - REINDEX CONCURRENTLY: > Imo pretty close to being comittable and pretty useful, but it got > redesigned pretty late and it mostly had review from me and fujii and > it could use a bit more input > => unclear I think this really has no hope of being bulletproof until we have MVCC-based catalog scans. The work up to now has been good exploratory effort, but it's not going to be committable without that infrastructure, IMHO anyway. > - psql watch > Waiting on author since 4 days > => wait or boot? If there is agreement on the basic design of the feature, ie "\watch [n]" as a repeating version of "\g", it seems to me that this could be fixed and committed in a day or less. I'd be willing to look at it sometime next week. > - transforms: > I am not sure what solution is proposed for the dependency issue > between shared objects. > There also hasn't been code level review for a while... > => unclear Needs to be moved; no way this is ready in the next week or two. 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] Getting to 9.3 beta
Magnus Hagander writes: > On Fri, Mar 29, 2013 at 4:05 PM, Tom Lane wrote: >> Next week is going to be tied up with the back-branch releases, but >> maybe we could target beta for the week after? The main gating factor >> at this point really would be how quickly we could write some draft >> release notes, so people know what to test. > I think we need to give it at least one more week before we can target > a beta. Sure, I was just suggesting a best-case scenario. Probably a more realistic thing would be to target beta wrap for April 15, which would leave an extra week or so for documentation polishing and alpha-level testing. > Of course, that doesn't prevent from starting work on the release > notes meanwhile :) Right ;-) 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] Drastic performance loss in assert-enabled build in HEAD
Kevin Grittner writes: > Tom Lane wrote: >> So maybe I'm nuts to care about the performance of an assert-enabled >> backend, but I don't really find a 4X runtime degradation acceptable, >> even for development work. Does anyone want to fess up to having caused >> this, or do I need to start tracking down what changed? > I checked master HEAD for a dump of regression and got about 4 > seconds. I checked right after my initial push of matview code and > got 2.5 seconds. I checked just before that and got 1 second. > There was some additional pg_dump work for matviews after the > initial push which may or may not account for the rest of the time. I poked at this a bit, and eventually found that the performance differential goes away if I dike out the pg_relation_is_scannable() call in getTables()'s table-collection query. What appears to be happening is that those calls cause a great many more relcache entries to be made than used to happen, plus many entries are made earlier in the run than they used to be. Many of those entries have subsidiary memory contexts, which results in an O(N^2) growth in the time spent in AllocSetCheck, since postgres.c does MemoryContextCheck(TopMemoryContext) once per received command, and pg_dump will presumably issue O(N) commands in an N-table database. So one question that brings up is whether we need to dial back the amount of work done in memory context checking, but I'm loath to do so in development builds. That code has fingered an awful lot of bugs. OTOH, if the number of tables in the regression DB continues to grow, we may have little choice. Anyway, the immediate takeaway is that this represents a horribly expensive way for pg_dump to find out which matviews aren't scannable. The cheap way for it to find out would be if we had a boolean flag for that in pg_class. Would you review the bidding as to why things were done the way they are? Because in general, having to ask the kernel something is going to suck in any case, so basing it on the size of the disk file doesn't seem to me to be basically a good thing. We could alleviate the pain by changing pg_dump's query to something like (case when c.relkind = 'm' then pg_relation_is_scannable(c.oid) else false end) but TBH this feels like bandaging a bad design. Another reason why I don't like this code is that pg_relation_is_scannable is broken by design: relid = PG_GETARG_OID(0); relation = RelationIdGetRelation(relid); result = relation->rd_isscannable; RelationClose(relation); You can't do that: if the relcache entry doesn't already exist, this will try to construct one while not holding any lock on the relation, which is subject to all sorts of race conditions. (In general, direct calls on RelationIdGetRelation are probably broken. I see you introduced more than one such in the matviews patch, and I'm willing to bet they are all unsafe.) 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] Getting to 9.3 beta
On 03/29/2013 11:05 AM, Tom Lane wrote: Bruce Momjian writes: Our final 9.3 commit-fest has has exceeded the two-month mark, so it is time to start targeting a date to close it and get to 9.3 beta. I see 25 items will needing attention before we can close it: https://commitfest.postgresql.org/action/commitfest_view?id=17 What is a reasonable timeframe to target for completion of these items? TBH, once Andrew commits the JSON patch, I wouldn't have a problem with moving all the rest to Returned With Feedback or the next CF. I can do that pretty much right away. cheers andrew -- 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] Getting to 9.3 beta
On 2013-03-29 10:15:42 -0400, Bruce Momjian wrote:
> Our final 9.3 commit-fest has has exceeded the two-month mark, so it is
> time to start targeting a date to close it and get to 9.3 beta. I see
> 25 items will needing attention before we can close it:
Very much agreed!
> https://commitfest.postgresql.org/action/commitfest_view?id=17
>
> What is a reasonable timeframe to target for completion of these items?
Here's my take on it:
- Extension templates:
Not ready yet, probably takes a bit more work till committable, last
version got in rather late.
=> move to next fest
- Performance Improvement by reducing WAL for Update Operation:
Not ready yet, some redesigns are recently made, benefits aren't all
that great
=> move to next fest
- Index based regexp search for pg_trgm:
Seems like the patch is undergoing restructuring of the regex access API
=> move to next fest
- Truncate trailing nulls from heap rows to reduce the size of the null
bitmap:
Not sure, benefits don't seem to be all that big for realistic
usecases?
=> move or reject?
- allowing privileges on untrusted languages:
Direction forward seems unclear, discussion required
=> move
- replace plugins directory with GUC
Peter has agreed to boot it to the next fest afaics
(515357f4.6000...@gmx.net)
=> move (done)
- sepgsql: name qualified default security label
has been committed by robert, updated CF
- sepgsql: db_schema:search permission and sepgsql: db_procedure:execute
permission:
Not much review afaics.
=> looks unfair, but unless some comitter (robert?) takes it
on I don't see much alternative to booting it to the next fest?
- Patch to compute Max LSN of Data Pages:
I *personally* don't really see a point in including it in postgres,
there doesn't really seem to be any real demand met by the tool
=> reject?
- Make recovery.conf parameters into GUCs:
Nice to see progress, but this seems to be too late for 9.3.
=> move to -next
- pg_retainxlog for contrib:
5102f7c9.6050...@gmx.net sums up my opinion of the utility, just
doesn't seem to be reliable enough to be distributed with postgres.
=> reject/redesign?
- REINDEX CONCURRENTLY:
Imo pretty close to being comittable and pretty useful, but it got
redesigned pretty late and it mostly had review from me and fujii and
it could use a bit more input
=> unclear
- built-in/SQL Command to edit postgresql.conf ("SET PERSISTENT" patch):
Different patches afloat, none is really ready. I personally think
Zoltan's patch is more realistic, but ...
=> move
- pg_ctl idempotent option:
looks ready to me
=> commit
- New statistics for WAL buffer dirty writes:
waiting on other for months
=> returned with feedback
- pg_stat_statements: query, session, and eviction identification:
Seems to need at least docs
=> wait for author, seems to be easy enough?
- Json API and extraction functions:
Seems to be committable
=> commit
- psql watch
Waiting on author since 4 days
=> wait or boot?
- User control over psql's error stream
Unclear, there doesn't seem to be much progress and review
=> return with feedback
- plpgsql_check_function:
Tom says (27661.1364267...@sss.pgh.pa.us) that even if the approach
can be aggreed uppon it needs quite a bit more work
=> move
- transforms:
I am not sure what solution is proposed for the dependency issue
between shared objects.
There also hasn't been code level review for a while...
=> unclear
- Check file parameters to psql before prompt for password:
No answer to Stephen's review in december
=> return with feedback
- pkg-config for libpq and ecpg:
Issues seem to be fixable easy enough
=> wait
Greetings,
Andres Freund
--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, 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] Getting to 9.3 beta
On Fri, Mar 29, 2013 at 4:05 PM, Tom Lane wrote: > Bruce Momjian writes: >> Our final 9.3 commit-fest has has exceeded the two-month mark, so it is >> time to start targeting a date to close it and get to 9.3 beta. I see >> 25 items will needing attention before we can close it: > >> https://commitfest.postgresql.org/action/commitfest_view?id=17 > >> What is a reasonable timeframe to target for completion of these items? > > TBH, once Andrew commits the JSON patch, I wouldn't have a problem with > moving all the rest to Returned With Feedback or the next CF. None of > the others seem to me to be close-to-committable (with the possible > exception of the Max LSN patch, which I've not looked at), and April is > not the time to be doing development. I haven't looked through too many of them. as for the one I have on there myself, the pg_retainxlog one, i think the consensus was to turn it into a documentation patch - and we can keep adding doc patches into beta. But, to the point. It might be a decent starting point to at least move all patches that are "waiting on author" and doesn't get almost daily status updates (or has a very clear statement from the author on when an update can be expected). That'll cut the list down quite a bit I think, and should make it easier to push harder on the others perhaps. > Next week is going to be tied up with the back-branch releases, but > maybe we could target beta for the week after? The main gating factor > at this point really would be how quickly we could write some draft > release notes, so people know what to test. I think we need to give it at least one more week before we can target a beta. Packagers will if anything have *more* work to deal with these backbranch releases than usual, and asking them to push another one just one (or even two) weeks after that would be rushing it unnecessarily... Of course, that doesn't prevent from starting work on the release notes meanwhile :) -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/ -- 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] Getting to 9.3 beta
Bruce Momjian writes: > Our final 9.3 commit-fest has has exceeded the two-month mark, so it is > time to start targeting a date to close it and get to 9.3 beta. I see > 25 items will needing attention before we can close it: > https://commitfest.postgresql.org/action/commitfest_view?id=17 > What is a reasonable timeframe to target for completion of these items? TBH, once Andrew commits the JSON patch, I wouldn't have a problem with moving all the rest to Returned With Feedback or the next CF. None of the others seem to me to be close-to-committable (with the possible exception of the Max LSN patch, which I've not looked at), and April is not the time to be doing development. Next week is going to be tied up with the back-branch releases, but maybe we could target beta for the week after? The main gating factor at this point really would be how quickly we could write some draft release notes, so people know what to test. 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] Changing recovery.conf parameters into GUCs
On 29 March 2013 01:17, Michael Paquier wrote: > I highly recommend that > you use one of the latest updated version I sent. Fujii's version had some > bugs, one of them being that as standbyModeRequested can be set to true if > specified in postgresql.conf, a portion of the code using in xlog.c can be > triggered even if ArchiveRecoveryRequested is not set to true. I also added > fixes related to documentation. Yes, that is what I meant by "Fujii's patch". He would still be credited first, having done the main part of the work. I'll call it Michael's updated version to avoid any further confusion. -- Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, 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
[HACKERS] Getting to 9.3 beta
Our final 9.3 commit-fest has has exceeded the two-month mark, so it is time to start targeting a date to close it and get to 9.3 beta. I see 25 items will needing attention before we can close it: https://commitfest.postgresql.org/action/commitfest_view?id=17 What is a reasonable timeframe to target for completion of these items? -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + -- 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] Changing recovery.conf parameters into GUCs
On Fri, Mar 29, 2013 at 01:56:50PM +, Simon Riggs wrote: > On 29 March 2013 13:24, Michael Paquier wrote: > > On Fri, Mar 29, 2013 at 9:59 PM, Simon Riggs wrote: > >> > >> On 29 March 2013 01:17, Michael Paquier wrote: > >> > On Fri, Mar 29, 2013 at 12:48 AM, Simon Riggs > >> > wrote: > >> Early discussions had difficulties because of the lack of config > >> directories, include_if_exists and this patch. We now have the > >> technical capability to meet every request. Circumstances have changed > >> and outcomes may change also. > > > > Thanks for the clarifications. The following questions are still unanswered: > > Minor points only. We can implement this differently if you have > alternate proposals. Seems we are doing design on this long after the final 9.3 commit fest should have closed. I think we need to punt this to 9.4. -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + -- 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] Changing recovery.conf parameters into GUCs
On 29 March 2013 13:24, Michael Paquier wrote: > On Fri, Mar 29, 2013 at 9:59 PM, Simon Riggs wrote: >> >> On 29 March 2013 01:17, Michael Paquier wrote: >> > On Fri, Mar 29, 2013 at 12:48 AM, Simon Riggs >> > wrote: >> Early discussions had difficulties because of the lack of config >> directories, include_if_exists and this patch. We now have the >> technical capability to meet every request. Circumstances have changed >> and outcomes may change also. > > Thanks for the clarifications. The following questions are still unanswered: Minor points only. We can implement this differently if you have alternate proposals. > 1) If recovery.trigger and recovery.conf are specified. To which one the > priority is given? Neither. No priority is required. If either is present we are triggered. > 2) If both recovery.trigger and recovery.conf are used, let's imagine that > the server removes recovery.trigger but fails in renaming recovery.conf but > a reason or another. Isn't there a risk of inconsistency if both triggering > methods are used at the same time? No. If writes to the filesystem fail, you have much bigger problems. > 3) Forcing a harcode of include_is_exists = 'recovery.conf' at the bottom of > postgresql.conf doesn't look like a hack? Well, that's just an emotive term to describe something you don't like. There are no significant downsides, just a few lines of code, like we have in many places for various purposes, such as the support of multiple APIs for triggering standbys. > 4) Based on your proposal, are all the parameters included in > postgresql.conf.sample or not? Or only primary_conninfo, trigger_file and > standby_mode? Other values are specific to particular situations (e.g. PITR) and if set in the wrong context could easily break replication. We could add them if people wish it, but it would be commented out with a clear "don't touch these" message, so it seems more sensible to avoid them IMHO. -- Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, 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] Changing recovery.conf parameters into GUCs
On Fri, Mar 29, 2013 at 9:59 PM, Simon Riggs wrote: > On 29 March 2013 01:17, Michael Paquier wrote: > > On Fri, Mar 29, 2013 at 12:48 AM, Simon Riggs > wrote: > Early discussions had difficulties because of the lack of config > directories, include_if_exists and this patch. We now have the > technical capability to meet every request. Circumstances have changed > and outcomes may change also. > Thanks for the clarifications. The following questions are still unanswered: 1) If recovery.trigger and recovery.conf are specified. To which one the priority is given? 2) If both recovery.trigger and recovery.conf are used, let's imagine that the server removes recovery.trigger but fails in renaming recovery.conf but a reason or another. Isn't there a risk of inconsistency if both triggering methods are used at the same time? 3) Forcing a harcode of include_is_exists = 'recovery.conf' at the bottom of postgresql.conf doesn't look like a hack? 4) Based on your proposal, are all the parameters included in postgresql.conf.sample or not? Or only primary_conninfo, trigger_file and standby_mode? -- Michael
Re: [HACKERS] Changing recovery.conf parameters into GUCs
On 29 March 2013 01:17, Michael Paquier wrote: > The main argument on which this proposal is based on is to keep > backward-compatibility. The main objective is to get recovery parameters as GUCs, as I said > On Fri, Mar 29, 2013 at 12:48 AM, Simon Riggs wrote: >> What we want to do is make recovery parameters into GUCs, allowing >> them to be reset by SIGHUP and also to allow all users to see the >> parameters in use, from any session. >From the user's perspective, we are making no changes. All recovery parameters will work exactly the same as they always did, just now we get to see their values more easily and we can potentially place them in a different file if we wish. The user will have no idea that we plan to do some internal refactoring of how we process the parameters. So IMHO simplicity means continuing to work the way it always did work. We simply announce "PostgreSQL now supports configuration directories. All parameters, including recovery parameters, can be placed in any configuration file, or in $PGDATA/recovery.conf, as before". We introduced "pg_ctl promote" with a new API without removing existing ones, and some people are still in favour of keeping both APIs. Doing the same thing here makes sense and reduces conceptual change. Early discussions had difficulties because of the lack of config directories, include_if_exists and this patch. We now have the technical capability to meet every request. Circumstances have changed and outcomes may change also. -- Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, 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] in-catalog Extension Scripts and Control parameters (templates?)
Hi, Thanks you for testing and reporting those strange bugs, I should be able to fix them by Tuesday at the earliest. Heikki Linnakangas writes: > create template for extension sslinfo version '1.0' with (schema public) as > $$ DO EVIL STUFF $$; What you're saying is that we should restrict the capability to superusers only, where I didn't think about those security implications before and though that it wouldn't be a necessary limitation. I will add the usual superuser() checks in the next version of the patch. > Documentation doesn't build, multiple errors. In addition to the reference > pages, there should be a section in the main docs about these templates. I really would like for a simpler setup to build documentation on my OS of choice, and realize that's no excuse. Will clean that up in next version of the patch. >> postgres=# create template for extension myextension version '1.0' with () >> as 'foobar'; >> CREATE TEMPLATE FOR EXTENSION >> postgres=# create extension myextension; >> ERROR: syntax error at or near "foobar" >> LINE 1: create extension myextension; >> ^ > > Confusing error message. Do we need to compute a different error message when applying the script from a template or from a file on-disk? Also please keep in mind that those error messages are typically to be seen by the extension's author. >> postgres=# create template for extension myextension version '1.0' with () >> as $$create table foobar(i int4) $$; >> CREATE TEMPLATE FOR EXTENSION >> postgres=# create extension myextension; >> CREATE EXTENSION >> postgres=# select * from foobar; >> ERROR: relation "foobar" does not exist >> LINE 1: select * from foobar; >> ^ > > Where did that table go? Well that's not the answer I wanted to make, but: select c.oid, relname, nspname from pg_class c join pg_namespace n on n.oid = c.relnamespace where relname ~ 'foobar'; oid | relname | nspname ---+-+- 41412 | foobar | 1.0 (1 row) > Ah, here... Where did that "1.0" schema come from? I need to sort that out. Didn't have that problem in my tests (included in the regression tests), will add your test case and see about fixing that bug in the next version of the patch. >> postgres=> create template for extension myextension version '1.0' with >> (schema public) as $$ create function evilfunc() returns int4 AS >> evilfunc' language internal; $$; >> CREATE TEMPLATE FOR EXTENSION >> postgres=> create extension myextension version '1.0';ERROR: permission >> denied for language internal >> postgres=> drop template for extension myextension version '1.0'; >> ERROR: extension with OID 16440 does not exist > > Something wrong with catalog caching. Or something wrong with dependencies maybe… will have a look at that too, and add some regression tests. >> $ make -s install >> /usr/bin/install: cannot stat `./hstore--1.0.sql': No such file or directory >> make: *** [install] Error 1 > > Installing hstore fails. Works for me. Anyway that part was to show up how we could have been managing the hstore 1.1 update in the past, I don't intend for it to get commited unless specifically asked to do so. I guess I should now remove hstore changes from the patch now, and will do so in the next version of the patch. > If we check for an existing extension at CREATE, should also check for that > in ALTER ... RENAME TO. Indeed. Will fix that too. > Also: > pg_dump does not dump the owner of an extension template correctly. Will look into that too. Thanks for your reviewing and testing, sorry to have missed those bugs. The new version of the patch, early next week, will include fixes for all of those and some more testing. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- 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] sql_drop Event Triggerg
Alvaro Herrera writes: > Pushed, with some further minor changes. One not-so-minor change I Thanks a lot for all the work you did put into this patch! > introduced was that pg_event_trigger_dropped_objects() now only works > within a sql_drop event function. The reason I decided to do this was > that if we don't have that protection, then it is possible to have a > ddl_command_end trigger calling pg_event_trigger_dropped_objects(); and > if there is an sql_drop trigger, then it'd return the list of dropped > objects, but if there's no sql_drop trigger, it'd raise an error. That > seemed surprising enough action-at-a-distance that some protection is > warranted. +1 I hope that we can add another such function for ddl_command_start and ddl_command_end function to get at some object information from there, now that the support code is in place. That would allow making any use of the event trigger facility in 9.3 without having to write a C coded extension… which has been the goal for the last two cycles… Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- 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] Enabling Checksums
On 2013-03-28 21:02:06 -0400, Robert Haas wrote: > On Wed, Mar 27, 2013 at 10:15 AM, Andres Freund > wrote: > > On 2013-03-27 10:06:19 -0400, Robert Haas wrote: > >> On Mon, Mar 18, 2013 at 4:31 PM, Greg Smith wrote: > >> > to get them going again. If the install had checksums, I could have > >> > figured > >> > out which blocks were damaged and manually fixed them, basically go on a > >> > hunt for torn pages and the last known good copy via full-page write. > >> > >> Wow. How would you extract such a block image from WAL? > >> > >> That would be a great tool to have, but I didn't know there was any > >> practical way of doing it today. > > > > Given pg_xlogdump that should be doable with 5min of hacking in 9.3. Just > > add > > some hunk to write out the page to the if (config->bkp_details) hunk in > > pg_xlogdump.c:XLogDumpDisplayRecord. I have done that for some debugging > > already. > > > > If somebody comes up with a sensible & simple UI for this I am willing to > > propose a patch adding it to pg_xlogdump. One would have to specify the > > rel/file/node, the offset, and the target file. > > Hmm. Cool. But, wouldn't the hard part be to figure out where to > start reading the WAL in search of the *latest* FPI? I'd expect having to read the whole WAL and write out all the available FPIs. You might be able to a guess a bit based on the LSN in the header. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, 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
