Re: [COMMITTERS] pgsql: Mark pg_start_backup and pg_stop_backup as parallel-restricted.

2017-03-06 Thread Robert Haas
On Mon, Mar 6, 2017 at 3:23 PM, Tom Lane  wrote:
> Yes, I think it's rather silly not to do so.  We have made comparable
> backpatched fixes multiple times in the past.  What is worth discussing is
> whether there are *additional* things we ought to do in 9.6 to prevent
> misbehavior in installations initdb'd pre-9.6.3.
>
> If there's a cheap way of testing "AmInParallelWorker", I'd be in favor of
> adding a quick-n-dirty test and ereport(ERROR) to these functions in the
> 9.6 branch, so that at least you get a clean error and not some weird
> misbehavior.  Not sure if there's anything more we can do than that.

Sounds like you want IsParallelWorker().

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


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


Re: [COMMITTERS] pgsql: Mark pg_start_backup and pg_stop_backup as parallel-restricted.

2017-03-06 Thread Stephen Frost
Tom,

* Tom Lane (t...@sss.pgh.pa.us) wrote:
> David Steele  writes:
> > On 3/6/17 12:48 PM, Robert Haas wrote:
> >> This issue also exists in 9.6, but we obviously can't do anything
> >> about 9.6 clusters that already exist.  Possibly this could be
> >> back-patched so that future 9.6 clusters would come out OK, or
> >> possibly we should back-patch some other fix, but that would need more
> >> discussion.
> 
> > I think it would be worth back-patching the catalog fix for future 9.6 
> > clusters as a start.
> 
> Yes, I think it's rather silly not to do so.  We have made comparable
> backpatched fixes multiple times in the past.  What is worth discussing is
> whether there are *additional* things we ought to do in 9.6 to prevent
> misbehavior in installations initdb'd pre-9.6.3.
> 
> If there's a cheap way of testing "AmInParallelWorker", I'd be in favor of
> adding a quick-n-dirty test and ereport(ERROR) to these functions in the
> 9.6 branch, so that at least you get a clean error and not some weird
> misbehavior.  Not sure if there's anything more we can do than that.

That's more-or-less what I was thinking (and suggested to David over IM
a little while ago, actually).  I don't know if there's an easy way to
do such a check, but I don't think it would really need to be
particularly cheap, just not overly complex.  These code paths are
certainly not ones that need to be high-performance.

Thanks!

Stephen


signature.asc
Description: Digital signature


Re: [COMMITTERS] pgsql: Mark pg_start_backup and pg_stop_backup as parallel-restricted.

2017-03-06 Thread Tom Lane
David Steele  writes:
> On 3/6/17 12:48 PM, Robert Haas wrote:
>> This issue also exists in 9.6, but we obviously can't do anything
>> about 9.6 clusters that already exist.  Possibly this could be
>> back-patched so that future 9.6 clusters would come out OK, or
>> possibly we should back-patch some other fix, but that would need more
>> discussion.

> I think it would be worth back-patching the catalog fix for future 9.6 
> clusters as a start.

Yes, I think it's rather silly not to do so.  We have made comparable
backpatched fixes multiple times in the past.  What is worth discussing is
whether there are *additional* things we ought to do in 9.6 to prevent
misbehavior in installations initdb'd pre-9.6.3.

If there's a cheap way of testing "AmInParallelWorker", I'd be in favor of
adding a quick-n-dirty test and ereport(ERROR) to these functions in the
9.6 branch, so that at least you get a clean error and not some weird
misbehavior.  Not sure if there's anything more we can do than that.

regards, tom lane


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


Re: [COMMITTERS] pgsql: Mark pg_start_backup and pg_stop_backup as parallel-restricted.

2017-03-06 Thread David Steele

On 3/6/17 12:48 PM, Robert Haas wrote:

Mark pg_start_backup and pg_stop_backup as parallel-restricted.

They depend on backend-private state that will not be synchronized by
the parallel machinery, so they should not be marked parallel-safe.
This issue also exists in 9.6, but we obviously can't do anything
about 9.6 clusters that already exist.  Possibly this could be
back-patched so that future 9.6 clusters would come out OK, or
possibly we should back-patch some other fix, but that would need more
discussion.


I think it would be worth back-patching the catalog fix for future 9.6 
clusters as a start.  Parallelism is off by default in 9.6 so that 
mitigates some of the problem.


I don't have any regression tests that cover backups when parallelism is 
enabled in 9.6, but I'm willing to do that and see if this is a 
realistic issue or not.


--
-David
da...@pgmasters.net


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