Re: [HACKERS] Somebody has not thought through subscription locking considerations

2017-04-10 Thread Peter Eisentraut
On 3/31/17 20:25, Petr Jelinek wrote: > On 01/04/17 01:57, Petr Jelinek wrote: >> That being said, looking at use-cases for SetSubscriptionRelState that's >> basically CREATE SUBSCRIPTION, ALTER SUBSCRIPTION REFRESH and tablesync >> worker. So the DDL thing applies to first ones as well and

Re: [HACKERS] Somebody has not thought through subscription locking considerations

2017-03-31 Thread Petr Jelinek
On 01/04/17 01:57, Petr Jelinek wrote: > On 01/04/17 01:20, Tom Lane wrote: >> Petr Jelinek writes: >>> But the pg_subscription_rel is also not accessed on heap_open, the >>> problematic code is called from heap_drop_with_catalog. And VACUUM FULL >>> pg_class calls

Re: [HACKERS] Somebody has not thought through subscription locking considerations

2017-03-31 Thread Petr Jelinek
On 01/04/17 01:57, Petr Jelinek wrote: > On 01/04/17 01:20, Tom Lane wrote: >> Petr Jelinek writes: >>> But the pg_subscription_rel is also not accessed on heap_open, the >>> problematic code is called from heap_drop_with_catalog. And VACUUM FULL >>> pg_class calls

Re: [HACKERS] Somebody has not thought through subscription locking considerations

2017-03-31 Thread Petr Jelinek
On 01/04/17 01:20, Tom Lane wrote: > Petr Jelinek writes: >> But the pg_subscription_rel is also not accessed on heap_open, the >> problematic code is called from heap_drop_with_catalog. And VACUUM FULL >> pg_class calls heap_drop_with_catalog() when doing the heap

Re: [HACKERS] Somebody has not thought through subscription locking considerations

2017-03-31 Thread Tom Lane
Petr Jelinek writes: > But the pg_subscription_rel is also not accessed on heap_open, the > problematic code is called from heap_drop_with_catalog. And VACUUM FULL > pg_class calls heap_drop_with_catalog() when doing the heap swap (and it > goes through

Re: [HACKERS] Somebody has not thought through subscription locking considerations

2017-03-31 Thread Petr Jelinek
On 01/04/17 00:52, Tom Lane wrote: > Petr Jelinek writes: >> On 31/03/17 21:00, Tom Lane wrote: >>> Looking at dependency info isn't going to fix this, it only moves the >>> unsafe catalog access somewhere else (ie pg_depend instead of >>> pg_subscription_rel). I

Re: [HACKERS] Somebody has not thought through subscription locking considerations

2017-03-31 Thread Tom Lane
Petr Jelinek writes: > On 31/03/17 21:00, Tom Lane wrote: >> Looking at dependency info isn't going to fix this, it only moves the >> unsafe catalog access somewhere else (ie pg_depend instead of >> pg_subscription_rel). I suspect the only safe solution is doing an

Re: [HACKERS] Somebody has not thought through subscription locking considerations

2017-03-31 Thread Petr Jelinek
On 31/03/17 21:00, Tom Lane wrote: > Petr Jelinek writes: >> On 31/03/17 20:23, Tom Lane wrote: >>> No, the problematic part is that there is any heap_open happening at >>> all. That open could very easily result in a recursive attempt to read >>> pg_class, for

Re: [HACKERS] Somebody has not thought through subscription locking considerations

2017-03-31 Thread Tom Lane
Petr Jelinek writes: > On 31/03/17 20:23, Tom Lane wrote: >> No, the problematic part is that there is any heap_open happening at >> all. That open could very easily result in a recursive attempt to read >> pg_class, for example, which is going to be fatal if we're

Re: [HACKERS] Somebody has not thought through subscription locking considerations

2017-03-31 Thread Petr Jelinek
On 31/03/17 20:23, Tom Lane wrote: > Petr Jelinek writes: >> On 31/03/17 19:35, Tom Lane wrote: >>> At the very least, it would be a good idea to exclude the system >>> catalogs from logical replication, wouldn't it? > >> They are excluded. > > Well, the exclusion

Re: [HACKERS] Somebody has not thought through subscription locking considerations

2017-03-31 Thread Tom Lane
Petr Jelinek writes: > On 31/03/17 19:35, Tom Lane wrote: >> At the very least, it would be a good idea to exclude the system >> catalogs from logical replication, wouldn't it? > They are excluded. Well, the exclusion is apparently not checked early enough. I

Re: [HACKERS] Somebody has not thought through subscription locking considerations

2017-03-31 Thread Petr Jelinek
On 31/03/17 19:35, Tom Lane wrote: > Masahiko Sawada writes: >> On Fri, Mar 31, 2017 at 9:53 AM, Petr Jelinek >> wrote: >>> On 30/03/17 07:25, Tom Lane wrote: I await with interest an explanation of what "VACUUM FULL pg_class" is

Re: [HACKERS] Somebody has not thought through subscription locking considerations

2017-03-31 Thread Tom Lane
Masahiko Sawada writes: > On Fri, Mar 31, 2017 at 9:53 AM, Petr Jelinek > wrote: >> On 30/03/17 07:25, Tom Lane wrote: >>> I await with interest an explanation of what "VACUUM FULL pg_class" is >>> doing trying to acquire ShareRowExclusiveLock

Re: [HACKERS] Somebody has not thought through subscription locking considerations

2017-03-31 Thread Masahiko Sawada
On Fri, Mar 31, 2017 at 9:53 AM, Petr Jelinek wrote: > On 30/03/17 07:25, Tom Lane wrote: >> I noticed this failure report: >> https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=dangomushi=2017-03-29%2019%3A45%3A27 >> >> in which we find >> >> *** >>

Re: [HACKERS] Somebody has not thought through subscription locking considerations

2017-03-30 Thread Petr Jelinek
On 30/03/17 07:25, Tom Lane wrote: > I noticed this failure report: > https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=dangomushi=2017-03-29%2019%3A45%3A27 > > in which we find > > *** > /home/buildfarm/data/buildroot/HEAD/pgsql.build/src/test/regress/expected/updatable_views.out >

[HACKERS] Somebody has not thought through subscription locking considerations

2017-03-29 Thread Tom Lane
I noticed this failure report: https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=dangomushi=2017-03-29%2019%3A45%3A27 in which we find *** /home/buildfarm/data/buildroot/HEAD/pgsql.build/src/test/regress/expected/updatable_views.out Thu Mar 30 04:45:43 2017 ---