Re: [HACKERS] Rename max_parallel_degree?

[Tom Lane]

I wrote:
> At the risk of opening another can of worms, what about renaming
> max_worker_processes as well?  It would be a good thing if that
> had "cluster" in it somewhere, or something that indicates it's a
> system-wide value not a per-session value.  "max_workers_per_cluster"
> would answer, though I'm not in love with it particularly.

Actually, after a bit more thought, maybe "max_background_workers" would
be a good name?  That matches its existing documentation more closely:

 Sets the maximum number of background processes that the system
 can support.  This parameter can only be set at server start.  The
 default is 8.

However, that would still leave us with max_background_workers as the
cluster-wide limit and max_parallel_workers as the per-query-node limit.
That naming isn't doing all that much to clarify the distinction.

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] Rename max_parallel_degree?

[Robert Haas]

On Tue, May 31, 2016 at 5:58 PM, Tom Lane  wrote:
> Alvaro Herrera  writes:
>> Robert Haas wrote:
>>> I just want to point out that if we change #1, we're breaking
>>> postgresql.conf compatibility for, IMHO, not a whole lot of benefit.
>>> I'd just leave it alone.
>
>> We can add the old name as a synonym in guc.c to maintain compatibility.
>
> I doubt this is much of an issue at this point; max_worker_processes has
> only been there a release or so, and surely there are very few people
> explicitly setting it, given its limited use-case up to now.  It will be
> really hard to change it after 9.6, but I think we could still get away
> with that today.

max_worker_processes was added in 9.4, so it's been there for two
releases, but it probably is true that few people have set it.
Nevertheless, I don't think there's much evidence that it is a bad
enough name that we really must change it.

...Robert


-- 
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] Statement timeout

[Amit Kapila]

On Wed, Jun 1, 2016 at 6:03 AM, Tatsuo Ishii  wrote:
>
> From the document about statement_timeout (config.sgml):
>
> Abort any statement that takes more than the specified number of
> milliseconds, starting from the time the command arrives at the
server
> from the client.  If log_min_error_statement is set to
> ERROR or lower, the statement that timed out will
also be
> logged.  A value of zero (the default) turns this off.
>
> Apparently "starting from the time the command arrives at the server
> from the client" referrers to the timing of execute message arrives to
> server the in my example. And I think current behavior of our code is
> doing different from what he document advertises. Or at least counter
> intuitive to users.
>

It seems to me the above documentation is more relevant with respect to
simple query.  However, I agree that it is better if statement_timeout is
the timeout for each execution of the parsed statement.


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

Re: [HACKERS] parallel.c is not marked as test covered

[Noah Misch]

On Sun, May 29, 2016 at 01:31:24AM -0400, Noah Misch wrote:
> On Sun, May 15, 2016 at 12:53:13PM +, Clément Prévost wrote:
> > On Mon, May 9, 2016 at 4:50 PM Andres Freund  wrote:
> > > I think it's a good idea to run a force-parallel run on some buildfarm
> > > members. But I'm rather convinced that the core tests run by all animals
> > > need some minimal coverage of parallel queries. Both because otherwise
> > > it'll be hard to get some coverage of unusual platforms, and because
> > > it's imo something rather relevant to test during development.
> > >
> > Good point.
> > 
> > After some experiments, I found out that, for my setup (9b7bfc3a88ef7b), a
> > parallel seq scan is used given both parallel_setup_cost
> > and parallel_tuple_cost are set to 0 and given that the table is at least 3
> > times as large as the biggest test table tenk1.
> > 
> > The attached patch is a regression test using this method that is
> > reasonably small and fast to run. I also hid the workers count from the
> > explain output when costs are disabled as suggested by Tom Lane and Robert
> > Haas on this same thread (
> > http://www.postgresql.org/message-id/CA+TgmobBQS4ss3+CwoZOKgbsBqKfRndwc=hlialaep5axqc...@mail.gmail.com
> > ).
> > 
> > Testing under these conditions does not test the planner job at all but at
> > least some parallel code can be run on the build farm and the test suite
> > gets 2643 more lines and 188 more function covered.
> > 
> > I don't know however if this test will be reliable on other platforms, some
> > more feedback is needed here.
> 
> [This is a generic notification.]
> 
> The above-described topic is currently a PostgreSQL 9.6 open item.  Robert,
> 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
> 9.6 open item, please let us know.  Otherwise, please observe the policy on
> open item ownership[1] and send a status update within 72 hours 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 9.6rc1.  Consequently, I will appreciate your
> efforts toward speedy resolution.  Thanks.
> 
> [1] 
> http://www.postgresql.org/message-id/20160527025039.ga447...@tornado.leadboat.com

I enjoy reviewing automated test patches, so I persuaded Robert to transfer
ownership of this open item to me.  I will update this thread no later than
2016-06-07 09:00 UTC.  There is an 85% chance I will have reviewed the
proposed patch by then.


-- 
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] Rename max_parallel_degree?

[Josh berkus]

On 05/31/2016 09:15 AM, Robert Haas wrote:
> On Sun, May 29, 2016 at 1:33 AM, Noah Misch  wrote:
>> On Fri, May 06, 2016 at 02:52:30PM -0400, Robert Haas wrote:
>>> OK, my reading of this thread is that there is a consensus is to
>>> redefine max_parallel_degree=1 as "no parallelism" and
>>> max_parallel_degree>1 as "parallelism using a leader plus N-1
>>> workers", and along with that, to keep the names unchanged.  However,
>>> I don't think I can get that done before beta1, at least not without a
>>> serious risk of breaking stuff.  I can look at this post-beta1.
>>
>> [This is a generic notification.]
>>
>> The above-described topic is currently a PostgreSQL 9.6 open item.  Robert,
>> 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
>> 9.6 open item, please let us know.  Otherwise, please observe the policy on
>> open item ownership[1] and send a status update within 72 hours 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 9.6rc1.  Consequently, I will appreciate your
>> efforts toward speedy resolution.  Thanks.
> 
> Here is a patch.  Note that I still don't agree with this change, but
> I'm bowing to the will of the group.
> 
> I think that some of the people who were in favor of this change
> should review this patch, including especially the language I wrote
> for the documentation.  If that happens, and the reviews are positive,
> then I will commit this.  If that does not happen, then I will
> interpret that to mean that there isn't actually all that much
> interest in changing this after all and will accordingly recommend
> that this open item be removed without further action.
> 
> Here is a test which shows how it works:
> 
> rhaas=# set max_parallel_degree = 100;
> SET
> rhaas=# alter table pgbench_accounts set (parallel_degree = 10);
> ALTER TABLE
> rhaas=# explain (analyze) select count(*) from pgbench_accounts;
> 
> QUERY PLAN
> 
>  Finalize Aggregate  (cost=177436.04..177436.05 rows=1 width=8)
> (actual time=383.244..383.244 rows=1 loops=1)
>->  Gather  (cost=177435.00..177436.01 rows=10 width=8) (actual
> time=383.040..383.237 rows=9 loops=1)
>  Workers Planned: 9
>  Workers Launched: 8


I realize there's a lot of water under the bridge here, but I think
we're going to get 1000 questions on -general of the type:  "I asked for
8 parallel workers, why did I only get 7?".  I believe we will regret
this change.

So, one vote from me to revert.

-- 
--
Josh Berkus
Red Hat OSAS
(any opinions are my own)


-- 
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] Rename max_parallel_degree?

[Tom Lane]

Josh berkus  writes:
> I realize there's a lot of water under the bridge here, but I think
> we're going to get 1000 questions on -general of the type:  "I asked for
> 8 parallel workers, why did I only get 7?".  I believe we will regret
> this change.
> So, one vote from me to revert.

Well, that gets back to the question of whether average users will
understand the "degree" terminology.  For the record, while I do not
like the current behavior either, this was not the solution I favored.
I thought we should rename the GUC and keep it as meaning the number
of additional worker processes.

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] Rename max_parallel_degree?

[Josh berkus]

On 05/31/2016 10:03 AM, Tom Lane wrote:
> Josh berkus  writes:
>> I realize there's a lot of water under the bridge here, but I think
>> we're going to get 1000 questions on -general of the type:  "I asked for
>> 8 parallel workers, why did I only get 7?".  I believe we will regret
>> this change.
>> So, one vote from me to revert.
> 
> Well, that gets back to the question of whether average users will
> understand the "degree" terminology.  For the record, while I do not
> like the current behavior either, this was not the solution I favored.
> I thought we should rename the GUC and keep it as meaning the number
> of additional worker processes.

I will happily bet anyone a nice dinner in Ottawa that most users will
not understand it.

Compare this:

"max_parallel is the maximum number of parallel workers which will work
on each stage of the query which is parallizable.  If you set it to 4,
you get up to 4 workers."

with this:

"max_parallel_degree is the amount of parallelism in the query, with the
understanding that the original parent process counts as 1, which means
that if you set it to 1 you get no parallelism, and if you want 4
parallel workers you need to set it to 5."

Which one of those is going to require more explanations on -general and
-novice?  Bets?

Let's not be complicated for the sake of being complicated.

-- 
--
Josh Berkus
Red Hat OSAS
(any opinions are my own)


-- 
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] Rename max_parallel_degree?

[Peter Geoghegan]

On Tue, May 31, 2016 at 10:10 AM, Josh berkus  wrote:
> "max_parallel_degree is the amount of parallelism in the query, with the
> understanding that the original parent process counts as 1, which means
> that if you set it to 1 you get no parallelism, and if you want 4
> parallel workers you need to set it to 5."
>
> Which one of those is going to require more explanations on -general and
> -novice?  Bets?
>
> Let's not be complicated for the sake of being complicated.

But the distinction between parallel workers and backends that can
participate in parallel query does need to be user-visible. Worker
processes are a commodity (i.e. the user must consider
max_worker_processes).

-- 
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] Re: PATCH: Split stats file per database WAS: autovacuum stress-testing our system

[Tom Lane]

Tomas Vondra  writes:
> On 05/31/2016 06:59 PM, Tom Lane wrote:
>> I'm confused here --- are you speaking of having removed
>>  if (msg->cutoff_time > req->request_time)
>>  req->request_time = msg->cutoff_time;
>> ? That is not a check for clock skew, it's intending to be sure that
>> req->request_time reflects the latest request for this DB when we've
>> seen more than one request. But since req->request_time isn't
>> actually being used anywhere, it's useless code.

> Ah, you're right. I've made the mistake of writing the e-mail before 
> drinking any coffee today, and I got distracted by the comment change.

>> I reformatted the actual check for clock skew, but I do not think I
>> changed its behavior.

> I'm not sure it does not change the behavior, though. request_time only 
> became unused after you removed the two places that set the value (one 
> of them in the clock skew check).

Well, it's unused in the sense that the if-test quoted above is the only
place in HEAD that examines the value of request_time.  And since that
if-test only controls whether we change the value, and not whether we
proceed to make the clock skew check, I don't see how it's related
to clock skew or indeed anything else at all.

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] Re: [COMMITTERS] pgsql: Avoid extra locks in GetSnapshotData if old_snapshot_threshold

[Robert Haas]

On Fri, May 27, 2016 at 10:58 AM, Kevin Grittner  wrote:
>> As far as I can see normal index builds will allow concurrent hot
>> prunes and everything; since those only require page-level
>> exclusive locks.
>>
>> So for !concurrent builds we might end up with a corrupt index.
>
> ... by which you mean an index that omits certainly-dead heap
> tuples which have been the subject of early pruning or vacuum, even
> if there are still registered snapshots that include those tuples?
> Or do you see something else?

I think that is the danger.

> Again, since both the heap pages involved and all the new index
> pages would have LSNs recent enough to trigger the old snapshot
> check, I'm not sure where the problem is, but will review closely
> to see what I might have missed.

This is a good point, though, I think.

>> I think many of the places relying on heap scans with !mvcc
>> snapshots are problematic in that way. Outdated reads will not be
>> detected by TestForOldSnapshot() (given the (snapshot)->satisfies
>> == HeapTupleSatisfiesMVCC condition therein), and rely on the
>> snapshot to be actually working.  E.g. CLUSTER/ VACUUM FULL rely
>> on accurate HEAPTUPLE_RECENTLY_DEAD
>
> Don't the "RECENTLY" values imply that the transaction is still
> running which cause the tuple to be dead?  Since tuples are not
> subject to early pruning or vacuum when that is the case, where do
> you see a problem?  The snapshot itself has the usual xmin et al.,
> so I'm not sure what you might mean by "the snapshot to be actually
> working" if not the override at the time of pruning/vacuuming.

Anybody who calls HeapTupleSatisfiesVacuum() with an xmin value that
is newer that the oldest registered snapshot in the system (based on
some snapshots being ignored) might get a return value of
HEAPTUPLE_DEAD rather than HEAPTUPLE_RECENTLY_DEAD.  It seems
necessary to carefully audit all calls to HeapTupleSatisfiesVacuum()
to see whether that difference matters.  I took a quick look and
here's what I see:

statapprox_heap(): Statistical information for the DBA.  The
difference is non-critical.

heap_prune_chain(): Seeing the tuple as dead might cause it to be
removed early.  This should be OK.  Removing the tuple early will
cause the page LSN to be bumped unless RelationNeedsWAL() returns
false, and TransactionIdLimitedForOldSnapshots() includes that as a
condition for disabling early pruning.

IndexBuildHeapRangeScan(): We might end up with indexIt = false
instead of indexIt = true.  That should be OK because anyone using the
old snapshot will see a new page LSN and error out.  We might also
fail to set indexInfo->ii_BrokenHotChain = true.  I suspect that's a
problem, but I'm not certain of it.

acquire_sample_rows: Both return values are treated in the same way.
No problem.

copy_heap_data: We'll end up setting isdead = true instead of
tups_recently_dead += 1.  That means that the new heap won't include
the tuple, which is OK because old snapshots can't read the new heap
without erroring out, assuming that the new heap has LSNs.  The xmin
used here comes from vacuum_set_xid_limits() which goes through
TransactionIdLimitedForOldSnapshots() so this should be OK for the
same reasons as heap_prune_chain().  Another effect of seeing the
tuple as prematurely dead is that we'll call rewrite_heap_dead_tuple()
on it; rewrite_heap_dead_tuple() will presume that if this tuple is
dead, its predecessor in the ctid chain is also dead.  I don't see any
obvious problem with that.

lazy_scan_heap(): Basically, the same thing as heap_prune_chain().

CheckForSerializableConflictOut: Maybe a problem?  If the tuple is
dead, there's no issue, but if it's recently-dead, there might be.

We might want to add comments to some of these places addressing
snapshot_too_old specifically.

-- 
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] Question and suggestion about application binary compatibility policy

[Marco Atzeri]

On 31/05/2016 08:10, Tsunakawa, Takayuki wrote:

From: Michael Meskes [mailto:mes...@postgresql.org]

Yes, but Windows users probably don't understand or know it.  So, I
suggested explicitly describing the application binary compatibility
policy in the PostgreSQL manual.  What do you think about it?


Couldn't you point your customer to the system documentation?

Personally I don't think standard system behavior should be documented for
each application relying on it but ymmv.


I couldn't find appropriate system documentation.  Regarding Linux, I remember I saw some 
HOWTO on tldp.org website which explains the concept of shared library soname, but it's 
not very friendly for users who just want to know the application binary compatibility 
policy of PostgreSQL.  And I don't think there's suitable documentation on Windows.  Even 
if there is any, users will not be sure whether PostgreSQL follows those 
platform-specific conventions.  They may have doubts about it, because even the product 
version "PostgreSQL x.y.z" causes misconception that x is the major version and 
y is the minor one.

So, I suggested documenting the compatibility policy for clarification and user 
friendliness as in the Oracle Database documentation below.

http://docs.oracle.com/database/121/UPGRD/app.htm#UPGRD12547


BTW, although this may be a separate topic, it may be better that we add the 
major version in the library names like libpq5.dll and libecpg6.dll, so that 
the application can fail to run with the incompatible versions of libpq and 
libecpg.  FYI:

https://en.wikipedia.org/wiki/Side-by-side_assembly

[Excerpt]
Microsoft Visual C++ 2005 and 2008 employ SxS with all C runtime libraries. 
However, runtime libraries in Visual C++ 2010 no longer use this technology; 
instead, they include the version number of a DLL in its file name, which means 
that different versions of one DLL will technically be completely different 
DLLs now.


Any comments on these?  If there's no strong objection, I think I'll submit a 
documentation patch in the future.

Regards
Takayuki Tsunakawa



Hi,
on cygwin the postgresql binary package already include
the library versions:

  /usr/bin/cygecpg-6.dll
  /usr/bin/cygecpg_compat-3.dll
  /usr/bin/cygpgtypes-3.dll
  /usr/bin/cygpq-5.dll

attached the patch used for the build.

Regards
Marco


--- origsrc/postgresql-9.4.2/src/Makefile.shlib 2015-05-20 00:33:58.0 
+0200
+++ src/Makefile.shlib  2015-05-27 23:01:09.379468300 +0200
@@ -267,7 +267,7 @@ endif
 ifeq ($(PORTNAME), cygwin)
   LINK.shared  = $(CC) -shared
   ifdef SO_MAJOR_VERSION
-shlib  = cyg$(NAME)$(DLSUFFIX)
+shlib  = cyg$(NAME)-$(SO_MAJOR_VERSION)$(DLSUFFIX)
   endif
   haslibarule   = yes
 endif
@@ -359,12 +359,9 @@ ifeq ($(PORTNAME), cygwin)
 # Cygwin case
 
 $(shlib): $(OBJS) | $(SHLIB_PREREQS)
-   $(CC) $(CFLAGS)  -shared -o $@  $(OBJS) $(LDFLAGS) $(LDFLAGS_SL) 
$(SHLIB_LINK) $(LIBS) $(LDAP_LIBS_BE)
+   $(CC) $(CFLAGS)  -shared -o $@ -Wl,--out-implib=$(stlib) $(OBJS) 
$(LDFLAGS) $(LDFLAGS_SL) $(SHLIB_LINK) $(LIBS) $(LDAP_LIBS_BE)
 
-$(stlib): $(OBJS) | $(SHLIB_PREREQS)
-   rm -f $@
-   $(LINK.static) $@ $^
-   $(RANLIB) $@
+$(stlib): $(shlib) ;
 
 else
 
--- origsrc/postgresql-9.4.2/src/interfaces/libpq/Makefile  2015-05-20 
00:33:58.0 +0200
+++ src/interfaces/libpq/Makefile   2015-05-27 22:56:43.193200600 +0200
@@ -45,7 +45,7 @@ OBJS += ip.o md5.o
 OBJS += encnames.o wchar.o
 
 ifeq ($(PORTNAME), cygwin)
-override shlib = cyg$(NAME)$(DLSUFFIX)
+override shlib = cyg$(NAME)-$(SO_MAJOR_VERSION)$(DLSUFFIX)
 endif
 
 ifeq ($(PORTNAME), win32)

-- 
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] Binary I/O for isn extension

[Shay Rojansky]

>
> When adding new functions to an extension you need to bump the version of
> the extension by renaming the file, updating the .control file, creating an
> upgrade script, and updating the Makefile to include the new files.


Thanks for the guidance, I'll fix all that and resubmit a patch.

Re: [HACKERS] Binary I/O for isn extension

[Shay Rojansky]

>
> I added this patch to the next CF (2016-09) under "Miscellaneous".
>

Thanks!


> Out of curiosity, what is the motivation?


I'm the owner of Npgsql, the open-source .NET driver for PostgreSQL, which
is a binary-first driver. That is, working with types that have no binary
I/O is possible but awkward.

I hope the general direction is (or will be) to have binary I/O for all
supported types, both for compatibility with binary-first consumers such as
Npgsql and for general efficiency.

Re: [HACKERS] Binary I/O for isn extension

[Andreas Karlsson]

Hi,

Thanks for the patch, you can add it to our commitfest app so it gets 
reviewed in the next commitfest.


https://commitfest.postgresql.org/

A suggestion to make it easier for your patch to be accepted:

When adding new functions to an extension you need to bump the version 
of the extension by renaming the file, updating the .control file, 
creating an upgrade script, and updating the Makefile to include the new 
files.


Andreas


--
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] IPv6 link-local addresses and init data type

[Andreas Karlsson]

On 05/31/2016 04:06 AM, Tom Lane wrote:

Andreas Karlsson  writes:

On 05/31/2016 02:37 AM, Haribabu Kommi wrote:

The % delimiter character is not only used at the end of the IPV6 address,
from the RFC document, it is possible as follows also.

fe80::%2/64

we need to handle both the scenarios,  it may not be a straight
forward to store the zone id data.



This case needs to be handled by the parser for at least the cidr type,
but I do not think it would make parsing any trickier.


Unless there's a semantic difference between fe80::1%2/64 and
fe80::1/64%2, this doesn't seem like a big deal to me.


As far as I can till only fe80::1%2/64 is valid, but I am not 100% sure.

Andreas


--
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] Binary I/O for isn extension

[Fabien COELHO]

Thanks for the patch, you can add it to our commitfest app so it gets 
reviewed in the next commitfest.


I did this.


A suggestion to make it easier for your patch to be accepted:

When adding new functions to an extension you need to bump the version of the 
extension by renaming the file, updating the .control file, creating an 
upgrade script, and updating the Makefile to include the new files.


Indeed.

Moreover I'm not sure that a type may me updated incrementaly to add new 
send/receive functions.


--
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] Question and suggestion about application binary compatibility policy

[Michael Meskes]

> I couldn't find appropriate system documentation.  Regarding Linux, I
> remember I saw some HOWTO on tldp.org website which explains the
> concept of shared library soname, but it's not very friendly for
> users who just want to know the application binary compatibility
> policy of PostgreSQL.  And I don't think there's suitable 

I would expect Unix sysadmins to understand that howto, but there are
others, e.g. a random hit from google: https://www.bottomupcs.com/libra
ries_and_the_linker.xhtml

There even is a wikipedia page about it: https://en.wikipedia.org/wiki/
Soname

> documentation on Windows.  Even if there is any, users will not be
> sure whether PostgreSQL follows those platform-specific
> conventions.  They may have doubts about it, because even the product
> version "PostgreSQL x.y.z" causes misconception that x is the major
> version and y is the minor one.

I don't know anything about the Windows setup in PostgreSQL, but I
would find it fair to assume that PostgreSQL follows conventions.

> BTW, although this may be a separate topic, it may be better that we
> add the major version in the library names like libpq5.dll and
> libecpg6.dll, so that the application can fail to run with the
> incompatible versions of libpq and libecpg.  FYI:

Does this mean you cannot have to versions of libpq installed on the
same Windows system at the same time?

Michael
-- 
Michael Meskes
Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org)
Meskes at (Debian|Postgresql) dot Org
Jabber: michael at xmpp dot meskes dot org
VfL Borussia! Força Barça! SF 49ers! Use Debian GNU/Linux, PostgreSQL


-- 
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] Binary I/O for isn extension

[Fabien COELHO]

Hello,

Attached is a small patch which adds binary input/output for the types 
added by the isn extension.


I added this patch to the next CF (2016-09) under "Miscellaneous".

Out of curiosity, what is the motivation?

--
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] Re: PATCH: Split stats file per database WAS: autovacuum stress-testing our system

[Tomas Vondra]

Hi,

On 05/26/2016 10:10 PM, Tom Lane wrote:

Tomas Vondra  writes:

Attached is a patch that should fix the coalescing, including the clock
skew detection. In the end I reorganized the code a bit, moving the
check at the end, after the clock skew detection. Otherwise I'd have to
do the clock skew detection on multiple places, and that seemed ugly.


I hadn't been paying any attention to this thread, I must confess.
But I rediscovered this no-coalescing problem while pursuing the poor
behavior for shared catalogs that Peter complained of in
https://www.postgresql.org/message-id/56ad41ac.1030...@gmx.net

I posted a patch at
https://www.postgresql.org/message-id/13023.1464213...@sss.pgh.pa.us
which I think is functionally equivalent to what you have here, but
it goes to some lengths to make the code more readable, whereas this
is just adding another layer of complication to something that's
already a mess (eg, the request_time field is quite useless as-is).
So I'd like to propose pushing that in place of this patch ... do you
care to review it first?


I've reviewed the patch today, and it seems fine to me - correct and 
achieving the same goal as the patch posted to this thread (plus fixing 
the issue with shared catalogs and fixing many comments).


FWIW do you still plan to back-patch this? Minimizing the amount of 
changes was one of the things I had in mind when writing "my" patch, 
which is why I ended up with parts that are less readable.


The one change I'm not quite sure about is the removal of clock skew 
detection in pgstat_recv_inquiry(). You've removed the first check on 
the inquiry, replacing it with this comment:


It seems sufficient to check for clock skew once per write round.

But the first check was comparing msg/req, while the second check looks 
at dbentry/cur_ts. I don't see how those two clock skew check are 
redundant - if they are, the comment should explain that I guess.


Another thing is that if you believe merging requests across databases 
is a silly idea, maybe we should bite the bullet and replace the list of 
requests with a single item. I'm not convinced about this, though.


regards

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

[HACKERS] Logical replication & oldest XID.

[Konstantin Knizhnik]

Hi,

We are using logical replication in multimaster and are faced with some 
interesting problem with "frozen" procArray->replication_slot_xmin.
This variable is adjusted by ProcArraySetReplicationSlotXmin which is 
invoked by ReplicationSlotsComputeRequiredXmin, which
is in turn is called by LogicalConfirmReceivedLocation. If transactions 
are executed at all nodes of multimaster, then everything works fine: 
replication_slot_xmin is advanced. But if we send transactions only to 
one multimaster node and broadcast this changes to other nodes, then no 
data is send through replications slot at this nodes. No data sends - no 
confirmations, LogicalConfirmReceivedLocation is not called and 
procArray->replication_slot_xmin preserves original value 599.


As a result GetOldestXmin function always returns 599, so autovacuum is 
actually blocked and our multimaster is not able to perform cleanup of 
XID->CSN map, which cause shared memory overflow. This situation happens 
only when write transactions are sent only to one node or if there are 
no write transactions at all.


Before implementing some workaround (for example forces all of 
ReplicationSlotsComputeRequiredXmin), I want to understand if it is real 
problem of logical replication or we are doing something wrong? BDR 
should be faced with the same problem if all updates are performed from 
one node...


--
Konstantin Knizhnik
Postgres Professional: http://www.postgrespro.com
The Russian Postgres 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] Rename max_parallel_degree?

[Robert Haas]

On Sun, May 29, 2016 at 1:33 AM, Noah Misch  wrote:
> On Fri, May 06, 2016 at 02:52:30PM -0400, Robert Haas wrote:
>> OK, my reading of this thread is that there is a consensus is to
>> redefine max_parallel_degree=1 as "no parallelism" and
>> max_parallel_degree>1 as "parallelism using a leader plus N-1
>> workers", and along with that, to keep the names unchanged.  However,
>> I don't think I can get that done before beta1, at least not without a
>> serious risk of breaking stuff.  I can look at this post-beta1.
>
> [This is a generic notification.]
>
> The above-described topic is currently a PostgreSQL 9.6 open item.  Robert,
> 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
> 9.6 open item, please let us know.  Otherwise, please observe the policy on
> open item ownership[1] and send a status update within 72 hours 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 9.6rc1.  Consequently, I will appreciate your
> efforts toward speedy resolution.  Thanks.

Here is a patch.  Note that I still don't agree with this change, but
I'm bowing to the will of the group.

I think that some of the people who were in favor of this change
should review this patch, including especially the language I wrote
for the documentation.  If that happens, and the reviews are positive,
then I will commit this.  If that does not happen, then I will
interpret that to mean that there isn't actually all that much
interest in changing this after all and will accordingly recommend
that this open item be removed without further action.

Here is a test which shows how it works:

rhaas=# set max_parallel_degree = 100;
SET
rhaas=# alter table pgbench_accounts set (parallel_degree = 10);
ALTER TABLE
rhaas=# explain (analyze) select count(*) from pgbench_accounts;

QUERY PLAN

 Finalize Aggregate  (cost=177436.04..177436.05 rows=1 width=8)
(actual time=383.244..383.244 rows=1 loops=1)
   ->  Gather  (cost=177435.00..177436.01 rows=10 width=8) (actual
time=383.040..383.237 rows=9 loops=1)
 Workers Planned: 9
 Workers Launched: 8
 ->  Partial Aggregate  (cost=176435.00..176435.01 rows=1
width=8) (actual time=375.569..375.569 rows=1 loops=9)
   ->  Parallel Seq Scan on pgbench_accounts
(cost=0.00..173935.00 rows=100 width=0) (actual
time=0.103..260.352 rows=111 loops=9)
 Planning time: 0.135 ms
 Execution time: 384.387 ms
(8 rows)

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


parallel-degree-1based.patch
Description: binary/octet-stream

-- 
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] Statement timeout

[Tom Lane]

Tatsuo Ishii  writes:
>> Oops. Previous example was not appropriate. Here are revised
>> examples. (in any case, the time consumed at parse and bind are small,
>> and I omit the CHECK_FOR_INTERRUPTS after these commands)

FWIW, I think the existing behavior is just fine.  It corresponds to what
PQexec has always done with multi-statement query strings; that is,
statement_timeout governs the total time to execute the transaction (the
whole query string, unless you put transaction control commands in there).
In extended query mode, since you can only put one textual query per Parse
message, that maps to statement_timeout governing the total time from
initial Parse to Sync.  Which is what it does.

What you propose here is not a bug fix but a redefinition of what
statement_timeout does; and you've made it inconsistent with simple query
mode.  I don't really think it's an improvement.

BTW, aren't you missing a re-enable of the timeout for statements after
the first one?

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] Re: [COMMITTERS] pgsql: Avoid extra locks in GetSnapshotData if old_snapshot_threshold

[Kevin Grittner]

On Tue, May 31, 2016 at 10:03 AM, Robert Haas  wrote:
> On Fri, May 27, 2016 at 10:58 AM, Kevin Grittner  wrote:
>>> As far as I can see normal index builds will allow concurrent hot
>>> prunes and everything; since those only require page-level
>>> exclusive locks.
>>>
>>> So for !concurrent builds we might end up with a corrupt index.
>>
>> ... by which you mean an index that omits certainly-dead heap
>> tuples which have been the subject of early pruning or vacuum, even
>> if there are still registered snapshots that include those tuples?
>> Or do you see something else?
>
> I think that is the danger.

Well, the *perceived* danger -- since every page in the new index
would be new enough to be recognized as too recently modified to be
safe for a snapshot that could still see the omitted rows, the only
question I'm sorting out on this is how safe it might be to cause
the index to be ignored in planning when using such a snapshot.
That and demonstrating the safe behavior to those not following
closely enough to see what will happen without a demonstration.

>> Again, since both the heap pages involved and all the new index
>> pages would have LSNs recent enough to trigger the old snapshot
>> check, I'm not sure where the problem is,

> This is a good point, though, I think.

The whole perception of risk in this area seems to be based on
getting that wrong; although the review of this area may yield some
benefit in terms of minimizing false positives.

>>> I think many of the places relying on heap scans with !mvcc
>>> snapshots are problematic in that way. Outdated reads will not be
>>> detected by TestForOldSnapshot() (given the (snapshot)->satisfies
>>> == HeapTupleSatisfiesMVCC condition therein), and rely on the
>>> snapshot to be actually working.  E.g. CLUSTER/ VACUUM FULL rely
>>> on accurate HEAPTUPLE_RECENTLY_DEAD
>>
>> Don't the "RECENTLY" values imply that the transaction is still
>> running which cause the tuple to be dead?  Since tuples are not
>> subject to early pruning or vacuum when that is the case, where do
>> you see a problem?  The snapshot itself has the usual xmin et al.,
>> so I'm not sure what you might mean by "the snapshot to be actually
>> working" if not the override at the time of pruning/vacuuming.
>
> Anybody who calls HeapTupleSatisfiesVacuum() with an xmin value that
> is newer that the oldest registered snapshot in the system (based on
> some snapshots being ignored) might get a return value of
> HEAPTUPLE_DEAD rather than HEAPTUPLE_RECENTLY_DEAD.

Since the override xmin cannot advance past the earliest
transaction which is still active, HEAPTUPLE_DEAD indicates that
the transaction causing the tuple to be dead has completed and the
tuple is irrevocably dead -- even if there are still snapshots
registered which can see it.  Seeing HEAPTUPLE_DEAD rather than
HEAPTUPLE_RECENTLY_DEAD is strictly limited to tuples which are
certainly and permanently dead and for which the only possible
references are non-MVCC snapshots or existing snapshots subject to
"snapshot too old" monitoring.

> IndexBuildHeapRangeScan(): We might end up with indexIt = false
> instead of indexIt = true.  That should be OK because anyone using the
> old snapshot will see a new page LSN and error out.  We might also
> fail to set indexInfo->ii_BrokenHotChain = true.  I suspect that's a
> problem, but I'm not certain of it.

The latter flag is what I'm currently digging at; but my hope is
that whenever old_snapshot_threshold >= 0 we can set
indexInfo->ii_BrokenHotChain = true to cause the planner to skip
consideration of the index if the snapshot is an "old" one.  That
will avoid some false positives (seeing the error when not strictly
necessary).  If that works out the way I hope, the only down side
is that a scan using a snapshot from an old transaction or cursor
would use some other index or a heap scan; but we already have that
possibility in some cases -- that seems to be the point of the
flag.

> CheckForSerializableConflictOut: Maybe a problem?  If the tuple is
> dead, there's no issue, but if it's recently-dead, there might be.

If the tuple is not visible to the scan, the behavior is unchanged
(a simple return from the function on either HEAPTUPLE_DEAD or
HEAPTUPLE_RECENTLY_DEAD with !visible) and (thus) clearly correct.
If the tuple is visible to us it is currently subject to early
pruning or vacuum (since those operations would get the same
modified xmin) but has not yet had any such treatment since we made
it to this function in the first place.  The processing for SSI
purposes would be unaffected by the possibility that there could
later be early pruning/vacuuming.

Thanks for the review and feedback!

--
Kevin Grittner
EDB: 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] Suggestion for --truncate-tables to pg_restore

[Hendrik Visage]

Hi there,

 Refering to https://www.postgresql.org/message-id/1352742344.21373.4@mofo


 I'm running into situations where I'd need to bulk transfer of data
tables across servers, but a drop and recreate schema isn't feasible
as we are running different permissions etc. on the two databases.

Thus my question: Anybody working on getting this into pg_restore
proper, and any advice on getting this feature incorporated?

HEndrik


-- 
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 pg_dump's error reporting doesn't work worth squat

[Tom Lane]

Kyotaro HORIGUCHI  writes:
> At Fri, 27 May 2016 13:20:20 -0400, Tom Lane  wrote in 
> <14603.1464369...@sss.pgh.pa.us>
>> Kyotaro HORIGUCHI  writes:
>>> By the way, the reason of the "invalid snapshot identifier" is
>>> that some worker threads try to use it after the connection on
>>> the first worker closed.

>> ... BTW, I don't quite see what the issue is there.

> The master session died from lack of libz and the failure of
> compressLevel's propagation already fixed. Some of the children
> that started transactions after the master's death will get the
> error.

I don't think I believe that theory, because it would require the master
to not notice the lack of libz before it launches worker processes, but
instead while the workers are working.  But AFAICS, while there are worker
processes open, the master does nothing except wait for workers and
dispatch new jobs to them; it does no database work of its own.  So the
libz-isn't-there error has to have occurred in one of the workers.

> If we want prevent it perfectly, one solution could be that
> non-master children explicitly wait the master to arrive at the
> "safe" state before starting their transactions. But I suppose it
> is not needed here.

Actually, I believe the problem is in archive_close_connection, around
line 295 in HEAD: once the master realizes that one child has failed,
it first closes its own database connection and only second tries to kill
the remaining children.  So there's a race condition wherein remaining
children have time to see the missing-snapshot error.

In the patch I posted yesterday, I reversed the order of those two
steps, which should fix this problem in most scenarios:
https://www.postgresql.org/message-id/7005.1464657...@sss.pgh.pa.us

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] [PATCH][Documination] Add optional USING keyword before opclass name in INSERT statemet

[Robert Haas]

On Thu, May 26, 2016 at 3:28 PM, Tom Lane  wrote:
> Nikolay Shaplov  writes:
>> Actually I did not expected any discussion for this case. Documentations
>> missed an optional keyword, documentation should be fixed.
>
> 99% of the time, you'd be right.  But this is an unusual case, for the
> reasons I mentioned before.

I tend to agree with Nikolay.  I can't see much upside in making this
change.  At best, nothing will break.  At worst, something will break.
But how do we actually come out ahead?  The only thing you've offered
is that it might make it easier to make the relevant documentation
pages 100% clear, but I feel like a man of your elocution can probably
surmount that impediment.  I guess we might save a few parser states,
too.

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