Re: [HACKERS] Logical Replication WIP

[Erik Rijkers]

On 2016-12-18 11:12, Petr Jelinek wrote:

(now using latest: patchset:)

0001-Add-PUBLICATION-catalogs-and-DDL-v14.patch
0002-Add-SUBSCRIPTION-catalog-and-DDL-v14.patch
0003-Define-logical-replication-protocol-and-output-plugi-v14.patch
0004-Add-logical-replication-workers-v14.patch
0005-Add-separate-synchronous-commit-control-for-logical--v14.patch


BTW you don't need to add primary key to pgbench_history. Simply ALTER
TABLE pgbench_history REPLICA IDENTITY FULL; should be enough.


Either should, but neither is.

set-up:
Before creating the publication/subscription:
On master I run   pgbench -qis 1,  then set replica identity (and/or add 
serial column) for pgbench_history, then dump/restore the 4 pgbench 
tables from master to replica.
Then enabling publication/subscription.  logs looks well.  (Other tests  
I've devised earlier (on other tables) still work nicely.)


Now when I do a pgbench-run on master, something like:

   pgbench -c 1 -T 20 -P 1

I often see this (when running pgbench):

ERROR:  publisher does not send replica identity column expected by the 
logical replication target public.pgbench_tellers
or, sometimes  (less often) the same ERROR for pgbench_accounts appears 
(as in the subsciber-log below)


-- publisher log
2016-12-19 07:44:22.738 CET 22690 LOG:  logical decoding found 
consistent point at 0/14598C78
2016-12-19 07:44:22.738 CET 22690 DETAIL:  There are no running 
transactions.
2016-12-19 07:44:22.738 CET 22690 LOG:  exported logical decoding 
snapshot: "000130FA-1" with 0 transaction IDs
2016-12-19 07:44:22.886 CET 22729 LOG:  starting logical decoding for 
slot "sub1"
2016-12-19 07:44:22.886 CET 22729 DETAIL:  streaming transactions 
committing after 0/14598CB0, reading WAL from 0/14598C78
2016-12-19 07:44:22.886 CET 22729 LOG:  logical decoding found 
consistent point at 0/14598C78
2016-12-19 07:44:22.886 CET 22729 DETAIL:  There are no running 
transactions.
2016-12-19 07:45:25.568 CET 22729 LOG:  could not receive data from 
client: Connection reset by peer
2016-12-19 07:45:25.568 CET 22729 LOG:  unexpected EOF on standby 
connection
2016-12-19 07:45:25.580 CET 26696 LOG:  starting logical decoding for 
slot "sub1"
2016-12-19 07:45:25.580 CET 26696 DETAIL:  streaming transactions 
committing after 0/1468E0D0, reading WAL from 0/1468DC90
2016-12-19 07:45:25.589 CET 26696 LOG:  logical decoding found 
consistent point at 0/1468DC90
2016-12-19 07:45:25.589 CET 26696 DETAIL:  There are no running 
transactions.


-- subscriber log
2016-12-19 07:44:22.878 CET 17027 LOG:  starting logical replication 
worker for subscription 24581
2016-12-19 07:44:22.883 CET 22726 LOG:  logical replication apply for 
subscription sub1 started
2016-12-19 07:45:11.069 CET 22726 WARNING:  leaked hash_seq_search scan 
for hash table 0x2def1a8
2016-12-19 07:45:25.566 CET 22726 ERROR:  publisher does not send 
replica identity column expected by the logical replication target 
public.pgbench_accounts
2016-12-19 07:45:25.568 CET 16984 LOG:  worker process: logical 
replication worker 24581 (PID 22726) exited with exit code 1
2016-12-19 07:45:25.568 CET 17027 LOG:  starting logical replication 
worker for subscription 24581
2016-12-19 07:45:25.574 CET 26695 LOG:  logical replication apply for 
subscription sub1 started
2016-12-19 07:46:10.950 CET 26695 WARNING:  leaked hash_seq_search scan 
for hash table 0x2def2c8
2016-12-19 07:46:10.950 CET 26695 WARNING:  leaked hash_seq_search scan 
for hash table 0x2def2c8
2016-12-19 07:46:10.950 CET 26695 WARNING:  leaked hash_seq_search scan 
for hash table 0x2def2c8



Sometimes  replication (caused by a pgbench run)  runs for a few seconds 
replicating all 4 pgbench tables correctly, but never longer than 10 to 
20 seconds.


If you cannot reproduce with the provided info it  I will make a more 
precise setup-desciption, but it's so solidly failing here that I hope 
that won't be necessary.



Erik Rijkers






--
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] Declarative partitioning vs. sql_inheritance

[Amit Langote]

On 2016/12/17 10:40, Robert Haas wrote:
> On Fri, Dec 16, 2016 at 7:39 PM, Tom Lane  wrote:
>> Peter Eisentraut  writes:
>>> On 12/16/16 11:05 AM, Robert Haas wrote:
 If we were going to do anything about this,
 my vote would be to remove sql_inheritance.
>>
>>> Go for it.
>>
>>> Let's also remove the table* syntax then.
>>
>> Meh --- that might break existing queries, to what purpose?
>>
>> We certainly shouldn't remove query syntax without a deprecation period.
>> I'm less concerned about that for GUCs.
> 
> I agree.  Patch attached, just removing the GUC and a fairly minimal
> amount of the supporting infrastructure.

+1 to removing the sql_inheritance GUC.  The patch looks good to me.

Thanks,
Amit




-- 
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] postgres_fdw bug in 9.6

[Ashutosh Bapat]

On Fri, Dec 16, 2016 at 9:43 PM, Tom Lane  wrote:
> Etsuro Fujita  writes:
>> On 2016/12/16 11:25, Etsuro Fujita wrote:
>>> As I said upthread, an alternative I am thinking is (1) to create an
>>> equivalent nestloop join path using inner/outer paths of a foreign join
>>> path, except when that join path implements a full join, in which case a
>>> merge/hash join path is used, (2) store it in fdw_outerpath as-is, and
>>> (3) during an EPQ recheck, apply postgresRecheckForeignScan to the outer
>>> subplan created from the fdw_outerpath as-is.  What do you think about
>>> that?
>
>> Let me explain about #1 and #2 a bit more.  What I have in mind is:
>
>> * modify postgresGetForeignPaths so that a simple foreign table scan
>> path is stored into the base relation's PgFdwRelationInfo.
>> * modify postgresGetForeignJoinPaths so that
>>  (a) a local join path for a 2-way foreign join is created using
>> simple foreign table scan paths stored in the base relations'
>> PgFdwRelationInfos, and stored into the join relation's PgFdwRelationInfo.
>>  (b) a local join path for a 3-way foreign join, whose left input is
>> a 2-way foreign join, is created using a local join path stored in the
>> left input join relation's PgFdwRelationInfo and a simple foreign table
>> scan path stored into the right input base relation's PgFdwRelationInfo.
>>  (c) Likewise for higher level foreign joins.
>>  (d) local join paths created are passed to create_foreignscan_path
>> and stored into the fdw_outerpaths of the resulting ForeignPahts.
>
> Hm, isn't this overcomplicated?  As you said earlier, we don't really
> care all that much whether the fdw_outerpath is free of lower foreign
> joins, because EPQ setup will select those lower join's substitute EPQ
> plans anyway.  All that we need is that the EPQ tree be a legal
> implementation of the join order with join quals applied at the right
> places.  So I think the rule could be
>
> "When first asked to produce a path for a given foreign joinrel, collect
> the cheapest paths for its left and right inputs, and make a nestloop path
> (or hashjoin path, if full join) from those, using the join quals needed
> for the current input relation pair.  Use this as the fdw_outerpath for
> all foreign paths made for the joinrel."

We could use fdw_outerpath of the left and right inputs instead of
looking for the cheapest paths. For base relations as left or right
relations, use the unparameterized cheapest foreign path. This will
ensure that all joins in fdw_outerpath are local joins, making EPQ
checks slightly efficient by avoiding redirection at every foreign
path.

-- 
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database 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] postgres_fdw bug in 9.6

[Etsuro Fujita]

On 2016/12/17 1:13, Tom Lane wrote:

Etsuro Fujita  writes:

On 2016/12/16 11:25, Etsuro Fujita wrote:

As I said upthread, an alternative I am thinking is (1) to create an
equivalent nestloop join path using inner/outer paths of a foreign join
path, except when that join path implements a full join, in which case a
merge/hash join path is used, (2) store it in fdw_outerpath as-is, and
(3) during an EPQ recheck, apply postgresRecheckForeignScan to the outer
subplan created from the fdw_outerpath as-is.  What do you think about
that?



Let me explain about #1 and #2 a bit more.  What I have in mind is:



* modify postgresGetForeignPaths so that a simple foreign table scan
path is stored into the base relation's PgFdwRelationInfo.
* modify postgresGetForeignJoinPaths so that
 (a) a local join path for a 2-way foreign join is created using
simple foreign table scan paths stored in the base relations'
PgFdwRelationInfos, and stored into the join relation's PgFdwRelationInfo.
 (b) a local join path for a 3-way foreign join, whose left input is
a 2-way foreign join, is created using a local join path stored in the
left input join relation's PgFdwRelationInfo and a simple foreign table
scan path stored into the right input base relation's PgFdwRelationInfo.
 (c) Likewise for higher level foreign joins.
 (d) local join paths created are passed to create_foreignscan_path
and stored into the fdw_outerpaths of the resulting ForeignPahts.



Hm, isn't this overcomplicated?  As you said earlier, we don't really
care all that much whether the fdw_outerpath is free of lower foreign
joins, because EPQ setup will select those lower join's substitute EPQ
plans anyway.  All that we need is that the EPQ tree be a legal
implementation of the join order with join quals applied at the right
places.


Exactly.  I thought the EPQ trees without lower foreign joins would be 
better because that would be easier to understand.



So I think the rule could be



"When first asked to produce a path for a given foreign joinrel, collect
the cheapest paths for its left and right inputs, and make a nestloop path
(or hashjoin path, if full join) from those, using the join quals needed
for the current input relation pair.


Seems reasonable.


Use this as the fdw_outerpath for
all foreign paths made for the joinrel."


I'm not sure that would work well for foreign joins with sort orders. 
Consider a merge join, whose left input is a 2-way foreign join with a 
sort order that implements a full join and whose right input is a sorted 
local table scan.  If the EPQ subplan for the foreign join wouldn't 
produce the right sort order, the merge join might break during EPQ 
rechecks (note that in this case the EPQ subplan for the foreign join 
might produce more than a single row during an EPQ recheck).  So, I 
think we would need to add an explicit sort to the fdw_outerpath for the 
foreign join.



The important point here is that we avoid using a merge join because that
has assumptions about input ordering that likely won't be satisfied by
the child paths chosen through this method.  (I guess you could fall back
to it for the case of no quals in a fulljoin, because then the ordering
assumptions are vacuous anyway.)


I agree on that point.  I'll create a patch.

Best regards,
Etsuro Fujita




--
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] Creating a DSA area to provide work space for parallel execution

[Thomas Munro]

On Sat, Dec 17, 2016 at 5:41 AM, Robert Haas  wrote:
> On Wed, Dec 14, 2016 at 3:25 PM, Robert Haas  wrote:
>> Thoughts?
>
> Hearing no objections, I've gone ahead and committed this.  If that
> makes somebody really unhappy I can revert it, but I am betting that
> the real story is that nobody cares about preserving T_ID().

I suppose LWLock could include a uint16 member 'id' without making
LWLock any larger, since it would replace the padding between
'tranche' and 'state'.  But I think a better solution, if anyone
really wants these T_ID numbers from a dtrace script, would be to add
lock address to the existing lwlock probes, and then figure out a way
to add some new probes to report the base and stride in the right
places so you can do the book keeping in dtrace variables.

-- 
Thomas Munro
http://www.enterprisedb.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] Retire src/backend/port/dynloader/linux.c ?

[Peter Eisentraut]

On 12/18/16 10:19 PM, Tom Lane wrote:
> Andres Freund  writes:
>> Shouldn't we just remove that code?
> 
> What for?  It's maintenance-free ... hasn't been touched since 2004.
> While I agree with you that it's *probably* dead code, it's hard to
> see much upside from removing it.
> 
> If we want to get into arguing whether code is dead or not, there's
> an awful lot of potentially removable stuff in the tree, but I doubt
> it's worth the trouble to figure out what's really dead.

If someone wants to dive into it, I think you could probably remove most
or all of the prehistoric pre-dlopen code for *bsd and darwin as well.

The hpux and win32 code could be moved to libpgport, and then we could
just call dlopen() etc. directly and remove this whole subdirectory.

-- 
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] Retire src/backend/port/dynloader/linux.c ?

[Tom Lane]

Andres Freund  writes:
> Shouldn't we just remove that code?

What for?  It's maintenance-free ... hasn't been touched since 2004.
While I agree with you that it's *probably* dead code, it's hard to
see much upside from removing it.

If we want to get into arguing whether code is dead or not, there's
an awful lot of potentially removable stuff in the tree, but I doubt
it's worth the trouble to figure out what's really dead.

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] Measuring replay lag

[Peter Eisentraut]

On 11/22/16 4:27 AM, Thomas Munro wrote:
> Thanks very much for testing!  New version attached.  I will add this
> to the next CF.

I don't see it there yet.

-- 
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] Declarative partitioning - another take

[Amit Langote]

On 2016/12/17 11:32, Amit Langote wrote:
> On Sat, Dec 17, 2016 at 1:07 AM, Robert Haas  wrote:
>> On Fri, Dec 16, 2016 at 3:02 AM, Amit Langote
>>  wrote:
>>> Aside from the above, I found few other issues and fixed them in the
>>> attached patches.  Descriptions follow:
>>
>> To avoid any further mistakes on my part, can you please resubmit
>> these with each patch file containing a proposed commit message
>> including patch authorship information, who reported the issue, links
>> to relevant discussion if any, and any other attribution information
>> which I should not fail to include when committing?
> 
> I think it's a good advice and will keep in mind for any patches I
> post henceforth.
> 
> In this particular case, I found all the issues myself while working
> with some more esoteric test scenarios, except the first patch (1/7),
> where I have mentioned in the description of the patch in the email,
> that there were independent reports of the issue by Tomas Vondra and
> David Fetter.

Here are updated patches including the additional information.

Thanks,
Amit
>From c2afac3447a8bd2f7f9004e8b7e258039ca2296b Mon Sep 17 00:00:00 2001
From: amit 
Date: Tue, 13 Dec 2016 15:07:06 +0900
Subject: [PATCH 1/7] Invalidate the parent's relcache after partition
 creation.

CREATE TABLE PARTITION OF failed to invalidate the parent table's
relcache, causing the subsequent commands in the same transaction to
not see the new partition.

Reported by: Tomas Vondra and David Fetter
Patch by: Amit Langote
Reports: https://www.postgresql.org/message-id/22dd313b-d7fd-22b5-0787-654845c8f849%402ndquadrant.com
 https://www.postgresql.org/message-id/20161215090916.GB20659%40fetter.org
---
 src/backend/catalog/heap.c   |  7 ++-
 src/backend/commands/tablecmds.c | 13 -
 src/include/catalog/heap.h   |  2 +-
 3 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c
index c09c9f28a7..e5d6aecc3f 100644
--- a/src/backend/catalog/heap.c
+++ b/src/backend/catalog/heap.c
@@ -3230,9 +3230,12 @@ RemovePartitionKeyByRelId(Oid relid)
  * StorePartitionBound
  *		Update pg_class tuple of rel to store the partition bound and set
  *		relispartition to true
+ *
+ * Also, invalidate the parent's relcache, so that the next rebuild will load
+ * the new partition's info into its partition descriptor.
  */
 void
-StorePartitionBound(Relation rel, Node *bound)
+StorePartitionBound(Relation rel, Relation parent, Node *bound)
 {
 	Relation	classRel;
 	HeapTuple	tuple,
@@ -3273,4 +3276,6 @@ StorePartitionBound(Relation rel, Node *bound)
 	CatalogUpdateIndexes(classRel, newtuple);
 	heap_freetuple(newtuple);
 	heap_close(classRel, RowExclusiveLock);
+
+	CacheInvalidateRelcache(parent);
 }
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 7a574dc50d..1c219b03dd 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -777,10 +777,11 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId,
 		 * it does not return on error.
 		 */
 		check_new_partition_bound(relname, parent, bound);
-		heap_close(parent, NoLock);
 
 		/* Update the pg_class entry. */
-		StorePartitionBound(rel, bound);
+		StorePartitionBound(rel, parent, bound);
+
+		heap_close(parent, NoLock);
 
 		/*
 		 * The code that follows may also update the pg_class tuple to update
@@ -13141,7 +13142,7 @@ ATExecAttachPartition(List **wqueue, Relation rel, PartitionCmd *cmd)
 			  cmd->bound);
 
 	/* Update the pg_class entry. */
-	StorePartitionBound(attachRel, cmd->bound);
+	StorePartitionBound(attachRel, rel, cmd->bound);
 
 	/*
 	 * Generate partition constraint from the partition bound specification.
@@ -13352,12 +13353,6 @@ ATExecAttachPartition(List **wqueue, Relation rel, PartitionCmd *cmd)
 		}
 	}
 
-	/*
-	 * Invalidate the parent's relcache so that the new partition is now
-	 * included its partition descriptor.
-	 */
-	CacheInvalidateRelcache(rel);
-
 	ObjectAddressSet(address, RelationRelationId, RelationGetRelid(attachRel));
 
 	/* keep our lock until commit */
diff --git a/src/include/catalog/heap.h b/src/include/catalog/heap.h
index 77dc1983e8..0e4262f611 100644
--- a/src/include/catalog/heap.h
+++ b/src/include/catalog/heap.h
@@ -143,6 +143,6 @@ extern void StorePartitionKey(Relation rel,
 	Oid *partopclass,
 	Oid *partcollation);
 extern void RemovePartitionKeyByRelId(Oid relid);
-extern void StorePartitionBound(Relation rel, Node *bound);
+extern void StorePartitionBound(Relation rel, Relation parent, Node *bound);
 
 #endif   /* HEAP_H */
-- 
2.11.0

>From 21d38e05d1107a88178e9954b1f4a3804cd9e154 Mon Sep 17 00:00:00 2001
From: amit 
Date: Thu, 15 Dec 2016 16:27:04 +0900
Subject: [PATCH 2/7] Change how RelationGetPartitionQual() and related code
 works

Since we always want to recurse, ie, 

[HACKERS] Retire src/backend/port/dynloader/linux.c ?

[Andres Freund]

Hi,

I don't think PG works on any linux without dlopen(). And DLD (what's
used in the dlopen replacement) hasn't been maintained in a while.

See https://www.gnu.org/software/dld/

Shouldn't we just remove that code?

Greetings,

Andres Freund


-- 
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] Trouble building uuid-ossp extension in new versions of Postgres

[Ryan Murphy]

>
> On Mac, the recommended thing is to forget about the ossp code and
>> use "configure --with-uuid=e2fs".  Sorry if that wasn't clear enough.
>>
>>
> Ok thanks, I'll try this.
>

Thanks Tom, "uuid-ossp" built perfectly with "--with--uuid=e2fs".

Cheers and Happy Holidays!
Ryan

Re: [HACKERS] delta relations in AFTER triggers

[Thomas Munro]

On Sun, Dec 18, 2016 at 3:15 PM, Kevin Grittner  wrote:
> Patch v8 ...

FWIW here's that plpython patch, adjusted to apply on top of your latest patch.

-- 
Thomas Munro
http://www.enterprisedb.com


delta-relations-plpython.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] Hash tables in dynamic shared memory

[Thomas Munro]

On Sun, Nov 20, 2016 at 11:54 AM, John Gorman  wrote:
> I reviewed the dht-v2.patch and found that it is in excellent shape.

Thanks for reviewing!  And sorry for the late reply.

> Benchmarking shows that this performs somewhat faster than
> dynahash which is surprising because it is doing DSA address
> translations on the fly.

That is indeed surprising and I think warrants more investigation.

> One area where this could run faster is to reduce the amount of
> time when the hash is in the RESIZE_IN_PROGRESS state.
>
> When a hash partition reaches 75% full a resize begins by allocating
> an empty new hash bucket array with double the number of buckets.
> This begins the RESIZE_IN_PROGRESS state where there is both an
> old and a new hash bucket array.
>
> During the RESIZE state all operations such as find, insert,
> delete and iterate run slower due to having to look items up in
> two hash bucket arrays.
>
> Whenever a new item is inserted into the hash and the hash is in
> the RESIZE state the current code takes the time to move one item
> from the old hash partition to the new hash partition. During
> insertion an exclusive lock is already held on the partition so
> this is an efficient time to do the resize cleanup work.
>
> However since we only clean up one item per insertion we do not
> complete the cleanup and free the old hash bucket array until we
> are due to start yet another resize operation.
>
> So we are effectively always in the RESIZE state which slows down
> operations and wastes some space.

Right, that is the case in a workload that inserts a bunch of data but
then becomes read-only forever.  A workload that constantly does a mix
of writing and reading should settle down at a reasonable size and
stop resizing.

> Here are the timings for insert and find in nanoseconds on a
> Macbook Pro. The insert_resize figure includes the resize work to
> move one item from the old to the new hash bucket array.
>
> insert_resize: 945.98
> insert_clean:  450.39
> find_resize:   348.90
> find_clean:293.16
>
> The attached dht-v2-resize-cleanup.patch patch applies on top of
> the dht-v2.patch and speeds up the resize cleanup process so that
> hashes spend most of their time in the clean state.
>
> It does this by cleaning up more than one old item during
> inserts. This patch cleans up three old items.
>
> There is also the case where a hash receives all of its inserts
> at the beginning and the rest of the work load is all finds. This
> patch also cleans up two items for each find.
>
> The downside of doing extra cleanup early is some additional
> latency. Here are the experimental figures and the approximate
> formulas for different numbers of cleanups for inserts. Note that
> we cannot go lower than one cleanup per insert.
>
> Resize work in inserts: 729.79 insert + 216.19 * cleanups
> 1  945.98
> 2 1178.13
> 3 1388.73
> 4 1617.04
> 5 1796.91
>
> Here are the timings for different numbers of cleanups for finds.
> Note that since we do not hold an exclusive partition lock on a find
> there is the additional overhead of taking that lock.
>
> Resize work in finds: 348.90 dirty_find + 233.45 lock + 275.78 * cleanups
> 0  348.90
> 1  872.88
> 2 1138.98
> 3 1448.94
> 4 1665.31
> 5 1975.91
>
> The new effective latencies during the resize vs. the clean states.
>
> #define DHT_INSERT_CLEANS 3
> #define DHT_SEARCH_CLEANS 2
>
> insert_resize: 1388.73  -- 3 cleaned
> insert_clean:   450.39
> find_resize:   1138.98  -- 2 cleaned
> find_clean: 293.16
>
> The overall performance will be faster due to not having to examine
> more than one hash bucket array most of the time.

Thanks for doing all these experiments.  Yeah, I think it makes sense
to merge this change to improve that case.

Since Dilip Kumar's Parallel Bitmap Heap Scan project is no longer
using this, I think I should park it here unless/until another
potential use case pops up.  Some interesting candidates have been
mentioned already, and I'm fairly sure there are other uses too, but I
don't expect anyone to be interested in committing this patch until
something concrete shows up, so I'll go work on other things until
then.

-- 
Thomas Munro
http://www.enterprisedb.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] pg_background contrib module proposal

[David Fetter]

On Mon, Dec 12, 2016 at 10:17:24PM +0500, Andrew Borodin wrote:
> Hi!
> 
> Just in case you'd like to include sleepsort as a test, here it is
> wrapped as a regression test(see attachment). But it has serious
> downside: it runs no less than 5 seconds.

Couldn't it sleep in increments smaller than a second?  Like maybe
milliseconds?  Also, it's probably cleaner (or at least more
comprehensible) to write something using format() and dollar quoting
than the line with the double 's.

Best,
David.
-- 
David Fetter  http://fetter.org/
Phone: +1 415 235 3778  AIM: dfetter666  Yahoo!: dfetter
Skype: davidfetter  XMPP: david(dot)fetter(at)gmail(dot)com

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Parallel build with MSVC

[Noah Misch]

On Thu, Sep 08, 2016 at 08:54:08AM +0200, Christian Ullrich wrote:
> * Noah Misch wrote:
> 
> >Committed.
> 
> Much apologizings for coming in late again, but I just realized it would be
> better if the user-controlled flags came after all predefined options the
> user might want to override. Right now that is only /verbosity in both build
> and clean operations.
> 
> Patch attached, also reordering the ecpg-clean command line in clean.bat to
> match the others that have the project file first.

Committed.


-- 
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] delta relations in AFTER triggers

[Thomas Munro]

On Sun, Dec 18, 2016 at 3:15 PM, Kevin Grittner  wrote:
> On Sun, Dec 4, 2016 at 11:35 PM, Haribabu Kommi
>  wrote:
>
>> Moved to next CF with "waiting on author" status.
>
> Patch v8 attempts to address the issues explicitly raised in
> Thomas Munro's review.  An opaque query environment is created
> that, for now, only passes through ephemeral named relations, of
> which the only initial implementation is named tuplestores; but
> the techniques are intended to support both other types of
> ephemeral named relations and environmental properties (things
> affecting parsing, planning, and execution that are not coming from
> the system catalog) besides ENRs.  There is no clue in the access
> to the QE whether something is, for example, stored in a list or a
> hash table.  That's on purpose, so that the addition of other
> properties or changes to their implementation doesn't affect the
> calling code.

+1

> There were a few changes Thomas included in the version he posted,
> without really delving into an explanation for those changes.  Some
> or all of them are likely to be worthwhile, but I would rather
> incorporate them based on explicit discussion, so this version
> doesn't do much other than generalize the interface a little,
> change some names, and add more regression tests for the new
> feature.  (The examples I worked up for the rough proof of concept
> of enforcement of RI through set logic rather than row-at-a-time
> navigation were the basis for the new tests, so the idea won't get
> totally lost.)  Thomas, please discuss each suggested change (e.g.,
> the inclusion of the query environment in the parameter list of a
> few more functions).

I was looking for omissions that would cause some kind of statements
to miss out on ENRs arbitrarily.  It seemed to me that
parse_analyze_varparams should take a QueryEnvironment, mirroring
parse_analyze, so that PrepareQuery could pass it in.  Otherwise,
PREPARE wouldn't see ENRs.  Is there any reason why SPI_prepare should
see them but PREPARE not?

Should we attempt to detect if the tupledesc changes incompatibly
between planning and execution?

> Changed to "Needs review" status.

+   enr->md.enrtuples =
tuplestore_tuple_count(trigdata->tg_newtable);

I was wondering about this.  As I understand it, plans for statements
in plpgsql functions are automatically cached.  Is it OK for the
number of tuples on the first invocation of the trigger in a given
session to determine the estimate used for the plan?  I suppose that
is the case with regular tables, so maybe it is.

+   register_enr(estate.queryEnv, enr);
+   SPI_register_relation(enr);

Here plpgsql calls both register_enr and SPI_register_relation.  Yet
SPI_register_relation calls register_enr too, so is this a mistake?
Also, the return code isn't checked.

-- 
Thomas Munro
http://www.enterprisedb.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] building HEAD on macos fails with #error no source of random numbers configured

[Peter Eisentraut]

On 12/11/16 1:52 PM, Jim Nasby wrote:
> On 12/9/16 9:53 AM, Heikki Linnakangas wrote:
>>>
>>> I'm not sure what --no-recursion does, but I would say that we'd
>>> consider that unsupported as well.
>>
>> Interesting. Running config.status adds those --no-create --no-recursion
>> flags automatically. You can see them in the command-line at the top of
>> config.log, too. I never bothered to check what they do...
> 
> AIUI they have to do with config dependency checking (where a simple 
> make will detect if config needs to run again or not). I've been bitten 
> by this in the past as well. Maybe there's a way to get config to spit 
> out a warning for those options and have make filter the warning out.

When config.status is out of date against configure, then
Makefile.global runs config.status --recheck, which internally runs the
original configure line with --no-create and --no-recursion added.
--no-create means not to create any output files, because the makefile
rules will create those.  --no-recursion means not to run any configure
scripts in subdirectories (we don't use that functionality).

It's arguably a bit confusing that config.log then records the configure
line with --no-create and --no-recursion added.  But other than that,
everything works as intended.

-- 
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] dblink get_connect_string() passes FDW option "updatable" to the connect string, connection fails.

[Corey Huinker]

On Sun, Dec 18, 2016 at 4:57 PM, Michael Paquier 
wrote:

> On Mon, Dec 19, 2016 at 6:48 AM, Joe Conway  wrote:
> > Maybe if "CREATE FOREIGN DATA WRAPPER" had a way to specify that the FDW
> > supports a libpq connection it would make sense to allows other FDWs
> > with this attribute, but since there is none the current state strikes
> > me as a bad idea.
> >
> > Thoughts?
>
> libpq is proper to the implementation of the FDW, not the wrapper on
> top of it, so using in the CREATE FDW command a way to do the
> decision-making that does not look right to me. Filtering things at
> connection attempt is a better solution.
> --
> Michael
>

The only benefit I see would be giving the user a slightly more clear error
message like ('dblink doesn't work with %', 'mysql') instead of the error
about the failure of the connect string generated by the options that did
overlap.

Gaming out the options of a Wrapper X pointing to server Y:

1. Wrapper X does not have enough overlap in options to accidentally create
a legit connect string:
Connection fails, user gets a message about the incompleteness of the
connection.
2. Wrapper X has enough options (i.e. user+host+dbname,etc) to generate a
legit connect string (with matching port), but server+port pointed to by X
isn't listening or doesn't speak libpq:
Connection fails, user gets an error message.
3. Wrapper X has enough options (i.e. user+host+dbname,etc) to generate a
legit connect string (without matching port), but server+5432 pointed to by
X doesn't speak libpq:
Whatever *is* listening on 5432 has a chance to try to handshake
libpq-style, and I guess there's a chance a hostile service listening on
that port might know enough libpq to siphon off the credentials, but the
creds it would get would be to a pgsql service on Y and Y is already
compromised. Also, that vulnerability would exist for FDWs that do speak
libpq as well.
4. Wrapper X has enough overlap in options to create a legit connect string
which happens to speak libpq:
Connection succeeds, and it's a feature not a bug.

Re: [HACKERS] dblink get_connect_string() passes FDW option "updatable" to the connect string, connection fails.

[Michael Paquier]

On Mon, Dec 19, 2016 at 6:48 AM, Joe Conway  wrote:
> Maybe if "CREATE FOREIGN DATA WRAPPER" had a way to specify that the FDW
> supports a libpq connection it would make sense to allows other FDWs
> with this attribute, but since there is none the current state strikes
> me as a bad idea.
>
> Thoughts?

libpq is proper to the implementation of the FDW, not the wrapper on
top of it, so using in the CREATE FDW command a way to do the
decision-making that does not look right to me. Filtering things at
connection attempt is a better solution.
-- 
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] dblink get_connect_string() passes FDW option "updatable" to the connect string, connection fails.

[Joe Conway]

On 11/21/2016 03:59 PM, Corey Huinker wrote:
> On 11/21/2016 02:16 PM, Tom Lane wrote:
>> The dblink docs recommend using dblink_fdw as the FDW for this purpose,
>> which would only accept legal connstr options.  However, I can see the
>> point of using a postgres_fdw server instead, and considering that
>> dblink isn't actually enforcing use of any particular FDW type, it seems
>> like the onus should be on it to be more wary of what the options are.

> I have to admit, this was the first I'd heard of dblink_fdw. I'm glad it
> exists, though. And yes, I'd like to be able to use postgres_fdw entries
> because there's value in knowing that the connection for your ad-hoc SQL
> exactly matches the connection used for other FDW tables.

> I'm happy to write the patch, for both v10 and any back-patches we feel
> are necessary.

I looked at Corey's patch, which is straightforward enough, but I was
left wondering if dblink should be allowing any FDW at all (as it does
currently), or should it be limited to dblink_fdw and postgres_fdw? It
doesn't make sense to even try if the FDW does not connect via libpq.

Maybe if "CREATE FOREIGN DATA WRAPPER" had a way to specify that the FDW
supports a libpq connection it would make sense to allows other FDWs
with this attribute, but since there is none the current state strikes
me as a bad idea.

Thoughts?

Joe

-- 
Crunchy Data - http://crunchydata.com
PostgreSQL Support for Secure Enterprises
Consulting, Training, & Open Source Development



signature.asc
Description: OpenPGP digital signature

Re: [HACKERS] move collation import to backend

[Peter Eisentraut]

On 11/30/16 8:18 AM, Peter Eisentraut wrote:
>> It seems not to be project style to have prototypes in the middle of the
>> file...
> 
> OK, will fix.

Updated patch with that fix.

-- 
Peter Eisentraut  http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
>From 0c17610b698cc335bc0aed1a66d151e96f618537 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut 
Date: Wed, 30 Nov 2016 12:00:00 -0500
Subject: [PATCH v3] Add function to import operation system collations

Move this logic out of initdb into a user-callable function.  This
simplifies the code and makes it possible to update the standard
collations later on if additional operating system collations appear.
---
 src/backend/catalog/pg_collation.c|  18 +++-
 src/backend/commands/collationcmds.c  | 149 +-
 src/bin/initdb/initdb.c   | 164 +-
 src/include/catalog/pg_collation_fn.h |   3 +-
 src/include/catalog/pg_proc.h |   3 +
 src/include/commands/collationcmds.h  |   2 +
 6 files changed, 172 insertions(+), 167 deletions(-)

diff --git a/src/backend/catalog/pg_collation.c b/src/backend/catalog/pg_collation.c
index f37cf37c4a..cda64c44a1 100644
--- a/src/backend/catalog/pg_collation.c
+++ b/src/backend/catalog/pg_collation.c
@@ -41,7 +41,8 @@ Oid
 CollationCreate(const char *collname, Oid collnamespace,
 Oid collowner,
 int32 collencoding,
-const char *collcollate, const char *collctype)
+const char *collcollate, const char *collctype,
+bool if_not_exists)
 {
 	Relation	rel;
 	TupleDesc	tupDesc;
@@ -72,10 +73,21 @@ CollationCreate(const char *collname, Oid collnamespace,
 			  PointerGetDatum(collname),
 			  Int32GetDatum(collencoding),
 			  ObjectIdGetDatum(collnamespace)))
-		ereport(ERROR,
+	{
+		if (if_not_exists)
+		{
+			ereport(NOTICE,
 (errcode(ERRCODE_DUPLICATE_OBJECT),
- errmsg("collation \"%s\" for encoding \"%s\" already exists",
+ errmsg("collation \"%s\" for encoding \"%s\" already exists, skipping",
 		collname, pg_encoding_to_char(collencoding;
+			return InvalidOid;
+		}
+		else
+			ereport(ERROR,
+	(errcode(ERRCODE_DUPLICATE_OBJECT),
+	 errmsg("collation \"%s\" for encoding \"%s\" already exists",
+			collname, pg_encoding_to_char(collencoding;
+	}
 
 	/*
 	 * Also forbid matching an any-encoding entry.  This test of course is not
diff --git a/src/backend/commands/collationcmds.c b/src/backend/commands/collationcmds.c
index 9bba748708..eafc0a99fa 100644
--- a/src/backend/commands/collationcmds.c
+++ b/src/backend/commands/collationcmds.c
@@ -136,7 +136,11 @@ DefineCollation(ParseState *pstate, List *names, List *parameters)
 			 GetUserId(),
 			 GetDatabaseEncoding(),
 			 collcollate,
-			 collctype);
+			 collctype,
+			 false);
+
+	if (!newoid)
+		return InvalidObjectAddress;
 
 	ObjectAddressSet(address, CollationRelationId, newoid);
 
@@ -177,3 +181,146 @@ IsThereCollationInNamespace(const char *collname, Oid nspOid)
  errmsg("collation \"%s\" already exists in schema \"%s\"",
 		collname, get_namespace_name(nspOid;
 }
+
+
+/*
+ * "Normalize" a locale name, stripping off encoding tags such as
+ * ".utf8" (e.g., "en_US.utf8" -> "en_US", but "br_FR.iso885915@euro"
+ * -> "br_FR@euro").  Return true if a new, different name was
+ * generated.
+ */
+static bool
+normalize_locale_name(char *new, const char *old)
+{
+	char	   *n = new;
+	const char *o = old;
+	bool		changed = false;
+
+	while (*o)
+	{
+		if (*o == '.')
+		{
+			/* skip over encoding tag such as ".utf8" or ".UTF-8" */
+			o++;
+			while ((*o >= 'A' && *o <= 'Z')
+   || (*o >= 'a' && *o <= 'z')
+   || (*o >= '0' && *o <= '9')
+   || (*o == '-'))
+o++;
+			changed = true;
+		}
+		else
+			*n++ = *o++;
+	}
+	*n = '\0';
+
+	return changed;
+}
+
+
+Datum
+pg_import_system_collations(PG_FUNCTION_ARGS)
+{
+	bool		if_not_exists = PG_GETARG_BOOL(0);
+	Oid nspid = PG_GETARG_OID(1);
+
+	FILE	   *locale_a_handle;
+	char		localebuf[NAMEDATALEN]; /* we assume ASCII so this is fine */
+	int			count = 0;
+
+	if (!superuser())
+		ereport(ERROR,
+(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+ (errmsg("must be superuser to import system collations";
+
+	locale_a_handle = OpenPipeStream("locale -a", "r");
+	if (locale_a_handle == NULL)
+		ereport(ERROR,
+(errcode_for_file_access(),
+ errmsg("could not execute command \"%s\": %m",
+		"locale -a")));
+
+	while (fgets(localebuf, sizeof(localebuf), locale_a_handle))
+	{
+		int			i;
+		size_t		len;
+		int			enc;
+		bool		skip;
+		char		alias[NAMEDATALEN];
+
+		len = strlen(localebuf);
+
+		if (len == 0 || localebuf[len - 1] != '\n')
+		{
+			elog(DEBUG1, "locale name too long, skipped: \"%s\"", localebuf);
+			continue;
+		}
+		localebuf[len - 1] = '\0';
+
+		/*
+		 * Some systems have locale names that don't consist entirely of ASCII
+		 * 

Re: [HACKERS] Logical Replication WIP

[Steve Singer]

On 12/18/2016 05:28 AM, Petr Jelinek wrote:

On 17/12/16 18:34, Steve Singer wrote:

On 12/16/2016 07:49 AM, Petr Jelinek wrote:
Yeah subscriptions are per database. I don't want to make v14 just 
for these 2 changes as that would make life harder for anybody 
code-reviewing the v13 so attached is diff with above fixes that 
applies on top of v13. 





Thanks that fixes those issues.

A few more I've noticed


pg_dumping subscriptions doesn't seem to work

./pg_dump -h localhost --port 5441 --include-subscriptions test
pg_dump: [archiver (db)] query failed: ERROR:  missing FROM-clause entry 
for table "p"

LINE 1: ...LECT rolname FROM pg_catalog.pg_roles WHERE oid = p.subowner...
 ^
pg_dump: [archiver (db)] query was: SELECT s.tableoid, s.oid, 
s.subname,(SELECT rolname FROM pg_catalog.pg_roles WHERE oid = 
p.subowner) AS rolname, s.subenabled,  s.subconninfo, s.subslotname, 
s.subpublications FROM pg_catalog.pg_subscription s WHERE s.subdbid = 
(SELECT oid FROM pg_catalog.pg_database   WHERE datname 
= current_database())


I have attached a patch that fixes this.

pg_dump is also generating warnings

pg_dump: [archiver] WARNING: don't know how to set owner for object type 
SUBSCRIPTION


I know that the plan is to add proper ACL's for publications and 
subscriptions later. I don't know if we want to leave the warning in 
until then or do something about it.



Also the tab-competion for create subscription doesn't seem to work as 
intended.
I've attached a patch that fixes it and patches to add tab completion 
for alter publication|subscription




>From 3ed2ad471766490310b1102d8c9166a597ac11af Mon Sep 17 00:00:00 2001
From: Steve Singer 
Date: Sun, 18 Dec 2016 12:37:29 -0500
Subject: [PATCH 3/4] Fix tab complete for CREATE SUBSCRIPTION

---
 src/bin/psql/tab-complete.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 8816f6c..6dd47f6 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -2298,8 +2298,10 @@ psql_completion(const char *text, int start, int end)
 /* CREATE SUBSCRIPTION */
 	else if (Matches3("CREATE", "SUBSCRIPTION", MatchAny))
 		COMPLETE_WITH_CONST("CONNECTION");
+	else if (Matches5("CREATE", "SUBSCRIPTION", MatchAny, "CONNECTION",MatchAny))
+		COMPLETE_WITH_CONST("PUBLICATION");	
 	/* Complete "CREATE SUBSCRIPTION  ...  WITH ( " */
-	else if (Matches2("CREATE", "SUBSCRIPTION") && TailMatches2("WITH", "("))
+	else if (HeadMatches2("CREATE", "SUBSCRIPTION") && TailMatches2("WITH", "("))
 		COMPLETE_WITH_LIST5("ENABLED", "DISABLED", "CREATE SLOT",
 			"NOCREATE SLOT", "SLOT NAME");
 
-- 
2.1.4

>From 36a0d4382f7ffd2b740298a2bafd49471766bdad Mon Sep 17 00:00:00 2001
From: Steve Singer 
Date: Sun, 18 Dec 2016 12:51:14 -0500
Subject: [PATCH 2/4] Add tab-complete for ALTER SUBSCRIPTION

---
 src/bin/psql/tab-complete.c | 11 ++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 4cbb848..8816f6c 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -1410,7 +1410,7 @@ psql_completion(const char *text, int start, int end)
 			"EVENT TRIGGER", "EXTENSION", "FOREIGN DATA WRAPPER", "FOREIGN TABLE", "FUNCTION",
 			"GROUP", "INDEX", "LANGUAGE", "LARGE OBJECT", "MATERIALIZED VIEW", "OPERATOR",
 			"POLICY", "PUBLICATION", "ROLE", "RULE", "SCHEMA", "SERVER", "SEQUENCE",
-			"SYSTEM", "TABLE", "TABLESPACE", "TEXT SEARCH", "TRIGGER", "TYPE",
+			"SUBSCRIPTION", "SYSTEM", "TABLE", "TABLESPACE", "TEXT SEARCH", "TRIGGER", "TYPE",
 		"USER", "USER MAPPING FOR", "VIEW", NULL};
 
 		COMPLETE_WITH_LIST(list_ALTER);
@@ -1446,6 +1446,15 @@ psql_completion(const char *text, int start, int end)
 		COMPLETE_WITH_LIST6("PUBLISH INSERT", "NOPUBLISH INSERT", "PUBLISH UPDATE",
 			"NOPUBLISH UPDATE", "PUBLISH DELETE", "NOPUBLISH DELETE");
 	}
+	/* ALTER SUBSCRIPTION  ... */
+	else if (Matches3("ALTER","SUBSCRIPTION",MatchAny))
+	{
+		COMPLETE_WITH_LIST5("WITH", "CONNECTION", "SET PUBLICATION", "ENABLE", "DISABLE");
+	}
+	else if (HeadMatches3("ALTER", "SUBSCRIPTION", MatchAny) && TailMatches2("WITH", "("))
+	{
+		COMPLETE_WITH_CONST("SLOT NAME");
+	}
 	/* ALTER SCHEMA  */
 	else if (Matches3("ALTER", "SCHEMA", MatchAny))
 		COMPLETE_WITH_LIST2("OWNER TO", "RENAME TO");
-- 
2.1.4

>From d4685b991245270221a33e0cf8e61d00fb0bf67e Mon Sep 17 00:00:00 2001
From: Steve Singer 
Date: Sun, 18 Dec 2016 12:47:15 -0500
Subject: [PATCH 1/4] Add tab-complete for ALTER PUBLICATION

---
 src/bin/psql/tab-complete.c | 16 +---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 6ad05b7..4cbb848 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -1409,8 +1409,8 @@ 

Re: [HACKERS] Hash Indexes

[Amit Kapila]

On Fri, Dec 16, 2016 at 9:57 PM, Robert Haas  wrote:
> On Thu, Dec 15, 2016 at 11:33 AM, Amit Kapila  wrote:
>> Attached are the two patches on top of remove-hash-wrtbuf.   Patch
>> fix_dirty_marking_v1.patch allows to mark the buffer dirty in one of
>> the corner cases in _hash_freeovflpage() and avoids to mark dirty
>> without need in _hash_squeezebucket().  I think this can be combined
>> with remove-hash-wrtbuf patch. fix_lock_chaining_v1.patch fixes the
>> chaining behavior (lock next overflow bucket page before releasing the
>> previous bucket page) was broken in _hash_freeovflpage().  These
>> patches can be applied in series, first remove-hash-wrtbuf, then
>> fix_dirst_marking_v1.patch and then fix_lock_chaining_v1.patch.
>
> I committed remove-hash-wrtbuf and fix_dirty_marking_v1 but I've got
> some reservations about fix_lock_chaining_v1.  ISTM that the natural
> fix here would be to change the API contract for _hash_freeovflpage so
> that it doesn't release the lock on the write buffer.  Why does it
> even do that?  I think that the only reason why _hash_freeovflpage
> should be getting wbuf as an argument is so that it can handle the
> case where wbuf happens to be the previous block correctly.
>

Yeah, as of now that is the only case, but for WAL patch, I think we
need to ensure that the action of moving all the tuples to the page
being written and the overflow page being freed needs to be logged
together as an atomic operation.  Now apart from that, it is
theoretically possible that write page will remain locked for multiple
overflow pages being freed (when the page being written has enough
space that it can accommodate tuples from multiple overflow pages).  I
am not sure if it is worth worrying about such a case because
practically it might happen rarely.  So, I have prepared a patch to
retain a lock on wbuf in _hash_freeovflpage() as suggested by you.

-- 
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com


fix_lock_chaining_v2.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] Proposal for changes to recovery.conf API

[Bruce Momjian]

On Sun, Dec 18, 2016 at 02:14:06PM +0100, Magnus Hagander wrote:
> >     turn
> >     > into another section that we keep around (whether as part of the
> release
> >     notes,
> >     > or as a separate "upgrade steps" section or something).
> >
> >     I suggest whatever we do, we place the information in a permanent
> >     location that isn't moved or removed.
> >
> >
> >
> > +1. Absolutely. That's a very important point.
> 
> That is really my only point --- wherever you put it, expect it to live
> there forever.
>
> Then in the end, it turns out we're in strong agreement :) 

Yes, but there was talk of adding it to the docs, then removing it in
later releases --- that's what I think is a bad idea.  I think it would
be logical to have a sub-document underneath each major release note
section with more detailed instructions.  This could be done for minor
releases as well where necessary.

-- 
  Bruce Momjian  http://momjian.us
  EnterpriseDB http://enterprisedb.com

+ As you are, so once was I.  As I am, so you will be. +
+  Ancient Roman grave inscription +


-- 
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] Proposal for changes to recovery.conf API

[Magnus Hagander]

On Sun, Dec 18, 2016 at 2:11 PM, Bruce Momjian  wrote:

> On Sun, Dec 18, 2016 at 02:02:58PM +0100, Magnus Hagander wrote:
> > > Again, I am fine putting this as a subsection of the release
> notes,
> > but
> > > let's not pretend it is some extra section we can remove in
> five
> > years.
> > >
> > >
> > > Depends on what we decide to do about it, but sure, it could
> certainly
> > turn
> > > into another section that we keep around (whether as part of the
> release
> > notes,
> > > or as a separate "upgrade steps" section or something).
> >
> > I suggest whatever we do, we place the information in a permanent
> > location that isn't moved or removed.
> >
> >
> >
> > +1. Absolutely. That's a very important point.
>
> That is really my only point --- wherever you put it, expect it to live
> there forever.
>
>
Then in the end, it turns out we're in strong agreement :)


-- 
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/

Re: [HACKERS] Proposal for changes to recovery.conf API

[Bruce Momjian]

On Sun, Dec 18, 2016 at 02:02:58PM +0100, Magnus Hagander wrote:
> >     Again, I am fine putting this as a subsection of the release notes,
> but
> >     let's not pretend it is some extra section we can remove in five
> years.
> >
> >
> > Depends on what we decide to do about it, but sure, it could certainly
> turn
> > into another section that we keep around (whether as part of the release
> notes,
> > or as a separate "upgrade steps" section or something).
> 
> I suggest whatever we do, we place the information in a permanent
> location that isn't moved or removed.
> 
> 
> 
> +1. Absolutely. That's a very important point.

That is really my only point --- wherever you put it, expect it to live
there forever.

-- 
  Bruce Momjian  http://momjian.us
  EnterpriseDB http://enterprisedb.com

+ As you are, so once was I.  As I am, so you will be. +
+  Ancient Roman grave inscription +


-- 
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] Trouble building uuid-ossp extension in new versions of Postgres

[Ryan Murphy]

> On Mac, the recommended thing is to forget about the ossp code and
> use "configure --with-uuid=e2fs".  Sorry if that wasn't clear enough.
>
>
Ok thanks, I'll try this.


> Reading over your post again, it sounds like you're trying to force-build
> contrib/uuid-ossp without having used any of the configure options that
> are supposed to enable it.  I'm not sure that would ever have worked very
> reliably.
>
>
Ok, that makes sense.  I had never learned the proper procedure for
building extensions, but have had success with going into e.g.
contrib/pgcrypto and typing make to build the pgcrypto extension.  Since
things like that have generally worked for me I didn't learn about the
proper flags etc.

I'll try some more things and get back to you, thanks for the help.

Best,
Ryan

Re: [HACKERS] Proposal for changes to recovery.conf API

[Magnus Hagander]

On Sun, Dec 18, 2016 at 1:57 PM, Bruce Momjian  wrote:

> On Sun, Dec 18, 2016 at 12:41:01PM +0100, Magnus Hagander wrote:
> > No, they become uninteresting to anyone who has passed Postgres 10.
> I
> > would argue they are still required to be around even after we stop
> > supporting Postgres 9.6 because we know everyone will not upgrade
> off of
> > supported releases once we stop supporting them.  This means we can
> > effectively never remove the information.
> >
> >
> > This is true, but I think it's also safe to say that it's acceptable
> that if
> > you are upgrading from an unsupported version you need to read more than
> one
> > set of documentation -- one set to get to a supported one, and one get
> on from
> > there.
>
> How do you propose they find that other documentation set if upgrading
> from 9.6 to version 16?  Do we put the old docs somewhere on our web
> site?  Do we have them download a tarball and unpack it?  How do we
> handle old translated doc versions?  I realize the wiki isn't
> translated, but if we have translators translate it, we should keep it
> available.
>

We have all the old documentation up on the website, yes. So far it goes
back to 6.3. There is no reason to stop that.

The wiki is not a good location for documentation that needs to be around
for a long time.

For translated docs, that's really up to each translation team, since we
don't have any central repository for it. We do have that for the
translation of the code, but not docs.



> > Right now if you migrate from Postgres 8.0 to Postgres 9.6, all the
> > information you need is in the 9.6 documentation.  If you were to
> remove
> > migration details from 8.4 to 9.0 they would have to look at the 9.0
> > docs to get a full picture of how to migrate.
> >
> >
> > In fairness, all the information you need is definitely not in the
> > documentation. You have all the release notes, that is true. But for a
> lot of
> > people and in the case of a lot of features, that is not at all enough
> > information. But it's true in the sense that it's just as much
> information as
>
> Uh, what exactly is this additional information you are talking about?
> Blogs?  Are you saying we have information about upgrades we have
> captured but discard?
>

Exactly the type of information that Simon is suggested that he writes this
time.

The *how* to migrate.

And no, we don't have this information "officially" today. Some of it is in
blogs, most of it is in the mailinglist archives. The proposal here is,
AIUI, to formalize that so we have it in the formal documentation in the
future.



> > Again, I am fine putting this as a subsection of the release notes,
> but
> > let's not pretend it is some extra section we can remove in five
> years.
> >
> >
> > Depends on what we decide to do about it, but sure, it could certainly
> turn
> > into another section that we keep around (whether as part of the release
> notes,
> > or as a separate "upgrade steps" section or something).
>
> I suggest whatever we do, we place the information in a permanent
> location that isn't moved or removed.
>
>
+1. Absolutely. That's a very important point.


-- 
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/

Re: [HACKERS] Proposal for changes to recovery.conf API

[Bruce Momjian]

On Sun, Dec 18, 2016 at 12:41:01PM +0100, Magnus Hagander wrote:
> No, they become uninteresting to anyone who has passed Postgres 10.  I
> would argue they are still required to be around even after we stop
> supporting Postgres 9.6 because we know everyone will not upgrade off of
> supported releases once we stop supporting them.  This means we can
> effectively never remove the information.
> 
> 
> This is true, but I think it's also safe to say that it's acceptable that if
> you are upgrading from an unsupported version you need to read more than one
> set of documentation -- one set to get to a supported one, and one get on from
> there.

How do you propose they find that other documentation set if upgrading
from 9.6 to version 16?  Do we put the old docs somewhere on our web
site?  Do we have them download a tarball and unpack it?  How do we
handle old translated doc versions?  I realize the wiki isn't
translated, but if we have translators translate it, we should keep it
available.

> Right now if you migrate from Postgres 8.0 to Postgres 9.6, all the
> information you need is in the 9.6 documentation.  If you were to remove
> migration details from 8.4 to 9.0 they would have to look at the 9.0
> docs to get a full picture of how to migrate.
> 
> 
> In fairness, all the information you need is definitely not in the
> documentation. You have all the release notes, that is true. But for a lot of
> people and in the case of a lot of features, that is not at all enough
> information. But it's true in the sense that it's just as much information as

Uh, what exactly is this additional information you are talking about? 
Blogs?  Are you saying we have information about upgrades we have
captured but discard?

> Again, I am fine putting this as a subsection of the release notes, but
> let's not pretend it is some extra section we can remove in five years.
> 
> 
> Depends on what we decide to do about it, but sure, it could certainly turn
> into another section that we keep around (whether as part of the release 
> notes,
> or as a separate "upgrade steps" section or something).

I suggest whatever we do, we place the information in a permanent
location that isn't moved or removed.

-- 
  Bruce Momjian  http://momjian.us
  EnterpriseDB http://enterprisedb.com

+ As you are, so once was I.  As I am, so you will be. +
+  Ancient Roman grave inscription +


-- 
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] Quorum commit for multiple synchronous replication.

[Michael Paquier]

On Fri, Dec 16, 2016 at 10:42 PM, Fujii Masao  wrote:
> Attached is the modified version of the patch. Barring objections, I will
> commit this version.

There is a whitespace:
$ git diff master --check
src/backend/replication/syncrep.c:39: trailing whitespace.
+ *

> Even after committing the patch, there will be still many source comments
> and documentations that we need to update, for example,
> in high-availability.sgml. We need to check and update them throughly later.

The current patch is complicated enough, so that's fine for me. I
checked the patch one last time and that looks good.
-- 
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] Proposal for changes to recovery.conf API

[Magnus Hagander]

On Sat, Dec 17, 2016 at 10:34 PM, Bruce Momjian  wrote:

> On Sat, Dec 17, 2016 at 02:52:41PM +0100, Magnus Hagander wrote:
> > The point is that the documentation about the recovery.conf changes
> in
> > Postgres are only interesting to people migrating to Postgres 10,
> i.e.
> > this is not quality documentation for someone going from Postgres 10
> to
> > Postgres 11.
> >
> >
> >
> > They will also be interesting to people going from 9.4 to 11, or from
> 9.3 to
> > 12. Or from 9.5 to 13.
> >
> > They only become uninteresting when we stop supporting 9.6 which is the
> last
> > version that didn't have that functionality.
>
> No, they become uninteresting to anyone who has passed Postgres 10.  I
> would argue they are still required to be around even after we stop
> supporting Postgres 9.6 because we know everyone will not upgrade off of
> supported releases once we stop supporting them.  This means we can
> effectively never remove the information.
>

This is true, but I think it's also safe to say that it's acceptable that
if you are upgrading from an unsupported version you need to read more than
one set of documentation -- one set to get to a supported one, and one get
on from there.


> Right now if you migrate from Postgres 8.0 to Postgres 9.6, all the
> information you need is in the 9.6 documentation.  If you were to remove
> migration details from 8.4 to 9.0 they would have to look at the 9.0
> docs to get a full picture of how to migrate.
>

In fairness, all the information you need is definitely not in the
documentation. You have all the release notes, that is true. But for a lot
of people and in the case of a lot of features, that is not at all enough
information. But it's true in the sense that it's just as much information
as you would've had if you'd done the incremental steps of upgrading,
because we didn't purge anything.



> Again, I am fine putting this as a subsection of the release notes, but
> let's not pretend it is some extra section we can remove in five years.


Depends on what we decide to do about it, but sure, it could certainly turn
into another section that we keep around (whether as part of the release
notes, or as a separate "upgrade steps" section or something).

//Magnus

Re: [HACKERS] Logical Replication WIP

[Petr Jelinek]

On 17/12/16 18:34, Steve Singer wrote:
> On 12/16/2016 07:49 AM, Petr Jelinek wrote:
>> Hi,
>>
>> attached is version 13 of the patch.
>>
>> I merged in changes from PeterE. And did following changes:
>> - fixed the ownership error messages for both provider and subscriber
>> - added ability to send invalidation message to invalidate whole
>> relcache and use it in publication code
>> - added the post creation/alter/drop hooks
>> - removed parts of docs that refer to initial sync (which does not exist
>> yet)
>> - added timeout handling/retry, etc to apply/launcher based on the GUCs
>> that exist for wal receiver (they could use renaming though)
>> - improved feedback behavior
>> - apply worker now uses owner of the subscription as connection user
>> - more tests
>> - check for max_replication_slots in launcher
>> - clarify the update 'K' sub-message description in protocol
> 
> A few things I've noticed so far
> 
> If I shutdown the publisher I see the following in the log
> 
> 2016-12-17 11:33:49.548 EST [1891] LOG:  worker process: ?)G? (PID 1987)
> exited with exit code 1
> 
> but then if I shutdown the subscriber postmaster and restart it switches to
> 2016-12-17 11:43:09.628 EST [2373] LOG:  worker process:  (PID 2393)
> exited with exit code 1
> 
> Not sure where the 'G' was coming from (other times I have seen an 'I'
> here or other random characters)
> 

Uninitialized bgw_name for apply worker. Rather silly bug. Fixed.

> 
> I don't think we are cleaning up subscriptions on a drop database
> 
> If I do the following
> 
> 1) Create a subscription in a new database
> 2) Stop the publisher
> 3) Drop the database on the subscriber
> 
> test=# create subscription mysuba connection 'host=localhost dbname=test
> port=5440' publication mypub;
> test=# \c b
> b=# drop database test;
> DROP DATABASE
> b=# select * FROM pg_subscription ;
>  subdbid | subname | subowner | subenabled | subconninfo  |
> subslotname | subpublications
> -+-+--++--+-+-
> 
>16384 | mysuba  |   10 | t  | host=localhost dbname=test
> port=5440 | mysuba  | {mypub}
> 

Good one. I added check that prevents dropping database when there is
subscription defined for it. I think we can't cascade here as
subscription may or may not hold resources (slot) in another
instance/database so preventing the drop is best we can do.

> 
> Also I don't think I can now drop mysuba
> b=# drop subscription mysuba;
> ERROR:  subscription "mysuba" does not exist
> 

Yeah subscriptions are per database.

I don't want to make v14 just for these 2 changes as that would make
life harder for anybody code-reviewing the v13 so attached is diff with
above fixes that applies on top of v13.

-- 
  Petr Jelinek  http://www.2ndQuadrant.com/
  PostgreSQL Development, 24x7 Support, Training & Services
diff --git a/src/backend/catalog/pg_subscription.c b/src/backend/catalog/pg_subscription.c
index 8be9f39..2c58cc6 100644
--- a/src/backend/catalog/pg_subscription.c
+++ b/src/backend/catalog/pg_subscription.c
@@ -107,7 +107,41 @@ GetSubscription(Oid subid, bool missing_ok)
 }
 
 /*
- * Free memory allocated by subscription struct. */
+ * Return number of subscriptions defined in given database.
+ * Used by dropdb() to check if database can indeed be dropped.
+ */
+int
+CountDBSubscriptions(Oid dbid)
+{
+	intnsubs = 0;
+	Relation		rel;
+	ScanKeyData		scankey;
+	SysScanDesc		scan;
+	HeapTuple		tup;
+
+	rel = heap_open(SubscriptionRelationId, RowExclusiveLock);
+
+	ScanKeyInit(,
+Anum_pg_subscription_subdbid,
+BTEqualStrategyNumber, F_OIDEQ,
+ObjectIdGetDatum(dbid));
+
+	scan = systable_beginscan(rel, InvalidOid, false,
+			  NULL, 1, );
+
+	while (HeapTupleIsValid(tup = systable_getnext(scan)))
+		nsubs++;
+
+	systable_endscan(scan);
+
+	heap_close(rel, NoLock);
+
+	return nsubs;
+}
+
+/*
+ * Free memory allocated by subscription struct.
+ */
 void
 FreeSubscription(Subscription *sub)
 {
diff --git a/src/backend/commands/dbcommands.c b/src/backend/commands/dbcommands.c
index 0919ad8..45d152c 100644
--- a/src/backend/commands/dbcommands.c
+++ b/src/backend/commands/dbcommands.c
@@ -37,6 +37,7 @@
 #include "catalog/pg_authid.h"
 #include "catalog/pg_database.h"
 #include "catalog/pg_db_role_setting.h"
+#include "catalog/pg_subscription.h"
 #include "catalog/pg_tablespace.h"
 #include "commands/comment.h"
 #include "commands/dbcommands.h"
@@ -790,6 +791,7 @@ dropdb(const char *dbname, bool missing_ok)
 	int			npreparedxacts;
 	int			nslots,
 nslots_active;
+	int			nsubscriptions;
 
 	/*
 	 * Look up the target database's OID, and get exclusive lock on it. We
@@ -875,6 +877,21 @@ dropdb(const char *dbname, bool missing_ok)
  errdetail_busy_db(notherbackends, npreparedxacts)));
 
 	/*
+	 * Check if there are subscriptions defined in the target database.
+	 *
+	 * We can't drop them automatically because 

Re: [HACKERS] Logical Replication WIP

[Petr Jelinek]

On 17/12/16 13:37, Erik Rijkers wrote:
> On 2016-12-16 13:49, Petr Jelinek wrote:
>>
>> version 13 of the patch.
>>
>> 0001-Add-PUBLICATION-catalogs-and-DDL-v13.patch.gz (~32 KB)
>> 0002-Add-SUBSCRIPTION-catalog-and-DDL-v13.patch.gz (~28 KB)
>> 0003-Define-logical-rep...utput-plugi-v13.patch.gz (~13 KB)
>> 0004-Add-logical-replication-workers-v13.patch.gz (~44 KB)
>> 0005-Add-separate-synch...or-logical--v13.patch.gz (~2 KB)
> 
> Hi,
> 
> You wrote on 2016-08-05: :
> 
>> What's missing:
>>  - sequences, I'd like to have them in 10.0 but I don't have good
>>way to implement it. PGLogical uses periodical syncing with some
>>buffer value but that's suboptimal. I would like to decode them
>>but that has proven to be complicated due to their sometimes
>>transactional sometimes nontransactional nature, so I probably
>>won't have time to do it within 10.0 by myself.
> 
> I ran into problems with sequences and I wonder if sequences-problems
> are still expected, as the above seems to imply.
> 
> (short story: I tried to run pgbench across logical replication; and
> therefore
> added a sequence to pgbench_history to give it a replica identity, and
> cannot get it to work reliably ).
> 

Sequences are not replicated but that should not prevent pgbench_history
itself from being replicated when you add serial to it.

BTW you don't need to add primary key to pgbench_history. Simply ALTER
TABLE pgbench_history REPLICA IDENTITY FULL; should be enough.

-- 
  Petr Jelinek  http://www.2ndQuadrant.com/
  PostgreSQL Development, 24x7 Support, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers