Re: [HACKERS] modeling parallel contention (was: Parallel Append implementation)
On Sat, Aug 5, 2017 at 6:17 PM, Peter Geogheganwrote: > On Thu, May 4, 2017 at 7:20 PM, David Rowley > wrote: >> I ended up writing the attached (which I'd not intended to post until >> some time closer to when the doors open for PG11). At the moment it's >> basically just a test patch to see how it affects things when we give >> workers a bit more to do before they come back to look for more work. >> In this case, I've just given them 10 pages to work on, instead of the >> 1 that's allocated in 9.6 and v10. > > I think that this could benefit parallel sort, beyond the obvious fact > that it too must have the contention you describe. > > We generally are faster at sorting presorted input for all kinds of > reasons (e.g., insertion sort fallback for quicksort, merging based on > replacement of heap's root item). It follows that it's to our > advantage to have parallel tuplesort read multiple pages in a range > into a worker at once within the parallel heap scan that feeds > workers. The leader, which merges worker runs, may ultimately have to > perform fewer comparisons as a result of this, which is where most of > the benefit would be. On the other hand, it could hurt Gather Merge for essentially symmetric reasons - Gather Merge works best if all the tuples are in roughly the same range of values. Otherwise the work isn't equally distributed. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_stop_backup(wait_for_archive := true) on standby server
On Sat, Aug 5, 2017 at 4:28 PM, Paul A Jungwirthwrote: > I don't have an opinion on the urgency of back-porting a fix, but if > pg_stop_backup(boolean) allows for inconsistent backups, it does sound > like a problem on 9.6 too. It doesn't. The talk about inconsistent backups is, I think, not a very precise way of speaking. All backups taken by copying the data directory are inconsistent until you run recovery for long enough to make them consistent. What we're talking about is whether it's guaranteed that all WAL segments needed to reach consistency will be archived by the time pg_stop_backup() returns. However, even if they're not, it doesn't mean that there's anything wrong with your backup per se; it just means you need to replay WAL files that were not in the archive at the time pg_stop_backup() completed when restoring it. So, for example, if you keep an archive of all of your WAL files for PITR purposes, you're fine. IIUC, to have a problem, you have to do the following: 1. Take a backup from the standby, not the master. 2. Take a backup using pg_start_backup() and pg_stop_backup(), not pg_basebackup. 3. Instead of keeping all of your WAL files, just keep the absolute minimum number required to restore the backup. 4. Instead of keeping the WAL files through the LSN returned by pg_stop_backup(), only keep the ones that were archived before pg_stop_backup() returned. All of (1)-(3) are legitimate user choices, although not everyone will make them. (4) is unfortunately the procedure recommended by our documentation, which is where the problem comes in. I think it's pretty lame that the documentation recommends ignoring the return value of pg_stop_backup(); ISTM that it would be very wise to recommend cross-checking the return value against the WAL files you're keeping for the backup. Even if we thought the waiting logic was working perfectly in every case, having a cross-check on something as important as backup integrity seems like an awfully good idea. For example, *even if* we were to apply the patch Michael Paquier proposed, it doesn't actually do anything except emit a warning when archive_mode isn't set to always, and that warning could easily be missed by an automated backup script, and archive_mode=always is probably not a common setting. So you still have the same problem in most cases. I think the root of this problem is that commit 7117685461af50f50c03f43e6a622284c8d54694 did only a pretty perfunctory update of the documentation, as the commit message itself admits: Only reference documentation is included. The main section on backup still needs to be rewritten to cover this, but since that is already scheduled for a separate large rewrite, it's not included in this patch. But it doesn't look like that ever really got done. https://www.postgresql.org/docs/10/static/continuous-archiving.html#backup-lowlevel-base-backup really doesn't talk at all about standbys or differences in required procedures between masters and standbys. It makes statements that are flagrantly riduculous in the case of a standby, like claiming that pg_start_backup() always performs a checkpoint and that pg_stop_backup "terminates the backup mode and performs an automatic switch to the next WAL segment." Well, obviously not. And at least to me, that's the real bug here. Somebody needs to go through and fix this documentation properly. -- 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] Re: Crash report for some ICU-52 (debian8) COLLATE and work_mem values
Adding -hackers. On Sat, Aug 05, 2017 at 03:55:13PM -0700, Noah Misch wrote: > On Thu, Aug 03, 2017 at 11:42:25AM -0700, Peter Geoghegan wrote: > > On Thu, Aug 3, 2017 at 8:49 AM, Daniel Verite> > wrote: > > > With query #2 it ends up crashing after ~5hours and produces > > > the log in log-valgrind-2.txt.gz with some other entries than > > > case #1, but AFAICS still all about reading uninitialised values > > > in space allocated by datumCopy(). > > > > Right. This part is really interesting to me: > > > > ==48827== Uninitialised value was created by a heap allocation > > ==48827==at 0x4C28C20: malloc (vg_replace_malloc.c:296) > > ==48827==by 0x80B597: AllocSetAlloc (aset.c:771) > > ==48827==by 0x810ADC: palloc (mcxt.c:862) > > ==48827==by 0x72BFEF: datumCopy (datum.c:171) > > ==48827==by 0x81A74C: tuplesort_putdatum (tuplesort.c:1515) > > ==48827==by 0x5E91EB: advance_aggregates (nodeAgg.c:1023) > > > > If you actually go to datum.c:171, you see that that's a codepath for > > pass-by-reference datatypes that lack a varlena header. Text is a > > datatype that has a varlena header, though, so that's clearly wrong. I > > don't know how this actually happened, but working back through the > > relevant tuplesort_begin_datum() caller, initialize_aggregate(), does > > suggest some things. (tuplesort_begin_datum() is where datumTypeLen is > > determined for the entire datum tuplesort.) > > > > I am once again only guessing, but I have to wonder if this is a > > problem in commit b8d7f053. It seems likely that the problem begins > > before tuplesort_begin_datum() is even called, which is the basis of > > this suspicion. If the problem is within tuplesort, then that could > > only be because get_typlenbyval() gives wrong answers, which seems > > very unlikely. > > [Action required within three days. This is a generic notification.] > > The above-described topic is currently a PostgreSQL 10 open item. Peter > (Eisentraut), since you committed the patch believed to have created it, you > own this open item. If some other commit is more relevant or if this does not > belong as a v10 open item, please let us know. Otherwise, please observe the > policy on open item ownership[1] and send a status update within three > calendar days of this message. Include a date for your subsequent status > update. Testers may discover new open items at any time, and I want to plan > to get them all fixed well in advance of shipping v10. Consequently, I will > appreciate your efforts toward speedy resolution. Thanks. > > [1] > https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.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] Subscription code improvements
On Wed, Aug 02, 2017 at 04:09:43PM -0400, Peter Eisentraut wrote: > On 8/1/17 00:17, Noah Misch wrote: > > The above-described topic is currently a PostgreSQL 10 open item. Peter, > > since you committed the patch believed to have created it, you own this open > > item. If some other commit is more relevant or if this does not belong as a > > v10 open item, please let us know. Otherwise, please observe the policy on > > open item ownership[1] and send a status update within three calendar days > > of > > this message. Include a date for your subsequent status update. Testers > > may > > discover new open items at any time, and I want to plan to get them all > > fixed > > well in advance of shipping v10. Consequently, I will appreciate your > > efforts > > toward speedy resolution. Thanks. > > I'm looking into this now and will report by Friday. This PostgreSQL 10 open item is past due for your status update. Kindly send a status update within 24 hours, and include a date for your subsequent status update. Refer to the policy on open item ownership: https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.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] Draft release notes up for review
Andres Freundwrites: > I just pushed a 9.4 specific bugfix. Do you want me to fix up the > release notes after you backpatch the minor release to 9.4, or what's > the best process? No sweat, I'll incorporate it when I do the further-back-branch notes tomorrow. 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] modeling parallel contention (was: Parallel Append implementation)
On Thu, May 4, 2017 at 7:20 PM, David Rowleywrote: > I ended up writing the attached (which I'd not intended to post until > some time closer to when the doors open for PG11). At the moment it's > basically just a test patch to see how it affects things when we give > workers a bit more to do before they come back to look for more work. > In this case, I've just given them 10 pages to work on, instead of the > 1 that's allocated in 9.6 and v10. I think that this could benefit parallel sort, beyond the obvious fact that it too must have the contention you describe. We generally are faster at sorting presorted input for all kinds of reasons (e.g., insertion sort fallback for quicksort, merging based on replacement of heap's root item). It follows that it's to our advantage to have parallel tuplesort read multiple pages in a range into a worker at once within the parallel heap scan that feeds workers. The leader, which merges worker runs, may ultimately have to perform fewer comparisons as a result of this, which is where most of the benefit would be. -- Peter Geoghegan -- 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] Draft release notes up for review
Hi Tom, On 2017-08-04 18:41:12 -0400, Tom Lane wrote: > I've committed the first-draft release notes for 9.6.4 at > https://git.postgresql.org/pg/commitdiff/03378c4da598840b0520a53580dd7713c95f21c8 I just pushed a 9.4 specific bugfix. Do you want me to fix up the release notes after you backpatch the minor release to 9.4, or what's the best process? Andres -- 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] Draft release notes up for review
Jonathan Katzwrites: > I see this one > > Fix potential data corruption when freezing a tuple whose XMAX is a > multixact with exactly one still-interesting member > But I’m unsure how prevalent it is and if it should be highlighted. I'm not sure about that either. I do not think anyone did the legwork to determine the exact consequences of that bug, or the probability of someone hitting it in the field. But I think the latter must be really low, because we haven't heard any field reports that seem to match up. 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] Draft release notes up for review
Hi Tom, > On Aug 4, 2017, at 6:41 PM, Tom Lanewrote: > > I've committed the first-draft release notes for 9.6.4 at > https://git.postgresql.org/pg/commitdiff/03378c4da598840b0520a53580dd7713c95f21c8 > > (If you prefer to read nicely-marked-up copy, they should be up at > https://www.postgresql.org/docs/devel/static/release-9-6-4.html > in a couple hours from now.) Thank you for putting these together. For the press release, are there any features you (or anyone on -hackers) do you see any fixes you would like to highlight? I see this one > Fix potential data corruption when freezing a tuple whose XMAX is a multixact with exactly one still-interesting member But I’m unsure how prevalent it is and if it should be highlighted. Thanks, Jonathan -- 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] GSoC 2017: Foreign Key Arrays
This is the query fired upon any UPDATE/DELETE for RI checks: SELECT 1 FROM ONLY x WHERE pkatt1 = $1 [AND ...] FOR KEY SHARE OF x in the case of foreign key arrays, it's wrapped in this query: SELECT 1 WHERE (SELECT count(DISTINCT y) FROM unnest($1) y) = (SELECT count(*) FROM () z) This is where the limitation appears, the DISTINCT keyword. Since in reality, count(DISTINCT) will fall back to the default btree opclass for the array element type regardless of the opclass indicated in the access method. Thus I believe going around DISTINCT is the way to go. This is what I came up with: SELECT 1 WHERE (SELECT COUNT(*) FROM ( SELECT y FROM unnest($1) y GROUP BY y ) ) = (SELECT count(*) () z) I understand there might be some syntax errors but this is just a proof of concept. Is this the right way to go? It's been a week and I don't think I made significant progress. Any pointers? Best Regards, MarkRofail
Re: [HACKERS] pg_stop_backup(wait_for_archive := true) on standby server
On Sat, Aug 5, 2017 at 1:28 PM, Paul A Jungwirthwrote: > On Sat, Aug 5, 2017 at 7:51 AM, Robert Haas wrote: >> After refreshing my memory further, I take it back. pg_stop_backup() >> doesn't even have a second argument on v9.6, so back-porting this fix >> to 9.6 is a meaningless thing; there's nothing to fix. > > According to the docs at > https://www.postgresql.org/docs/9.6/static/functions-admin.html there > is a one-arg version in 9.6. I'm sorry, I just realized you said a *second* argument. Apologies for the distraction! Paul -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_stop_backup(wait_for_archive := true) on standby server
On Sat, Aug 5, 2017 at 7:51 AM, Robert Haaswrote: > After refreshing my memory further, I take it back. pg_stop_backup() > doesn't even have a second argument on v9.6, so back-porting this fix > to 9.6 is a meaningless thing; there's nothing to fix. According to the docs at https://www.postgresql.org/docs/9.6/static/functions-admin.html there is a one-arg version in 9.6. I've been watching this thread since I noticed it a few days ago. I have a system on 9.6 with replication, where the standby uses `archive_mode: always` so that I can run WAL-E backups from there instead of from the master. I'm a little worried about my backup integrity now (It sounds like this might be a "it usually works" situation). Also, WAL-E currently uses the no-arg pg_stop_backup, but I was contemplating offering a patch to use non-exclusive backups when available, both to avoid stopping the standby and also to generate a tablespace description (which WAL-E requires when restoring with tablespaces). Just thought I'd offer a real use-case for the non-exclusive-mode functions on 9.6. I don't have an opinion on the urgency of back-porting a fix, but if pg_stop_backup(boolean) allows for inconsistent backups, it does sound like a problem on 9.6 too. Paul -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: LP_DEAD hinting and not holding on to a buffer pin on leaf page (Was: [HACKERS] [WIP] Zipfian distribution in pgbench)
On Fri, Aug 4, 2017 at 3:30 PM, Peter Geogheganwrote: > Yura Sokolov of Postgres Pro performed this benchmark at my request. > He took the 9.5 commit immediately proceeding 2ed5b87f9 as a baseline. I attach a simple patch that comments out the release of the buffer pin for logged tables where an MVCC snapshot is used for a scan that is not an index-only scan. This is the simplest way of evaluating the effects of disabling 2ed5b87f9. Yura or others may find this helpful. To be clear, I am certainly not suggesting that we should revert 2ed5b87f9. I do think that we need to give serious thought to fixing the regression that 2ed5b87f9 introduced for LP_DEAD setting by index scans, though. -- Peter Geoghegan diff --git a/src/backend/access/nbtree/nbtsearch.c b/src/backend/access/nbtree/nbtsearch.c new file mode 100644 index 642c894..696beda *** a/src/backend/access/nbtree/nbtsearch.c --- b/src/backend/access/nbtree/nbtsearch.c *** _bt_drop_lock_and_maybe_pin(IndexScanDes *** 58,63 --- 58,64 { LockBuffer(sp->buf, BUFFER_LOCK_UNLOCK); + #if 0 if (IsMVCCSnapshot(scan->xs_snapshot) && RelationNeedsWAL(scan->indexRelation) && !scan->xs_want_itup) *** _bt_drop_lock_and_maybe_pin(IndexScanDes *** 65,70 --- 66,72 ReleaseBuffer(sp->buf); sp->buf = InvalidBuffer; } + #endif } -- 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] git.postgresql.org (and other services) down
On 08/05/2017 08:36 AM, Joe Conway wrote: > One of the datacenters that provides two pg.org VM hosts has become > inaccessible. Services that may be affected are: > > git.postgresql.org > search.postgresql.org > Mailinglist archives backend > PostgreSQL Community Association of Canada > Download stats analytics database server > ftpmaster.postgresql.org > ns1.postgresql.org > website backend server > docbot.postgresql.org > > Some of these services have redundant servers, but some, notably > git.postgresql.org do not. > > I have contacted the provider and will keep the list posted with any > significant status updates. Update: We have not heard back from the provider yet, but access seem to have been restored and relatively stable at the moment, although there is still some packet loss. Joe -- Crunchy Data - http://crunchydata.com PostgreSQL Support for Secure Enterprises Consulting, Training, & Open Source Development signature.asc Description: OpenPGP digital signature
Re: [HACKERS] pg_stop_backup(wait_for_archive := true) on standby server
On Sat, Aug 5, 2017 at 12:07 PM, Michael Paquierwrote: > Because default values should be safe in the backup and restore area, > and wait_for_archive = false is not the default. Neither is archive_mode = always, without which wait_for_archive = true doesn't actually wait. > I would like to point > out that the 9.6 behavior has been discussed as being a bug upthread > for 9.6 by three people (David, Sawada-san and I) as there is a real > risk to take inconsistent backups from standbys (a WAL segment may not > be archived when pg_stop_backup reports back, so the user may not have > all the WAL it would need to get back to a consistent state), and that > the default should be to get consistent and safe backups. Sure, but that only happens if you haven't archived the very next WAL segment yet, which in many environments isn't going to be a problem. Furthermore, there's also the opposite danger of somebody's backup script hanging where it currently doesn't. I think it's just wrong to say that without this change you don't get consistent backups. It just means you have an additional step to do to make sure that you have all the WAL files you need - which is also true *with* the patch, because you still have to make sure a WAL rotation happens on the master. Besides, if not waiting is so bad, then what about the fact that 9.5, 9.4, 9.3, and 9.2 have the exact same code to not wait, and you're not proposing to change that? What about the fact waiting isn't even well-defined unless standbys support archiving? > Backup history files are around mainly for debugging purposes. While I > don't mind about the choice to not generate them on back-branches, the > inconsistency between primary and standbys in generating them is > really disturbing. We could have taken the occasion to address that > here as this is not invasive, but well... I do complain a lot about > keeping changes going to v10 non-invasive if possible. So no real > complain to do what's been done. I'm sorry you're disturbed, but I think that's clearly not a separate change and not a bug fix. The current behavior is well-documented, including in places your patch didn't change, like the pg_basebackup documentation. >> I think the right thing to do about 9.6 is >> document the behavior; there's no problem here that a user can't work >> around by doing it right. > > There are many ways for users to do it wrong in this area, that I am > of the opinion to give them safe defaults if we have a way to make > things work safe in the backend. And here we are talking about extra > checks to make sure that a WAL segment is correctly archived... I have > seen bugs lately in custom backup code which led to inconsistent > backups. I agree that there are many ways for users to do it wrong, and I do not think that changing critical behavior in minor releases will result in a reduction in the number of ways for users to do it wrong. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_stop_backup(wait_for_archive := true) on standby server
On Sat, Aug 5, 2017 at 4:51 PM, Robert Haaswrote: > On Sat, Aug 5, 2017 at 9:11 AM, Robert Haas wrote: >> If we apply these patches to 9.6, then pg_stop_backup() on a standby >> will start writing backup history files and removing no-longer-needed >> backup history files. That's a clear behavior change, and it isn't a >> bug fix. Making the waitforarchive option work is a bug fix, but I'm >> not convinced we should take it as a license to change other aspects >> of the behavior in a point release. > > After refreshing my memory further, I take it back. pg_stop_backup() > doesn't even have a second argument on v9.6, so back-porting this fix > to 9.6 is a meaningless thing; there's nothing to fix. What the > patches propose to do there instead is adopt the behavior proposed for > v10 when pg_stop_backup()'s second argument is true as the > unconditional behavior in v9.6. For that to be the right thing to do, > we have to decide that the current v9.6 behavior is a bug. But I > (still) think that's very arguable, because the whole point is that > we've just finished adding a flag in v10 to disable on the master the > *very same behavior* we're proposing to mandate on the standby in > v9.6. How can we say that v9.6's current behavior is a bug when v10 > produces the same behavior with wait_for_archive = false? That just > doesn't make any sense. Because default values should be safe in the backup and restore area, and wait_for_archive = false is not the default. I would like to point out that the 9.6 behavior has been discussed as being a bug upthread for 9.6 by three people (David, Sawada-san and I) as there is a real risk to take inconsistent backups from standbys (a WAL segment may not be archived when pg_stop_backup reports back, so the user may not have all the WAL it would need to get back to a consistent state), and that the default should be to get consistent and safe backups. > I've pushed a minimal fix for v10 which should address Sawada-san's > original complaint: now, if you say wait_for_archive = true on a > standby, you'll get a warning if it didn't wait, or if archive_mode = > always, it will wait. Backup history files are around mainly for debugging purposes. While I don't mind about the choice to not generate them on back-branches, the inconsistency between primary and standbys in generating them is really disturbing. We could have taken the occasion to address that here as this is not invasive, but well... I do complain a lot about keeping changes going to v10 non-invasive if possible. So no real complain to do what's been done. > I think the right thing to do about 9.6 is > document the behavior; there's no problem here that a user can't work > around by doing it right. There are many ways for users to do it wrong in this area, that I am of the opinion to give them safe defaults if we have a way to make things work safe in the backend. And here we are talking about extra checks to make sure that a WAL segment is correctly archived... I have seen bugs lately in custom backup code which led to inconsistent backups. -- 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] git.postgresql.org (and other services) down
One of the datacenters that provides two pg.org VM hosts has become inaccessible. Services that may be affected are: git.postgresql.org search.postgresql.org Mailinglist archives backend PostgreSQL Community Association of Canada Download stats analytics database server ftpmaster.postgresql.org ns1.postgresql.org website backend server docbot.postgresql.org Some of these services have redundant servers, but some, notably git.postgresql.org do not. I have contacted the provider and will keep the list posted with any significant status updates. Joe -- Crunchy Data - http://crunchydata.com PostgreSQL Support for Secure Enterprises Consulting, Training, & Open Source Development signature.asc Description: OpenPGP digital signature
Re: [HACKERS] foreign table creation and NOT VALID check constraints
On Thu, Aug 3, 2017 at 9:29 PM, Amit Langotewrote: > Thanks for committing the code changes. > > About the documentation changes, it seems that the only places where any > description of NOT VALID appears is ALTER TABLE, ALTER FOREIGN TABLE, and > ALTER DOMAIN references pages. Even if the CREATE (FOREIGN) TABLE syntax > allows it, NOT VALID does not appear in the syntax synopsis, so it seems > kind of a hidden feature. Maybe, we should fix that first (if at all). Yeah. If somebody wants to do a general survey of how NOT VALID is documented and improve that, cool. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_stop_backup(wait_for_archive := true) on standby server
On Sat, Aug 5, 2017 at 9:11 AM, Robert Haaswrote: > If we apply these patches to 9.6, then pg_stop_backup() on a standby > will start writing backup history files and removing no-longer-needed > backup history files. That's a clear behavior change, and it isn't a > bug fix. Making the waitforarchive option work is a bug fix, but I'm > not convinced we should take it as a license to change other aspects > of the behavior in a point release. After refreshing my memory further, I take it back. pg_stop_backup() doesn't even have a second argument on v9.6, so back-porting this fix to 9.6 is a meaningless thing; there's nothing to fix. What the patches propose to do there instead is adopt the behavior proposed for v10 when pg_stop_backup()'s second argument is true as the unconditional behavior in v9.6. For that to be the right thing to do, we have to decide that the current v9.6 behavior is a bug. But I (still) think that's very arguable, because the whole point is that we've just finished adding a flag in v10 to disable on the master the *very same behavior* we're proposing to mandate on the standby in v9.6. How can we say that v9.6's current behavior is a bug when v10 produces the same behavior with wait_for_archive = false? That just doesn't make any sense. I've pushed a minimal fix for v10 which should address Sawada-san's original complaint: now, if you say wait_for_archive = true on a standby, you'll get a warning if it didn't wait, or if archive_mode = always, it will wait. I think the right thing to do about 9.6 is document the behavior; there's no problem here that a user can't work around by doing it right. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_stop_backup(wait_for_archive := true) on standby server
Robert Haaswrites: > On Sat, Aug 5, 2017 at 4:14 AM, Michael Paquier > wrote: >> If no other committer wants to take a shot at those patches, it may be >> better to push them after the next minor release happens? I don't like >> delaying bug fixes, but the release is close by and time flies. > /me reads patches. > [ assorted objections ] Given Robert's objections, there's no way we should force these into Monday's releases. Safer to spend whatever time is needed to get it 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] PostgreSQL 10 (latest beta) and older ICU
On 8/1/17 11:28, Peter Eisentraut wrote: > On 8/1/17 08:28, Victor Wagner wrote: >> On Tue, 1 Aug 2017 08:16:54 -0400 >> Peter Eisentrautwrote: >> >>> On 8/1/17 02:12, Victor Wagner wrote: > We are only calling uloc_toLanguageTag() with keyword/value > combinations that ICU itself previously told us were supported. So > just ignoring errors doesn't seem proper in this case. > We know that this version of ICU is broken. But what choice we have? >>> I don't know that we can already reach that conclusion. Maybe the >> Because it was fixed in subsequent versions. > > I propose the attached patch to address this. committed -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_stop_backup(wait_for_archive := true) on standby server
On Sat, Aug 5, 2017 at 4:14 AM, Michael Paquierwrote: > On Sat, Aug 5, 2017 at 5:27 AM, Stephen Frost wrote: >> Unfortunately the day got away from me due to some personal... adventures >> (having to do with lack of air conditioning first and then lack of gas, >> amongst a lot of other things going on right now...). I just got things back >> online but, well, my day tomorrow is pretty well packed solid. I wouldn't >> complain if someone has a few cycles to push these, they look pretty good to >> me but I won't be able to work on PG stuff again until tomorrow night at the >> earliest. > > If no other committer wants to take a shot at those patches, it may be > better to push them after the next minor release happens? I don't like > delaying bug fixes, but the release is close by and time flies. /me reads patches. If we apply these patches to 9.6, then pg_stop_backup() on a standby will start writing backup history files and removing no-longer-needed backup history files. That's a clear behavior change, and it isn't a bug fix. Making the waitforarchive option work is a bug fix, but I'm not convinced we should take it as a license to change other aspects of the behavior in a point release. -errhint("WAL control functions cannot be executed during recovery."))); +errhint("WAL control functions cannot be executed during recovery; " +"they should be executed on the primary instead"))); I don't agree with this change at all. Executing WAL control functions on the master cannot reasonably be considered equivalent to doing it on the standby. This is like saying "don't take candy from a baby, take it from an adult instead". That could be good advice under the right set of circumstances, but it's also subject to unfortunate misinterpretation. -- 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] pgsql 10: hash indexes testing
On Sat, Aug 5, 2017 at 7:50 AM, Robert Haaswrote: > On Fri, Aug 4, 2017 at 2:45 PM, Amit Kapila wrote: >> I have not done anything for this comment as it doesn't sound wrong to >> me. I think it is not making much sense in the current code and we >> can remove it or change it as part of the separate patch if you or >> others think so. > > I don't get it. The comment claims we have to _hash_getnewbuf before > releasing the metapage write lock. But the function neither calls > _hash_getnewbuf nor releases the metapage write lock. Both the actions _hash_getnewbuf and release of the metapage lock are done in the caller (_hash_expandtable). > It then goes on > to say that for this reason we pass the new buffer rather than just > the block number. However, even if it were true that we have to call > _hash_getnewbuf before releasing the metapage write lock, what does > that have to do with the choice of passing a buffer vs. a block > number? > For this, you have to look at PG9.6 code. In PG9.6, we were passing old bucket's block number and new bucket's buffer and the reason is that in the caller (_hash_expandtable) we only have access to a buffer of new bucket block. > Explain to me what I'm missing, please, because I'm completely befuddled! > I think you need to compare the code of PG10 with PG9.6 for functions _hash_splitbucket and _hash_expandtable. I don't find this comment much useful starting PG10. Patch attached to remove it. > (On another note, I committed these patches.) > Thanks. -- With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com remove_unnecessary_comment_hash_v1.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] Row Level Security Documentation
Hello Rod, Patch applies cleanly, make html ok, new table looks good to me. I've turned it "Ready for Committer". Thanks! -- Fabien. -- 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] [WIP] Zipfian distribution in pgbench
Hello Alik, So I would be in favor of expanding the documentation but not restricting the parameter beyond avoiding value 1.0. I have removed restriction and expanded documentation in attaching patch v5. I've done some math investigations, which consisted in spending one hour with Christian, a statistician colleague of mine. He took an old book out of a shelf, opened it to page 550 (roughly in the middle), and explained to me how to build a real zipfian distribution random generator. The iterative method is for parameter a>1 and works for unbounded values. It is simple to add a bound. In practice the iterative method is quite effective, i.e. number of iterations is typically small, at least if the bound is large and if parameter a is not too close to 1. I've attached a python3 script which implements the algorithm. It looks like magic. Beware that a C implementation should take care of float and int overflows. # usage: a, #values, #tests sh> zipf.py 1.5 1000 100 # after 1.7 seconds c = [391586, 138668, 75525, 49339, 35222, 26621, ... ... 11, 13, 12, 11, 16] (1.338591 iterations per draw) sh> zipf.py 1.1 1000 100 # after 3.1 seconds c = [179302, 83927, 53104, 39015, 30557, 25164, ... ... 82, 95, 93, 81, 80] (2.681451 iterations per draw) I think that this method should be used for a>1, and the other very rough one can be kept for parameter a in [0, 1), a case which does not make much sense to a mathematician as it diverges if unbounded. -- Fabien.#! /usr/bin/env python3 # # generate Zipf distribution # # method taken from: # Luc Devroye, # "Non-Uniform Random Variate Generation" # p. 550-551. # Springer 1986 # # the method works for an infinite bound, the finite bound condition has been # added. a = 1.1 N = 100 M = 1 import sys if len(sys.argv) >= 3: a = float(sys.argv[1]) N = int(sys.argv[2]) if len(sys.argv) >= 4: M = int(sys.argv[3]) # beware, a close to 1 and n small (eg 100) leads to large number of iterations # i.e. rejection probability is high when a -> 1 # - 1.001: 280 # - 1.002: 139.2 # - 1.005: 55.9 # - 1.010: 28.4 # - 1.020: 14.8 # - 1.050: 6.2 # - 1.100: 3.5 # however if n is larger the number of iterations decreases significantly from random import random from math import exp def zipfgen(a, N): assert a > 1.0, "a must be greater than 1" b = 2.0 ** (a - 1.0) i = 0 # count iterations while True: i += 1 u, v = random(), random() try: x = int(u ** (- 1.0 / (a - 1.0))) t = (1.0 + 1.0 / x) ** (a - 1.0) # reject if too large or out of bound if v * x * (t - 1.0) / (b - 1.0) <= t / b and x <= N: break except OverflowError: # on u ** ... pass return (x, i) if M == 1: x, i = zipfgen(a, N) print("X = %d (%d)" % (x, i)) else: c = [0 for i in range(0, N)] cost = 0 for i in range(0, M): x, i = zipfgen(a, N) # assert 1 <= x and x <= N, "x = %d" % x cost += i c[x-1] += 1 print("c = %s (%f iterations per draw)" % (c, cost/M)) -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_stop_backup(wait_for_archive := true) on standby server
On Sat, Aug 5, 2017 at 5:27 AM, Stephen Frostwrote: > Unfortunately the day got away from me due to some personal... adventures > (having to do with lack of air conditioning first and then lack of gas, > amongst a lot of other things going on right now...). I just got things back > online but, well, my day tomorrow is pretty well packed solid. I wouldn't > complain if someone has a few cycles to push these, they look pretty good to > me but I won't be able to work on PG stuff again until tomorrow night at the > earliest. If no other committer wants to take a shot at those patches, it may be better to push them after the next minor release happens? I don't like delaying bug fixes, but the release is close by and time flies. -- Michael -- 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] [WIP] Zipfian distribution in pgbench
Hello Peter, I think that it would also be nice if there was an option to make functions like random_zipfian() actually return a value that has undergone perfect hashing. When this option is used, any given value that the function returns would actually be taken from a random mapping to some other value in the same range. So, you can potentially get a Zipfian distribution without the locality. I definitely agree. This is a standard problem with all non uniform random generators in pgbench, namely random_{gaussian,exponential}. However hashing is not a good solution on a finite domain because of the significant collision rate, so that typically 1/3 of values are empty and collisions cause spikes. Also, collisions would break PKs. The solution is to provide a (good) pseudo-random parametric permutation, which is non trivial especially for non powers of two, so ISTM that it should be a patch on its own. The good news is that it is on my todo list and I have some ideas on how to do it. The bad news is that given the rate at which I succeed in getting things committed in pgbench, it might take some years:-( For instance, simplistic functions and operators to extend the current set have been in the pipe for 15 months and missed pg10. -- Fabien. -- 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] PostgreSQL not setting OpenSSL session id context?
Awesome, thanks!! On Fri, Aug 4, 2017 at 11:54 PM, Tom Lanewrote: > Shay Rojansky writes: > > Great. Do you think it's possible to backport to the other maintained > > branches as well, seeing as how this is quite trivial and low-impact? > > Already done, will be in next week's minor releases. (You timed this > bug report well.) > > regards, tom lane >