Re: [HACKERS] Visual Studio 2010/Windows SDK 7.1 support
On Fri, 13 May 2011 21:52:47 -0400, Robert Haas wrote: You probably want to add it here, then: https://commitfest.postgresql.org/action/commitfest_view/open I's been in the last commitfest and i've recently moved it to the current one already. See https://commitfest.postgresql.org/action/patch_view?id=523 Regards, Brar -- 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] Reducing overhead of frequent table locks
On Fri, May 13, 2011 at 08:55:34PM -0400, Robert Haas wrote: > On Fri, May 13, 2011 at 4:16 PM, Noah Misch wrote: > > If I'm understanding correctly, your pseudocode would look roughly like > > this: > > > > ? ? ? ?if (level >= ShareUpdateExclusiveLock) > I think ShareUpdateExclusiveLock should be treated as neither weak nor > strong. Indeed; that should be ShareLock. > It certainly can't be treated as weak - i.e. use the fast > path - because it's self-conflicting. It could be treated as strong, > but since it doesn't conflict with any of the weak lock types, that > would only serve to prevent fast-path lock acquisitions that otherwise > could have succeeded. In particular, it would unnecessarily disable > fast-path lock acquisition for any relation being vacuumed, which > could be really ugly considering that one of the main workloads that > would benefit from something like this is the case where lots of > backends are fighting over a lock manager partition lock on a table > they all want to run read and/or modify. I think it's best for > ShareUpdateExclusiveLock to always use the regular lock-acquisition > path, but it need not worry about incrementing strong_lock_counts[] or > importing local locks in so doing. Agreed. > Also, I think in the step just after marker one, we'd only import only > local locks whose lock tags were equal to the lock tag on which we > were attempting to acquire a strong lock. The downside of this whole > approach is that acquiring a strong lock becomes, at least > potentially, a lot slower, because you have to scan through the whole > backend array looking for fast-path locks to import (let's not use the > term "local lock", which is already in use within the lock manager > code). But maybe that can be optimized enough not to matter. After > all, if the lock manager scaled perfectly at high concurrency, we > wouldn't be thinking about this in the first place. Incidentally, I used the term "local lock" because I assumed fast-path locks would still go through the lock manager far enough to populate the local lock table. But there may be no reason to do so. > > I wonder if, instead, we could signal all backends at > > marker 1 to dump the applicable parts of their local (memory) lock tables to > > files. ?Or to another shared memory region, if that didn't mean statically > > allocating the largest possible required amount. ?If we were willing to wait > > until all backends reach a CHECK_FOR_INTERRUPTS, they could instead make the > > global insertions directly. ?That might yield a decent amount of bug > > swatting to > > fill in missing CHECK_FOR_INTERRUPTS, though. > > I've thought about this; I believe it's unworkable. If one backend > goes into the tank (think: SIGSTOP, or blocking on I/O to an > unreadable disk sector) this could lead to cascading failure. True. It would need some fairly major advantages to justify that risk, and I don't see any. Overall, looks like a promising design sketch to me. Thanks. nm -- 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] plpgsql doesn't supply typmod for the Params it generates
On Thu, May 12, 2011 at 5:55 PM, Tom Lane wrote: > I wrote: >> I think the appropriate fix is pretty clear: add a function similar to >> exec_get_datum_type that returns the datum's typmod, and use that to set >> paramtypmod properly. What is worrying me is that it's not clear how >> much user-visible behavioral change will result, and therefore I'm not >> entirely comfortable with the idea of back-patching such a change into >> 9.0 --- and it wouldn't work at all in 8.4, where there simply isn't a >> good opportunity to set a typmod for parameters passed to the main >> executor (since the SPI interfaces plpgsql uses don't support that). > > Attached is a proposed patch for HEAD that sets up the Param's typmod > sanely. I've verified that this fixes the reported problem and does not > result in any changes in the regression tests, which makes me a bit more > optimistic about it ... but I'm still not convinced it'd be a good idea > to back-patch into 9.0. Back-patching doesn't seem very safe to me, either; though I'm not entirely certain what to do instead. Relaxing the check, as you proposed upthread, might be the way to go. -- 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] Visual Studio 2010/Windows SDK 7.1 support
On Fri, May 13, 2011 at 5:34 PM, Brar Piening wrote: > After some months of being able to regularly compile current head using > Visual Studio 2010 compilers and some more tests I no longer see any reason > why this patch would change without external feedback. You probably want to add it here, then: https://commitfest.postgresql.org/action/commitfest_view/open -- 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] the big picture for index-only scans
On Fri, May 13, 2011 at 6:34 PM, Cédric Villemain wrote: > Will you be able to do some ? or can you propose a simple process to > do efficient benchmark of the patch ? I will probably do some benchmarking at some point, unless someone else goes nuts and makes it moot before I get to that point. I think the main thing is to just apply the patch and beat up the server, and see if it's any slower than it was before. > If reviewers agree it is safe and benchmarks make evidences that no > basic performance issue are raised, then let's see if next items have > blockers or can be done. Sounds right to me. -- 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] Reducing overhead of frequent table locks
On Fri, May 13, 2011 at 4:16 PM, Noah Misch wrote: > The key is putting a rapid hard stop to all fast-path lock acquisitions and > then reconstructing a valid global picture of the affected lock table regions. > Your 1024-way table of strong lock counts sounds promising. (Offhand, I do > think they would need to be counts, not just flags.) > > If I'm understanding correctly, your pseudocode would look roughly like this: > > if (level >= ShareUpdateExclusiveLock) > ++strong_lock_counts[my_strong_lock_count_partition] > sfence > if (strong_lock_counts[my_strong_lock_count_partition] == 1) > /* marker 1 */ > import_all_local_locks > normal_LockAcquireEx > else if (level <= RowExclusiveLock) > lfence > if (strong_lock_counts[my_strong_lock_count_partition] == 0) > /* marker 2 */ > local_only > /* marker 3 */ > else > normal_LockAcquireEx > else > normal_LockAcquireEx I think ShareUpdateExclusiveLock should be treated as neither weak nor strong. It certainly can't be treated as weak - i.e. use the fast path - because it's self-conflicting. It could be treated as strong, but since it doesn't conflict with any of the weak lock types, that would only serve to prevent fast-path lock acquisitions that otherwise could have succeeded. In particular, it would unnecessarily disable fast-path lock acquisition for any relation being vacuumed, which could be really ugly considering that one of the main workloads that would benefit from something like this is the case where lots of backends are fighting over a lock manager partition lock on a table they all want to run read and/or modify. I think it's best for ShareUpdateExclusiveLock to always use the regular lock-acquisition path, but it need not worry about incrementing strong_lock_counts[] or importing local locks in so doing. Also, I think in the step just after marker one, we'd only import only local locks whose lock tags were equal to the lock tag on which we were attempting to acquire a strong lock. The downside of this whole approach is that acquiring a strong lock becomes, at least potentially, a lot slower, because you have to scan through the whole backend array looking for fast-path locks to import (let's not use the term "local lock", which is already in use within the lock manager code). But maybe that can be optimized enough not to matter. After all, if the lock manager scaled perfectly at high concurrency, we wouldn't be thinking about this in the first place. > At marker 1, we need to block until no code is running between markers two and > three. You could do that with a per-backend lock (LW_SHARED by the strong > locker, LW_EXCLUSIVE by the backend). That would probably still be a win over > the current situation, but it would be nice to have something even cheaper. I don't have a better idea than to use an LWLock. I have a patch floating around to speed up our LWLock implementation, but I haven't got a workload where the bottleneck is the actual speed of operation of the LWLock rather than the fact that it's contended in the first place. And the whole point of this would be to arrange things so that the LWLocks are uncontended nearly all the time. > Then you have the actual procedure for transfer of local locks to the global > lock manager. Using record locks in a file could work, but that's a system > call > per lock acquisition regardless of the presence of strong locks. Is that cost > sufficiently trivial? No, I don't think we want to go into kernel space. When I spoke of using a file, I was imagining it as an mmap'd region that one backend could write to which, at need, another backend could mmap and grovel through. Another (perhaps simpler) option would be to just put it in shared memory. That doesn't give you as much flexibility in terms of expanding the segment, but it would be reasonable to allow space for only, dunno, say 32 locks per backend in shared memory; if you need more than that, you flush them all to the main lock table and start over. You could possibly even just make this a hack for the particular special case where we're taking a relation lock on a non-shared relation; then you'd need only 128 bytes for a 32-entry array, plus the LWLock (I think the database ID is already visible in shared memory). > I wonder if, instead, we could signal all backends at > marker 1 to dump the applicable parts of their local (memory) lock tables to > files. Or to another shared memory region, if that didn't mean statically > allocating the largest possible required amount. If we were willing to wait > until all backends reach a CHECK_FOR_INTERRUPTS, they could instead make the > global insertions directly. That might yield a decent amount of bug s
Re: [HACKERS] pg_upgrade and PGPORT
Bruce Momjian wrote: > > >> ? ? ? Performing Consistency Checks > > >> ? ? ? - > > >> ? ? ? ignoring libpq environment variable PGPORT > > > > > > I haven't tried it, but I suppose option.c will now make use of PGPORT > > > and then later you get that message that it was ignored? > > > > Either way, it hardly seems necessary to emit a log message stating > > that you are unsetting an environment variable. > > I think the whole idea of worrying about libpq environment variables is > useless. I looked at the list of libpq environment variables and I saw > a lot of useful ones, like PGUSER and PGPASSFILE, which we currently > throw an error. > > I propose we only disable the use of PGHOST and even then that prevents > users from controlling tcp vs. unix domain connections. OK, it turns out the environment variable handling in pg_upgrade was worse than I thought. This patch: o disables only PGHOST and only if it is set to a non-local value; all other environment variables are honored; PGDATA isn't even seen by libpq o push --user value into the PGUSER environment variable so pg_ctl -w uses it; pg_ctl has no --user flag; this is important for pre-9.1 pg_ctl binaries o move putenv() function to utils.c now that it is used by option.c o allow pg_ctl failure to continue with a connection request to get a possible error message, then exit o update document to be clearer and mention environment variables Patch attached. -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + diff --git a/contrib/pg_upgrade/controldata.c b/contrib/pg_upgrade/controldata.c new file mode 100644 index 3ac2180..5ce4b95 *** a/contrib/pg_upgrade/controldata.c --- b/contrib/pg_upgrade/controldata.c *** *** 11,18 #include - static void putenv2(const char *var, const char *val); - /* * get_control_data() * --- 11,16 *** get_control_data(ClusterInfo *cluster, b *** 85,105 if (getenv("LC_MESSAGES")) lc_messages = pg_strdup(getenv("LC_MESSAGES")); ! putenv2("LC_COLLATE", NULL); ! putenv2("LC_CTYPE", NULL); ! putenv2("LC_MONETARY", NULL); ! putenv2("LC_NUMERIC", NULL); ! putenv2("LC_TIME", NULL); ! putenv2("LANG", #ifndef WIN32 NULL); #else /* On Windows the default locale cannot be English, so force it */ "en"); #endif ! putenv2("LANGUAGE", NULL); ! putenv2("LC_ALL", NULL); ! putenv2("LC_MESSAGES", "C"); snprintf(cmd, sizeof(cmd), SYSTEMQUOTE "\"%s/%s \"%s\"" SYSTEMQUOTE, cluster->bindir, --- 83,103 if (getenv("LC_MESSAGES")) lc_messages = pg_strdup(getenv("LC_MESSAGES")); ! pg_putenv("LC_COLLATE", NULL); ! pg_putenv("LC_CTYPE", NULL); ! pg_putenv("LC_MONETARY", NULL); ! pg_putenv("LC_NUMERIC", NULL); ! pg_putenv("LC_TIME", NULL); ! pg_putenv("LANG", #ifndef WIN32 NULL); #else /* On Windows the default locale cannot be English, so force it */ "en"); #endif ! pg_putenv("LANGUAGE", NULL); ! pg_putenv("LC_ALL", NULL); ! pg_putenv("LC_MESSAGES", "C"); snprintf(cmd, sizeof(cmd), SYSTEMQUOTE "\"%s/%s \"%s\"" SYSTEMQUOTE, cluster->bindir, *** get_control_data(ClusterInfo *cluster, b *** 374,388 /* * Restore environment variables */ ! putenv2("LC_COLLATE", lc_collate); ! putenv2("LC_CTYPE", lc_ctype); ! putenv2("LC_MONETARY", lc_monetary); ! putenv2("LC_NUMERIC", lc_numeric); ! putenv2("LC_TIME", lc_time); ! putenv2("LANG", lang); ! putenv2("LANGUAGE", language); ! putenv2("LC_ALL", lc_all); ! putenv2("LC_MESSAGES", lc_messages); pg_free(lc_collate); pg_free(lc_ctype); --- 372,386 /* * Restore environment variables */ ! pg_putenv("LC_COLLATE", lc_collate); ! pg_putenv("LC_CTYPE", lc_ctype); ! pg_putenv("LC_MONETARY", lc_monetary); ! pg_putenv("LC_NUMERIC", lc_numeric); ! pg_putenv("LC_TIME", lc_time); ! pg_putenv("LANG", lang); ! pg_putenv("LANGUAGE", language); ! pg_putenv("LC_ALL", lc_all); ! pg_putenv("LC_MESSAGES", lc_messages); pg_free(lc_collate); pg_free(lc_ctype); *** rename_old_pg_control(void) *** 529,568 pg_log(PG_FATAL, "Unable to rename %s to %s.\n", old_path, new_path); check_ok(); } - - - /* - * putenv2() - * - * This is like putenv(), but takes two arguments. - * It also does unsetenv() if val is NULL. - */ - static void - putenv2(const char *var, const char *val) - { - if (val) - { - #ifndef WIN32 - char *envstr = (char *) pg_malloc(strlen(var) + - strlen(val) + 2); - - sprintf(envstr, "%s=%s", var, val); - putenv(envstr); - - /* - * Do not free envstr because it becomes part of the environment on - * some operating systems. See port/unsetenv.c::unsetenv. - */ - #else - SetEnvironmentVariableA(var, val); - #endif - } - else - { - #ifndef WIN32 - unsetenv(var);
Re: [HACKERS] Why not install pgstattuple by default?
On Thu, 2011-05-12 at 19:37 -0400, Tom Lane wrote: > > > It should be okay to move, since the -devel subpackage requires the > main one. Therefore there is no configuration in which pg_config > would be present before and missing after the change. Thanks Tom. I can make this change in next build set. Regards, -- Devrim GÜNDÜZ Principal Systems Engineer @ EnterpriseDB: http://www.enterprisedb.com PostgreSQL Danışmanı/Consultant, Red Hat Certified Engineer Community: devrim~PostgreSQL.org, devrim.gunduz~linux.org.tr http://www.gunduz.org Twitter: http://twitter.com/devrimgunduz signature.asc Description: This is a digitally signed message part
Re: [HACKERS] 'tuple concurrently updated' error for alter role ... set
On May 13, 2011, at 2:07 AM, Alexey Klyukin wrote: > On May 13, 2011, at 1:28 AM, Tom Lane wrote: > >> >> We're not likely to do that, first because it's randomly different from >> the handling of every other system catalog update, and second because it >> would serialize all updates on this catalog, and probably create >> deadlock cases that don't exist now. (BTW, as the patch is given I'd >> expect it to still fail, though perhaps with lower probability than >> before. For this to actually stop all such cases, you'd have to hold >> the lock till commit, which greatly increases the risks of deadlock.) > >> >> I see no particular reason why conflicting updates like those *shouldn't* >> be expected to fail occasionally. > > Excellent question, I don't have enough context to properly answer that (other > than a guess that an unexpected transaction rollback is too unexpected :)) > Let me ask the customer first. The original use case is sporadical failures of some internal unit tests due to the error message in subject. -- Alexey Klyukin The PostgreSQL Company - Command Prompt, Inc. -- 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] Double ocurring Subplan
On Fri, May 13, 2011 at 6:02 PM, Tom Lane wrote: > Gurjeet Singh writes: > > I am listing the query, it's explain output and explain analyze output at > > the end. > > > The EXPLAIN output shows the 'Subplan 2' node only once, whereas EXPLAIN > > ANALYZE output shows that the 'Subplan 2' is being executed twice . Is > that > > true? Or is it just the plan printer getting confused? Is the confusion > > because of the 2 conditions in the WHERE clause of the correlated > subquery? > > The reason it looks like that is that the SubPlan is referenced in the > index condition, and there are actually two copies of that (indxqual and > indxqualorig). They both point at the same physical subplan, but there > are two entries in the parent node's subPlan list. In EXPLAIN you only > see one because ExecInitIndexScan skips initializing the indxquals in > EXPLAIN_ONLY mode. > > In short: it's cosmetic. > That's a relief > > We could probably suppress the duplicate printout when both references > are in the same plan node, but in bitmap scans the indxqual and > indxqualorig expressions are actually in different plan nodes (the > indexscan and heapscan respectively). I'm not sure how we could > suppress the duplicate printout in that case, or whether it would even > make sense to do so --- after all, the subplan can in fact get called by > both nodes. > As long as it's not being re-evaluated, it's not a big deal. It does confuse the casual onlooker, though; and if there's any automatic tool to parse and report explain analyze output, it might get its numbers quite wrong. Regards, -- Gurjeet Singh EnterpriseDB Corporation The Enterprise PostgreSQL Company
Re: [HACKERS] proposal: a validator for configuration files
Hi, On Apr 14, 2011, at 9:50 PM, Robert Haas wrote: > On Mon, Apr 4, 2011 at 2:03 PM, Alexey Klyukin > wrote: >> Here's the update of Selena's patch, which also shows all errors in >> configuration parameters (as well as parser errors) during reload. > > You should add this here: > > https://commitfest.postgresql.org/action/commitfest_view/open > > On a quick glance, this patch appears to contain some superfluous > hunks where you changed whitespace or variable names. You might want > to remove those and repost before adding to the CF app. Also, some > submission notes would be very helpful - when you send in the revised > version, detail in the email the exact purpose of the changes so that > someone can review the patch without having to read this thread and > all preceding threads in their entirety. Thank you for the feedback, I've updated the patch, attached is a new version. I'll add it to the commitfest after posting this message. The patch forces the parser to report all errors (max 100) from the ProcessConfigFile/ParseConfigFp. Currently, only the first parse error or an invalid directive is reported. Reporting all of them is crucial to automatic validation of postgres config files. This patch is based on the one submitted earlier by Selena Deckelmann: http://archives.postgresql.org/pgsql-hackers/2009-03/msg00345.php It incorporates suggestions by Tom Lane for avoiding excessive bloat in logs in case there is a junk instead of postgresql.conf. http://archives.postgresql.org/pgsql-hackers/2009-03/msg01142.php Regards, -- Alexey Klyukin The PostgreSQL Company - Command Prompt, Inc. parser_continue_on_errors.diff Description: Binary data -- 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] the big picture for index-only scans
2011/5/11 Robert Haas : > On Wed, May 11, 2011 at 3:17 AM, Simon Riggs wrote: >> Completely agree, but why are you saying that to me? >> >> When Tom asks me why I suggest something, nobody tells him "its a free >> software project etc...". >> >> What is the difference here? > > We're now 40 emails in this thread, and there seems to be far more > heat than light here. Here's an attempt at a summary: > > - Simon wants proof that the performance benefit of this feature is > worth any downsides it may have, which is standard procedure, and > isn't certain the feature will have a significant performance benefit. > - Numerous other people think Simon's doubts about the feature are > poorly justified (and some of them also think he's being a pain in the > neck). > - Various peripherally related topics, such as optimizing count(*), > which is not part of the vision for the first go-round that I sketched > out in my OP, and plan stability, which is another issue entirely, > have been discussed. > - Meanwhile, only one person has done any review of the actual code > that's been written, which is posted on the crash-safe visibility map > thread, which may be why multiple people seem confused about what it > does. > - And no one has done any benchmarking of that code. Will you be able to do some ? or can you propose a simple process to do efficient benchmark of the patch ? If reviewers agree it is safe and benchmarks make evidences that no basic performance issue are raised, then let's see if next items have blockers or can be done. > > I think it would be really helpful if some more people would review > the crash-safe visibility map patch, and if at least one person could > benchmark it. It would be useful to know (a) whether that noticeably > slows down the system during inserts, updates, and deletes, especially > at very high concurrency; and (b) how much of an impact the additional > WAL-logging has on VACUUM. On the other hand, arguing about whether > index-only scans are going to result in a significant performance > benefit is not useful. I am going to be both surprised and > disappointed if they don't, but there's only one way to find out, and > a theoretical argument isn't it. > > -- > 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 > -- Cédric Villemain 2ndQuadrant 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] Double ocurring Subplan
Gurjeet Singh writes: > I am listing the query, it's explain output and explain analyze output at > the end. > The EXPLAIN output shows the 'Subplan 2' node only once, whereas EXPLAIN > ANALYZE output shows that the 'Subplan 2' is being executed twice . Is that > true? Or is it just the plan printer getting confused? Is the confusion > because of the 2 conditions in the WHERE clause of the correlated subquery? The reason it looks like that is that the SubPlan is referenced in the index condition, and there are actually two copies of that (indxqual and indxqualorig). They both point at the same physical subplan, but there are two entries in the parent node's subPlan list. In EXPLAIN you only see one because ExecInitIndexScan skips initializing the indxquals in EXPLAIN_ONLY mode. In short: it's cosmetic. We could probably suppress the duplicate printout when both references are in the same plan node, but in bitmap scans the indxqual and indxqualorig expressions are actually in different plan nodes (the indexscan and heapscan respectively). I'm not sure how we could suppress the duplicate printout in that case, or whether it would even make sense to do so --- after all, the subplan can in fact get called by both nodes. 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] Formatting Curmudgeons WAS: MMAP Buffers
Kevin Barnard wrote: > A ticketing system with work flow could help with transparency. > If it's setup correctly the work flow could help document where > the item is in the review process. As another idea maybe have a > status to indicate that the patch has been reviewed for > formatting. It might make things easier to deal with because a > ticket identified as WIP is obviously not ready for a CF etc etc. > Hell you may even be able to find somebody to take care of > reviewing formatting and dealing with those issues before it get's > sent to a committer. What you describe and more-is the intent of the CommifFest process and its related web application. If you review these links and have suggestions on how to improve the process, or how to make it more obvious to newcomers, we'd be happy to hear about them. http://wiki.postgresql.org/wiki/CommitFest http://wiki.postgresql.org/wiki/Submitting_a_Patch http://wiki.postgresql.org/wiki/Reviewing_a_Patch http://wiki.postgresql.org/wiki/RRReviewers https://commitfest.postgresql.org/action/commitfest_view/open This process has, in my opinion, been a very big improvement on the vagueness that came before. -Kevin -- 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] Visual Studio 2010/Windows SDK 7.1 support
On Sun, 06 Feb 2011 23:06:21 +0100, Brar Piening wrote: On Sun, 30 Jan 2011 21:26:22 +0100, Magnus Hagander wrote: it's not something we should hold up the CF / release for. I agree. At least it should get some more testing besides mine. [...] Being somewhat short of time in the next weeks I'm at least willing to rebase the patch on request and do some more testing or fix issues someone else has detected before the next release (9.2?) goes beta. After some months of being able to regularly compile current head using Visual Studio 2010 compilers and some more tests I no longer see any reason why this patch would change without external feedback. I've tested the latest version (http://www.piening.info/VS2010v6.patch) with the following config.pl for x86 $config->{perl} = 'C:\Perl'; # ActivePerl 5.8.9 Build 829 $config->{tcl} = 'C:\Tcl'; # ActiveState ActiveTcl 8.4.19.5 $config->{python} = 'C:\Python27'; # Python 2.7.1 $config->{openssl} = 'C:\openssl'; # openssl-1.0.0d $config->{nls} = 'C:\Dev\gnuwin32'; # GetText 0.14.4 $config->{krb5} = 'C:\Dev\kfw-3-2-2-final'; $config->{xml} = 'C:\Dev\libxml2-2.7.7.win32'; $config->{xslt} = 'C:\Dev\libxslt-1.1.26.win32'; $config->{iconv} = 'C:\Dev\iconv-1.9.2.win32'; $config->{zlib} = 'C:\Dev\zlib125'; for x64 I've tested a less extensive configuration as it's still hard to get the above libraries as x64 binaries. I'd appreciate any reviews, tests or comments. If someone with some more C skills than me could silence the warnings this would be a huge gain for build speed and testing. My previous attempts silenced the warnings but built a non-connectable backend. Regards, Brar -- 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] Formatting Curmudgeons WAS: MMAP Buffers
On May 9, 2011, at 12:53 PM, Josh Berkus wrote: > > 2) Our process for reviewing and approving patches, and what criteria > such patches are required to meet, is *very* opaque to a first-time > submitter (as in no documentation the submitter knows about), and does > not become clearer as they go through the process. Aster, for example, > was completely unable to tell the difference between hackers who were > giving them legit feedback, and random list members who were > bikeshedding. As a result, they were never able to derive a concrete > list of "these are the things we need to fix to make the patch > acceptable," and gave up. > I know I'm not in the position to talk work flow as it effects others and not myself, but I though I would at least throw out an idea. Feel free to completely shoot it down and I won't take any offense. A ticketing system with work flow could help with transparency. If it's setup correctly the work flow could help document where the item is in the review process. As another idea maybe have a status to indicate that the patch has been reviewed for formatting. It might make things easier to deal with because a ticket identified as WIP is obviously not ready for a CF etc etc. Hell you may even be able to find somebody to take care of reviewing formatting and dealing with those issues before it get's sent to a committer. I know the core group is use to the mailing lists so a system that can be integrated into the mailing list would be a must I think. That shouldn't be too hard to setup. I don't think this is a cure all but transparency to status in the process is surely going to give first timers more of a warm and fuzzy. -- Kevin Barnard kevinbarn...@mac.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Double ocurring Subplan
I am listing the query, it's explain output and explain analyze output at the end. The EXPLAIN output shows the 'Subplan 2' node only once, whereas EXPLAIN ANALYZE output shows that the 'Subplan 2' is being executed twice . Is that true? Or is it just the plan printer getting confused? Is the confusion because of the 2 conditions in the WHERE clause of the correlated subquery? PG Version: PostgreSQL 9.0.3 on x86_64-unknown-linux-gnu, compiled by GCC gcc (Ubuntu/Linaro 4.4.4-14ubuntu5) 4.4.5, 64-bit The query: select d.m1 - h.m1 from tz_test as d join tz_test_history as h on d.server_id = h.server_id where d.server_id = 5 and h.recorded_time = (select max(recorded_time) from tz_test_history as h2 where h2.server_id = d.server_id and h2.recorded_time < d.recorded_time); The explain output: QUERY PLAN Nested Loop (cost=2.26..15.54 rows=1 width=8) -> Seq Scan on tz_test d (cost=0.00..1.62 rows=1 width=16) Filter: (server_id = 5) -> Index Scan using tz_test_hist_rec_time_idx on tz_test_history h (cost=2.26..11.64 rows=1 width=16) Index Cond: (h.recorded_time = (SubPlan 2)) Filter: (h.server_id = 5) SubPlan 2 -> Result (cost=2.25..2.26 rows=1 width=0) InitPlan 1 (returns $2) -> Limit (cost=0.00..2.25 rows=1 width=8) -> Index Scan Backward using tz_test_hist_rec_time_idx on tz_test_history h2 (cost=0.00..10800.38 rows=4800 width=8) Index Cond: ((recorded_time IS NOT NULL) AND (recorded_time < $1)) Filter: (server_id = $0) explain analyze output: QUERY PLAN -- Nested Loop (cost=2.26..15.54 rows=1 width=8) (actual time=0.236..0.289 rows=1 loops=1) -> Seq Scan on tz_test d (cost=0.00..1.62 rows=1 width=16) (actual time=0.031..0.072 rows=1 loops=1) Filter: (server_id = 5) -> Index Scan using tz_test_hist_rec_time_idx on tz_test_history h (cost=2.26..11.64 rows=1 width=16) (actual time=0.103..0.112 rows=1 loops=1) Index Cond: (h.recorded_time = (SubPlan 2)) Filter: (h.server_id = 5) SubPlan 2 -> Result (cost=2.25..2.26 rows=1 width=0) (actual time=0.078..0.080 rows=1 loops=1) InitPlan 1 (returns $2) -> Limit (cost=0.00..2.25 rows=1 width=8) (actual time=0.064..0.066 rows=1 loops=1) -> Index Scan Backward using tz_test_hist_rec_time_idx on tz_test_history h2 (cost=0.00..10800.38 rows=4800 width=8) (actual time=0.058..0.058 rows=1 loops=1) Index Cond: ((recorded_time IS NOT NULL) AND (recorded_time < $1)) Filter: (server_id = $0) SubPlan 2 -> Result (cost=2.25..2.26 rows=1 width=0) (actual time=0.078..0.080 rows=1 loops=1) InitPlan 1 (returns $2) -> Limit (cost=0.00..2.25 rows=1 width=8) (actual time=0.064..0.066 rows=1 loops=1) -> Index Scan Backward using tz_test_hist_rec_time_idx on tz_test_history h2 (cost=0.00..10800.38 rows=4800 width=8) (actual time=0.058..0.058 rows=1 loops=1) Index Cond: ((recorded_time IS NOT NULL) AND (recorded_time < $1)) Filter: (server_id = $0) Total runtime: 0.525 ms (21 rows) -- Gurjeet Singh EnterpriseDB Corporation The Enterprise PostgreSQL Company
[HACKERS] Reducing overhead of frequent table locks
On Fri, May 13, 2011 at 09:07:34AM -0400, Robert Haas wrote: > Actually, it's occurred to me from time to time that it would be nice > to eliminate ACCESS SHARE (and while I'm dreaming, maybe ROW SHARE and > ROW EXCLUSIVE) locks for tables as well. Under normal operating > conditions (i.e. no DDL running), these locks generate a huge amount > of lock manager traffic even though none of the locks conflict with > each other. Unfortunately, I don't really see a way to make this > work. But maybe it would at least be possible to create some sort of > fast path. For example, suppose every backend opens a file and uses > that file to record lock tags for the objects on which it is taking > "weak" (ACCESS SHARE/ROW SHARE/ROW EXCLUSIVE) locks on. Before taking > a "strong" lock (anything that conflicts with one of those lock > types), the exclusive locker is required to open all of those files > and transfer the locks into the lock manager proper. Of course, it's > also necessary to nail down the other direction: you have to have some > way of making sure that the backend can't record in it's local file a > lock that would have conflicted had it been taken in the actual lock > manager. But maybe there's some lightweight way we could detect that, > as well. For example, we could keep, say, a 1K array in shared > memory, representing a 1024-way partitioning of the locktag space. > Each byte is 1 if there are any "strong" locks on objects with that > locktag in the lock manager, and 0 if there are none (or maybe you > need a 4K array with exact counts, for bookkeeping). When a backend > wants to take a "weak" lock, it checks the array: if it finds a 0 then > it just records the lock in its file; otherwise, it goes through the > lock manager. When a backend wants a "strong" lock, it first sets the > byte (or bumps the count) in the array, then transfers any existing > weak locks from individual backends to the lock manager, then tries to > get its own lock. Possibly the array operations could be done with > memory synchronization primitives rather than spinlocks, especially on > architectures that support an atomic fetch-and-add. Of course I don't > know quite how we recover if we try to do one of these "lock > transfers" and run out of shared memory... and overall I'm hand-waving > here quite a bit, but in theory it seems like we ought to be able to > rejigger this locking so that we reduce the cost of obtaining a "weak" > lock, perhaps at the expense of making it more expensive to obtain a > "strong" lock, which are relatively rare by comparison. > > The key is putting a rapid hard stop to all fast-path lock acquisitions and then reconstructing a valid global picture of the affected lock table regions. Your 1024-way table of strong lock counts sounds promising. (Offhand, I do think they would need to be counts, not just flags.) If I'm understanding correctly, your pseudocode would look roughly like this: if (level >= ShareUpdateExclusiveLock) ++strong_lock_counts[my_strong_lock_count_partition] sfence if (strong_lock_counts[my_strong_lock_count_partition] == 1) /* marker 1 */ import_all_local_locks normal_LockAcquireEx else if (level <= RowExclusiveLock) lfence if (strong_lock_counts[my_strong_lock_count_partition] == 0) /* marker 2 */ local_only /* marker 3 */ else normal_LockAcquireEx else normal_LockAcquireEx At marker 1, we need to block until no code is running between markers two and three. You could do that with a per-backend lock (LW_SHARED by the strong locker, LW_EXCLUSIVE by the backend). That would probably still be a win over the current situation, but it would be nice to have something even cheaper. Then you have the actual procedure for transfer of local locks to the global lock manager. Using record locks in a file could work, but that's a system call per lock acquisition regardless of the presence of strong locks. Is that cost sufficiently trivial? I wonder if, instead, we could signal all backends at marker 1 to dump the applicable parts of their local (memory) lock tables to files. Or to another shared memory region, if that didn't mean statically allocating the largest possible required amount. If we were willing to wait until all backends reach a CHECK_FOR_INTERRUPTS, they could instead make the global insertions directly. That might yield a decent amount of bug swatting to fill in missing CHECK_FOR_INTERRUPTS, though. Improvements in this area would also have good synergy with efforts to reduce the global impact of temporary table usage. CREATE TEMP TABLE can be the major source of AccessExclusiveLock acquisitions. However, with the strong lock indicator partitioned 1024 wa
Re: [HACKERS] Foreign table permissions and cloning
2011/5/11 Shigeru Hanada : > (2011/04/26 5:42), Robert Haas wrote: >> OK. Turned out a little more cleanup was needed to make this all the >> way consistent with how we handle views; I have now done that. > > I noticed that some fixes would be needed for consistency about foreign > table privileges. Attached patch includes fixes below: > > 1) psql doesn't complete FOREIGN TABLE after GRANT/REVOKE. > 2) pg_dump uses GRANT .. ON TABLE for foreign tables, instead of ON > FOREIGN TABLE. > 3) GRANT document mentions that ALL TABLES includes foreign tables too. > 4) Rows of information_schema.foreign_tables/foreign_table_options are > visible to users who have any privileges on the foreign table. Thanks; I'm embarrassed I didn't notice those things myself. I've committed this patch with very slight adjustment. -- 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] hint bit cache v6
what's changed: *) as advertised, i'm no longer bothering to cache invalid bits. hint bit i/o via rollbacked transactions is not a big deal IMNSHO, so they will work as they have always done. *) all the tuple visibility routines are now participating in the cache *) xmax commit bits are now being cached, not just xmin. this prevents hint bit i/o following deletes. *) juggled the cache interaction a bit so the changes to the visibility routines could be a lot more surgical. specifically, I reverted SetHintBits() to return void and put the cache adjustment inside 'SetHintBitsCache()' which sets the bits, dirties, and adjusts the cache. SetHintBitsNonDirty() only sets the bits without dirtying the page, and is called when you get a cache hit. *) various minor cleanups, decided to keep pageindex as type TransactionId since that's what clog does. *) made a proper init function, put cache data into CacheMemoryContext, and moved the cache data pointer references into the page structure performance testing done: *) select only pgbench and standard i/o pgbech tests don't show performance difference within statistical noise. The test I need to do, a cache and clog thrashing test, I haven't found a clean way to put together yet. I'm really skeptical that any problems will show up there. That said, I am satisfied the patch does what it is supposed to do -- eliminates hint bit i/o without for cases where it is a big headache without penalizing other cases. Anyone that would like to comment on the technique or other points of the patch please do so (otherwise it's time for the 'fest). merlin hbache_v6.patch Description: Binary data -- 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] Debug contrib/cube code
Robert Haas writes: > On Fri, May 13, 2011 at 12:12 PM, Nick Raj wrote: >> Can you throw some light on how to debug contrib/cube code? > On my system, if I configure --enable-debug, the contrib modules are > compiled with debug support as well. Depending on what platform you're using, it may be hard to get breakpoints in dynamically loaded modules to work. On many platforms it helps to make sure that the module is loaded before you attach to the backend process with gdb. To do that, you can either call one of its functions, or just do a manual LOAD command. 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] Debug contrib/cube code
On Fri, May 13, 2011 at 12:12 PM, Nick Raj wrote: > Sorry i don't know about AFAICS. > Yes, i want to modify cube code for this i want to go in detail by > debugging. > Can you throw some light on how to debug contrib/cube code? "AFAICS" is short for "as far as I can see". On my system, if I configure --enable-debug, the contrib modules are compiled with debug support as well. -- 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] Backpatching of "Teach the regular expression functions to do case-insensitive matching"
Peter Eisentraut writes: > On ons, 2011-05-11 at 16:47 -0400, Tom Lane wrote: >> Hm, do you know how to enumerate the available locales on Windows? > EnumSystemLocalesEx() > Reference: > http://msdn.microsoft.com/en-us/library/dd317829(v=vs.85).aspx > Example: http://msdn.microsoft.com/en-us/library/dd319091(v=vs.85).aspx > As you can see in the example, this returns names like "en-US" and > "es-ES". I would imagine we normalize this to the usual "en_US", > "es_ES" (but we could also install the not normalized names, just like > we install "en_US.utf8"). I poked around in Microsoft's documentation a bit. It's not entirely clear to me that the locale names enumerated by this API match up with the names accepted by setlocale() --- in particular, http://msdn.microsoft.com/en-us/library/hzz3tw78(v=VS.90).aspx does not show any two-letter abbreviations for most of the languages. That would need some testing, but it seems likely that what we'd have to do is construct the longer-form locale name using info obtained from GetLocaleInfoEx. Also, the locale names from EnumSystemLocalesEx definitely don't include any code page identifier. We could probably just enter the alias under UTF8, and again under the ANSI code page number from GetLocaleInfoEx, if the locale has one. > There is an older interface EnumSystemLocales() which returns locale > IDs, which you then have to look up and convert into a name manually. > There is code for that in the old installer CVS on pgfoundry. But it's > very ugly, so I'd rather skip that and just concentrate on supporting > the newer interface. Another thing I found out from the docs is that locale IDs aren't unique in Vista and later: they have "supplemental" locales that have distinct names but share the same ID as the primary. It's entirely unclear how that maps to setlocale names, but it does seem like we probably don't want to use EnumSystemLocales. 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] Debug contrib/cube code
Sorry i don't know about AFAICS. Yes, i want to modify cube code for this i want to go in detail by debugging. Can you throw some light on how to debug contrib/cube code? On Fri, May 6, 2011 at 9:59 PM, Euler Taveira de Oliveira wrote: > Em 06-05-2011 02:14, Nick Raj escreveu: > > I am using postgresql-8.4.6. I want to debug the contrib/cube code. Can >> we able to debug that cube code? Because there is no .configure file >> to enable debug. Is there is any way to change make file to enable debug? >> >> What do you want to debug? AFAICS you need to change the code to achieve > what you want. > > > -- > Euler Taveira de Oliveira - Timbira http://www.timbira.com.br/ > PostgreSQL: Consultoria, Desenvolvimento, Suporte 24x7 e Treinamento >
Re: [HACKERS] SSI-related code drift between index_getnext() and heap_hot_search_buffer()
Robert Haas wrote: > Kevin Grittner wrote: >> Anyway, I could clean up all but that last issue in the old code. >> I'm not sure whether that makes sense if you're refactoring it >> anyway. Would you like me to look at the refactored code to >> suggest fixes? Would you rather do it yourself based on my >> answers here? Do we need to sort out that last issue before >> proceeding on the others? > I haven't a clue how to fix this. What I was doing was of course > targeted toward 9.2, but I have half a thought that making > index_getnext() call heap_hot_search_buffer() might be a sensible > thing to do in 9.1, because code duplication = more bugs. On the > third hand, at the moment the code that Heikki wrote to do that is > tangled up in a bunch of other things that we almost certainly > don't want to do in 9.1, and it's not obvious that it can be > cleanly untangled, so maybe that's not the right idea after all. > > I think a good starting point might be to design a test case that > fails with the current code, and come up with a plan for what to > do about it. I have a very ugly feeling about this problem. I > agree with your feeling that chasing down the update pointers > isn't sensible, but a whole-relation lock seems like it could lead > to a lot more rollbacks. OK, will work on a test case for this last issue, but it might make sense to address some of the other points separately first. For one thing it might allow you to continue on with your 9.2 work with clean tests. I can't do much on any of it today, as I have to deal with some other things before being away for a week. This is such a remote corner case that it would be really good if we can limit the relation locks to cases where we're somewhere near that corner. I've been trying to work out how to do that -- not there yet, but I see some glimmers of how it might be done. The nice thing about putting together a test case for something this hard to hit is that it helps clarify the dynamics of the problem, and solutions sometimes just pop out of it. FWIW, so far what I know is that it will take an example something like the one shown here: http://archives.postgresql.org/pgsql-hackers/2011-02/msg00325.php with the further requirements that the update in T3 must not be a HOT update, T1 would still need to acquire a snapshot before T2 committed while moving its current select down past the commit of T3, and that select would need to be modified so that it would scan the visible tuple and then stop (e.g., because of a LIMIT) before reaching the tuple which represents the next version of the row. -Kevin -- 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] Unix latch implementation that wakes on postmaster death
On 13 May 2011 16:18, Robert Haas wrote: > On Fri, May 13, 2011 at 10:48 AM, Tom Lane wrote: >> I'm not that thrilled with the "life sign" terminology, but don't >> have a better idea right offhand. > > Yeah, that made no sense to me. Can't we just refer to detecting > postmaster death? Fine by me. -- Peter Geoghegan http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training and 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] Unix latch implementation that wakes on postmaster death
On Fri, May 13, 2011 at 10:48 AM, Tom Lane wrote: > I'm not that thrilled with the "life sign" terminology, but don't > have a better idea right offhand. Yeah, that made no sense to me. Can't we just refer to detecting postmaster death? -- 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] Fw: [BUGS] BUG #6011: Some extra messages are output in the event log at PostgreSQL startup
"MauMau" wrote: > From: "Robert Haas" >> I think Tom had the right idea upthread: what we should do is >> make the "-s" option to pg_ctl suppress these messages (as it >> does with similar messages on Linux). Removing them altogether >> seems like overkill, for the reasons you mention. Agreed. > So I'll wait for more opinions for a week, then write and test a > patch, and submit it as a reply to this mail thread. Sounds reasonable. > Treat it as a bug. (I hope some committer will kindly back-patch > to older versions.) > > Make pg_ctl's -s option suppress informational event logging. This will ultimately be up to a committer (and I'm not one), but to me it seems reasonable to back-patch if it is addressed this way. > Existing software which use PostgreSQL needs to be modified to add > -s to pg_ctl. Moreover, pg_ctl unregister and pg_ctl register must > be performed to make the patch effective in existing > installations. That is probably a very *good* thing if you want it to be considered for back-patch. Having a bug fix release arbitrarily change existing behavior which isn't data-destroying or a security risk is not a good thing and is contrary to PostgreSQL policy for maintaining stable branches. We can't know who might be, for example, pulling such messages out of their logs for reporting or monitoring purposes. If we made changes that can conceivably break things on applying bug fix releases, we would have fewer people applying them, and that would be bad for everyone. > The two messages in question may be just annoying to users, and > they might want those messages to disappear without -s. They claim > that it is inconsistent that those messages are not recorded in > syslog on UNIX/Linux. I can only dream of what it's like to work somewhere that fussing over two informational log messages on an infrequent event like restarting a database (that *is* an infrequent event, right?) would be something I had time for. They are very fortunate people to be in such a position. It would appear that finding the time to add the -s switch shouldn't be too hard in such an environment. > the PostgreSQL Windows service must be registered by "pg_ctl > register -s" to make use of this patch. However, according to the > current manual, "pg_ctl register" does not take -s option. > Actually, pg_ctl does not refuse to take -s, so this is not a big > problem. > > pg_ctl register [-N servicename] [-U username] [-P password] > [-D datadir] [-w] [-t seconds] [-o options] When you write the patch, be sure to include a fix for the docs here, please. Thanks for taking the time to work through the issue. -Kevin -- 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] Unix latch implementation that wakes on postmaster death
Peter Geoghegan writes: > Attached is a patch that builds upon Florian Pflug's earlier proof of > concept program for monitoring the postmaster. Cool. Like Robert, no time to review this in detail now, but ... > How should I be handling the EXEC_BACKEND case? Assuming that the open pipe descriptor is inherited across exec on Windows (and if it's not, we're back to square one) all you should have to do is get the pipe descriptor variables passed down to the child processes. There's some grotty code in postmaster.c that's used for this purpose --- see struct BackendParameters and associated functions. Just add some code there to pass down the values. I'm not that thrilled with the "life sign" terminology, but don't have a better idea right offhand. 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] Fw: [BUGS] BUG #6011: Some extra messages are output in the event log at PostgreSQL startup
From: "Robert Haas" On Thu, May 12, 2011 at 2:57 PM, Kevin Grittner wrote: I wish the fix will be back-patched in 8.3, too. I guess the question is whether this is a bug which causes more problems than the potential breakage which might ensue for someone who relies on the current behavior. How sure can you be that nobody relies on seeing those messages? No information (like a history of database start times) is lost without these entries? I think Tom had the right idea upthread: what we should do is make the "-s" option to pg_ctl suppress these messages (as it does with similar messages on Linux). Removing them altogether seems like overkill, for the reasons you mention. Thank you, Magnus, Tom, Dave, Kevin, and Robert. Maybe I could get a consensus of opinion on the treatment of bug #6011. The summary is shown below. However, as mentioned previously, there are some concerns. So I'll wait for more opinions for a week, then write and test a patch, and submit it as a reply to this mail thread. How to treat Treat it as a bug. (I hope some committer will kindly back-patch to older versions.) Make pg_ctl's -s option suppress informational event logging. The modifications are: 1. write_event_log() (pg_ctl.c) If "-s" was specified and the level argument is EVENTLOG_INFORMATION_TYPE, just return without doing anything. 2. pgwin32_CommandLine() (pg_ctl.c) If "-s" was specified when running "pg_ctl register", add "-s" to "pg_ctl runservice" command line built in this function. Concerns Existing software which use PostgreSQL needs to be modified to add -s to pg_ctl. Moreover, pg_ctl unregister and pg_ctl register must be performed to make the patch effective in existing installations. The two messages in question may be just annoying to users, and they might want those messages to disappear without -s. They claim that it is inconsistent that those messages are not recorded in syslog on UNIX/Linux. As described in "How to treat", the PostgreSQL Windows service must be registered by "pg_ctl register -s" to make use of this patch. However, according to the current manual, "pg_ctl register" does not take -s option. Actually, pg_ctl does not refuse to take -s, so this is not a big problem. pg_ctl register [-N servicename] [-U username] [-P password] [-D datadir] [-w] [-t seconds] [-o options] Regards, MauMau -- 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] 'tuple concurrently updated' error for alter role ... set
Is this a TODO? I don't see it on the TODO list. --- Robert Haas wrote: > On Fri, May 13, 2011 at 12:56 AM, Tom Lane wrote: > > BTW, I thought a bit more about why I didn't like the initial proposal > > in this thread, and the basic objection is this: the AccessShareLock or > > RowExclusiveLock we take on the catalog is not meant to provide any > > serialization of operations on individual objects within the catalog. > > What it's there for is to interlock against operations that are > > operating on the catalog as a table, such as VACUUM FULL (which has to > > lock out all accesses to the catalog) or REINDEX (which has to lock out > > updates). ?So the catalog-level lock is the right thing and shouldn't be > > changed. ?If we want to interlock updates of individual objects then we > > need a different locking concept for that. > > Right, I agree. Fortunately, we don't have to invent a new one. > There is already locking being done exactly along these lines for > DROP, COMMENT, and SECURITY LABEL (which is important, because > otherwise we could leave behind orphaned security labels that would be > inherited by a later object with the same OID, leading to a security > problem). I think it would be sensible, and quite simple, to extend > that to other DDL operations. > > I think that we probably *don't* want to lock non-table objects when > they are just being *used*. We do that for tables (to lock against > concurrent drop operations) and in some workloads it becomes a severe > bottleneck. Doing it for functions and operators would make the > problem far worse, for no particular benefit. Unlike tables, there is > no underlying relation file to worry about, so the worst thing that > happens is someone continues to use a dropped object slightly after > it's gone, or the old definition of an object that's been modified. > > Actually, it's occurred to me from time to time that it would be nice > to eliminate ACCESS SHARE (and while I'm dreaming, maybe ROW SHARE and > ROW EXCLUSIVE) locks for tables as well. Under normal operating > conditions (i.e. no DDL running), these locks generate a huge amount > of lock manager traffic even though none of the locks conflict with > each other. Unfortunately, I don't really see a way to make this > work. But maybe it would at least be possible to create some sort of > fast path. For example, suppose every backend opens a file and uses > that file to record lock tags for the objects on which it is taking > "weak" (ACCESS SHARE/ROW SHARE/ROW EXCLUSIVE) locks on. Before taking > a "strong" lock (anything that conflicts with one of those lock > types), the exclusive locker is required to open all of those files > and transfer the locks into the lock manager proper. Of course, it's > also necessary to nail down the other direction: you have to have some > way of making sure that the backend can't record in it's local file a > lock that would have conflicted had it been taken in the actual lock > manager. But maybe there's some lightweight way we could detect that, > as well. For example, we could keep, say, a 1K array in shared > memory, representing a 1024-way partitioning of the locktag space. > Each byte is 1 if there are any "strong" locks on objects with that > locktag in the lock manager, and 0 if there are none (or maybe you > need a 4K array with exact counts, for bookkeeping). When a backend > wants to take a "weak" lock, it checks the array: if it finds a 0 then > it just records the lock in its file; otherwise, it goes through the > lock manager. When a backend wants a "strong" lock, it first sets the > byte (or bumps the count) in the array, then transfers any existing > weak locks from individual backends to the lock manager, then tries to > get its own lock. Possibly the array operations could be done with > memory synchronization primitives rather than spinlocks, especially on > architectures that support an atomic fetch-and-add. Of course I don't > know quite how we recover if we try to do one of these "lock > transfers" and run out of shared memory... and overall I'm hand-waving > here quite a bit, but in theory it seems like we ought to be able to > rejigger this locking so that we reduce the cost of obtaining a "weak" > lock, perhaps at the expense of making it more expensive to obtain a > "strong" lock, which are relatively rare by comparison. > > > > -- > 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 -- 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
Re: [HACKERS] 'tuple concurrently updated' error for alter role ... set
On Fri, May 13, 2011 at 12:56 AM, Tom Lane wrote: > BTW, I thought a bit more about why I didn't like the initial proposal > in this thread, and the basic objection is this: the AccessShareLock or > RowExclusiveLock we take on the catalog is not meant to provide any > serialization of operations on individual objects within the catalog. > What it's there for is to interlock against operations that are > operating on the catalog as a table, such as VACUUM FULL (which has to > lock out all accesses to the catalog) or REINDEX (which has to lock out > updates). So the catalog-level lock is the right thing and shouldn't be > changed. If we want to interlock updates of individual objects then we > need a different locking concept for that. Right, I agree. Fortunately, we don't have to invent a new one. There is already locking being done exactly along these lines for DROP, COMMENT, and SECURITY LABEL (which is important, because otherwise we could leave behind orphaned security labels that would be inherited by a later object with the same OID, leading to a security problem). I think it would be sensible, and quite simple, to extend that to other DDL operations. I think that we probably *don't* want to lock non-table objects when they are just being *used*. We do that for tables (to lock against concurrent drop operations) and in some workloads it becomes a severe bottleneck. Doing it for functions and operators would make the problem far worse, for no particular benefit. Unlike tables, there is no underlying relation file to worry about, so the worst thing that happens is someone continues to use a dropped object slightly after it's gone, or the old definition of an object that's been modified. Actually, it's occurred to me from time to time that it would be nice to eliminate ACCESS SHARE (and while I'm dreaming, maybe ROW SHARE and ROW EXCLUSIVE) locks for tables as well. Under normal operating conditions (i.e. no DDL running), these locks generate a huge amount of lock manager traffic even though none of the locks conflict with each other. Unfortunately, I don't really see a way to make this work. But maybe it would at least be possible to create some sort of fast path. For example, suppose every backend opens a file and uses that file to record lock tags for the objects on which it is taking "weak" (ACCESS SHARE/ROW SHARE/ROW EXCLUSIVE) locks on. Before taking a "strong" lock (anything that conflicts with one of those lock types), the exclusive locker is required to open all of those files and transfer the locks into the lock manager proper. Of course, it's also necessary to nail down the other direction: you have to have some way of making sure that the backend can't record in it's local file a lock that would have conflicted had it been taken in the actual lock manager. But maybe there's some lightweight way we could detect that, as well. For example, we could keep, say, a 1K array in shared memory, representing a 1024-way partitioning of the locktag space. Each byte is 1 if there are any "strong" locks on objects with that locktag in the lock manager, and 0 if there are none (or maybe you need a 4K array with exact counts, for bookkeeping). When a backend wants to take a "weak" lock, it checks the array: if it finds a 0 then it just records the lock in its file; otherwise, it goes through the lock manager. When a backend wants a "strong" lock, it first sets the byte (or bumps the count) in the array, then transfers any existing weak locks from individual backends to the lock manager, then tries to get its own lock. Possibly the array operations could be done with memory synchronization primitives rather than spinlocks, especially on architectures that support an atomic fetch-and-add. Of course I don't know quite how we recover if we try to do one of these "lock transfers" and run out of shared memory... and overall I'm hand-waving here quite a bit, but in theory it seems like we ought to be able to rejigger this locking so that we reduce the cost of obtaining a "weak" lock, perhaps at the expense of making it more expensive to obtain a "strong" lock, which are relatively rare by comparison. -- 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] Formatting Curmudgeons WAS: MMAP Buffers
On Thu, May 12, 2011 at 11:55 AM, Kevin Grittner wrote: > Robert Haas wrote: > >> Unfortunately, people often come into our community with incorrect >> assumptions about how it works, including: >> >> - someone's in charge >> - there's one right answer >> - it's our job to fix your problem > > Would it make sense to dispel such notions explicitly in the > Developer FAQ? Can't hurt, though these principles also apply (perhaps even more strongly) to bug reports and people wanting support. -- 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] Unix latch implementation that wakes on postmaster death
On Fri, May 13, 2011 at 8:06 AM, Peter Geoghegan wrote: > Attached is a patch that builds upon Florian Pflug's earlier proof of > concept program for monitoring the postmaster. The code creates a > non-blocking pipe in the postmaster that child processes block on > using a select() call. This all occurs in the latch code, which now > monitors postmaster death, but only for clients that request it (and, > almost invariably in addition to monitoring other things, like having > a timeout occur or a latch set). > > I've implemented an interface originally sketched by Heikki that > allows clients to specify events to wake on, and to see what event > actually caused the wakeup when we're done by bitwise AND'ing the > returned int against various new bitmasks. > > I've included my existing changes to the archiver as a convenience to > anyone that wants to quickly see the effects of the patch in action; > even though we don't have a tight loop that polls PostmasterIsAlive() > every second, we still wake up on postmaster death, so there is no > potential denial of service as previously described by Tom. This can > be easily observed by sending the postmaster SIGKILL while the > archiver is on - the archiver immediately finishes. Note that I've > deferred changing the existing call sites of WaitLatch()/ > WaitLatchOrSocket(), except to make them use the new interface. Just > as before, they don't ask to be woken on postmaster death, even though > in some cases they probably should. Whether or not they should and how > they should are questions for another day though. I don't immediately have time to look at this, but it sounds awesome! Thank you very much for working on this! -- 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] Unix latch implementation that wakes on postmaster death
Attached is a patch that builds upon Florian Pflug's earlier proof of concept program for monitoring the postmaster. The code creates a non-blocking pipe in the postmaster that child processes block on using a select() call. This all occurs in the latch code, which now monitors postmaster death, but only for clients that request it (and, almost invariably in addition to monitoring other things, like having a timeout occur or a latch set). I've implemented an interface originally sketched by Heikki that allows clients to specify events to wake on, and to see what event actually caused the wakeup when we're done by bitwise AND'ing the returned int against various new bitmasks. I've included my existing changes to the archiver as a convenience to anyone that wants to quickly see the effects of the patch in action; even though we don't have a tight loop that polls PostmasterIsAlive() every second, we still wake up on postmaster death, so there is no potential denial of service as previously described by Tom. This can be easily observed by sending the postmaster SIGKILL while the archiver is on - the archiver immediately finishes. Note that I've deferred changing the existing call sites of WaitLatch()/ WaitLatchOrSocket(), except to make them use the new interface. Just as before, they don't ask to be woken on postmaster death, even though in some cases they probably should. Whether or not they should and how they should are questions for another day though. I expect that this patch will be split into two separate patches: The latch patch (complete with currently missing win32 implementation) and the archiver patch. For now, I'd like to hear thoughts on how I've implemented the extra latch functionality. How should I be handling the EXEC_BACKEND case? -- Peter Geoghegan http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training and Services diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index e71090f..b1d38f5 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -10150,7 +10150,7 @@ retry: /* * Wait for more WAL to arrive, or timeout to be reached */ - WaitLatch(&XLogCtl->recoveryWakeupLatch, 500L); + WaitLatch(&XLogCtl->recoveryWakeupLatch, WL_LATCH_SET | WL_TIMEOUT, 500L); ResetLatch(&XLogCtl->recoveryWakeupLatch); } else diff --git a/src/backend/port/unix_latch.c b/src/backend/port/unix_latch.c index 6dae7c9..5bd389e 100644 --- a/src/backend/port/unix_latch.c +++ b/src/backend/port/unix_latch.c @@ -108,6 +108,19 @@ static void initSelfPipe(void); static void drainSelfPipe(void); static void sendSelfPipeByte(void); +/* + * Constants that represent which of a pair of fds given + * to pipe() is watched and owned in the context of + * dealing with life sign file descriptors + */ +#define LIFESIGN_FD_WATCH 0 +#define LIFESIGN_FD_OWN 1 + +/* + * 2 file descriptors that represent postmaster lifesign. + * First is LIFESIGN_FD_WATCH, second is LIFESIGN_FD_OWN. + */ +static int life_sign_fds[2]; /* * Initialize a backend-local latch. @@ -188,22 +201,22 @@ DisownLatch(volatile Latch *latch) * backend-local latch initialized with InitLatch, or a shared latch * associated with the current process by calling OwnLatch. * - * Returns 'true' if the latch was set, or 'false' if timeout was reached. + * Returns bit field indicating which condition(s) caused the wake-up. */ -bool -WaitLatch(volatile Latch *latch, long timeout) +int +WaitLatch(volatile Latch *latch, int wakeEvents, long timeout) { - return WaitLatchOrSocket(latch, PGINVALID_SOCKET, false, false, timeout) > 0; + return WaitLatchOrSocket(latch, wakeEvents, PGINVALID_SOCKET, timeout); } /* * Like WaitLatch, but will also return when there's data available in - * 'sock' for reading or writing. Returns 0 if timeout was reached, - * 1 if the latch was set, 2 if the socket became readable or writable. + * 'sock' for reading or writing. + * + * Returns bit field indicating which condition(s) caused the wake-up. */ int -WaitLatchOrSocket(volatile Latch *latch, pgsocket sock, bool forRead, - bool forWrite, long timeout) +WaitLatchOrSocket(volatile Latch *latch, int wakeEvents, pgsocket sock, long timeout) { struct timeval tv, *tvp = NULL; @@ -211,12 +224,13 @@ WaitLatchOrSocket(volatile Latch *latch, pgsocket sock, bool forRead, fd_set output_mask; int rc; int result = 0; + bool found = false; if (latch->owner_pid != MyProcPid) elog(ERROR, "cannot wait on a latch owned by another process"); /* Initialize timeout */ - if (timeout >= 0) + if (timeout >= 0 && (wakeEvents & WL_TIMEOUT)) { tv.tv_sec = timeout / 100L; tv.tv_usec = timeout % 100L; @@ -224,7 +238,7 @@ WaitLatchOrSocket(volatile Latch *latch, pgsocket sock, bool forRead, } waiting = true; - for (;;) + do { int hifd; @@ -235,16 +249,29 @@ WaitLatchOrSocket(volatile Lat
Re: [HACKERS] Formatting Curmudgeons WAS: MMAP Buffers
Heikki Linnakangas writes: >> Anyway, I think the intro message should be "Don't submit a big patch to >> PostgreSQL until you've done a small patch and some patch review" >> instead though. > > Well, my first patch was two-phase commit. And I had never even used > PostgreSQL before I dived into the source tree and started to work on > that. I did, however, lurk on the pgsql-hackers mailing list for a few > months before posting, so I knew the social dynamics. I basically did > exactly what Robert described elsewhere in this thread, and successfully > avoided the culture shock. I tend to share the experience, my first patch (not counting documentation patch) has been extensions. The fact that everybody here knew me before (from side projects, events, reviews in commit fests, design reviews on list…) certainly helped, but the real deal has been that the design was agreed on by everybody before I started — that took *lots of* time, but really paid off (good ideas all around, real buy in, some good beers shared, etc). 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] windows installer (similar to old EnterpriseDB installer)
On Fri, May 13, 2011 at 6:05 AM, Martin Belleau wrote: > Hi, > > First, sorry - I really didn't know to which list to post this. > > I'm looking to either write or get access to something like the EnterpriseDB > installer for windows, which doesn't seem to be kept up to date anymore. > > The installer needs to be configurable through command line so I can set the > database password and other options automatically. In particular I would > need pgAdmin and libpq to use postgresql from my 32-bit C++ appplication. > > Does such a thing exist for version 9.0? If not, any tips on how to get > started? The EnterpriseDB installer is up to date - we build, test and release it as part of the PostgreSQL community release process. -- Dave Page Blog: http://pgsnake.blogspot.com Twitter: @pgsnake EnterpriseDB UK: 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