Re: [HACKERS] modeling parallel contention (was: Parallel Append implementation)

2017-08-05 Thread Robert Haas
On Sat, Aug 5, 2017 at 6:17 PM, Peter Geoghegan  wrote:
> 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

2017-08-05 Thread Robert Haas
On Sat, Aug 5, 2017 at 4:28 PM, Paul A Jungwirth
 wrote:
> 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

2017-08-05 Thread Noah Misch
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

2017-08-05 Thread Noah Misch
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

2017-08-05 Thread Tom Lane
Andres Freund  writes:
> 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)

2017-08-05 Thread Peter Geoghegan
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.

-- 
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

2017-08-05 Thread Andres Freund
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

2017-08-05 Thread Tom Lane
Jonathan Katz  writes:
> 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

2017-08-05 Thread Jonathan Katz
Hi Tom,

> On Aug 4, 2017, at 6:41 PM, Tom Lane  wrote:
> 
> 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

2017-08-05 Thread Mark Rofail
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

2017-08-05 Thread Paul A Jungwirth
On Sat, Aug 5, 2017 at 1:28 PM, Paul A Jungwirth
 wrote:
> 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

2017-08-05 Thread Paul A Jungwirth
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'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)

2017-08-05 Thread Peter Geoghegan
On Fri, Aug 4, 2017 at 3:30 PM, Peter Geoghegan  wrote:
> 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

2017-08-05 Thread Joe Conway
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

2017-08-05 Thread Robert Haas
On Sat, Aug 5, 2017 at 12:07 PM, Michael Paquier
 wrote:
> 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

2017-08-05 Thread Michael Paquier
On Sat, Aug 5, 2017 at 4:51 PM, Robert Haas  wrote:
> 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

2017-08-05 Thread Joe Conway
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

2017-08-05 Thread Robert Haas
On Thu, Aug 3, 2017 at 9:29 PM, Amit Langote
 wrote:
> 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

2017-08-05 Thread Robert Haas
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.

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

2017-08-05 Thread Tom Lane
Robert Haas  writes:
> 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

2017-08-05 Thread Peter Eisentraut
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 Eisentraut  wrote:
>>
>>> 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

2017-08-05 Thread Robert Haas
On Sat, Aug 5, 2017 at 4:14 AM, Michael Paquier
 wrote:
> 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

2017-08-05 Thread Amit Kapila
On Sat, Aug 5, 2017 at 7:50 AM, Robert Haas  wrote:
> 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

2017-08-05 Thread Fabien COELHO


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

2017-08-05 Thread Fabien COELHO


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

2017-08-05 Thread Michael Paquier
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.
-- 
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

2017-08-05 Thread Fabien COELHO


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?

2017-08-05 Thread Shay Rojansky
Awesome, thanks!!

On Fri, Aug 4, 2017 at 11:54 PM, Tom Lane  wrote:

> 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
>