Re: SyncRepLock acquired exclusively in default configuration

2020-09-01 Thread Andres Freund
On 2020-09-02 10:58:58 +0900, Fujii Masao wrote: > Asim and Sawada-san, thanks for the review! I pushed the patch. Thanks for all your combined work!

Re: SyncRepLock acquired exclusively in default configuration

2020-09-01 Thread Fujii Masao
On 2020/08/28 21:20, Masahiko Sawada wrote: On Fri, 28 Aug 2020 at 10:33, Fujii Masao wrote: On 2020/08/27 15:59, Asim Praveen wrote: On 26-Aug-2020, at 11:10 PM, Fujii Masao wrote: I added the following comments based on the suggestion by Sawada-san upthread. Thought? + * Sinc

Re: SyncRepLock acquired exclusively in default configuration

2020-08-28 Thread Masahiko Sawada
On Fri, 28 Aug 2020 at 10:33, Fujii Masao wrote: > > > > On 2020/08/27 15:59, Asim Praveen wrote: > > > >> On 26-Aug-2020, at 11:10 PM, Fujii Masao > >> wrote: > >> > >> I added the following comments based on the suggestion by Sawada-san > >> upthread. Thought? > >> > >> + * Since this rou

Re: SyncRepLock acquired exclusively in default configuration

2020-08-28 Thread Asim Praveen
> On 28-Aug-2020, at 7:03 AM, Fujii Masao wrote: > > On 2020/08/27 15:59, Asim Praveen wrote: >> >> +1. May I suggest the following addition to the above comment (feel free to >> rephrase / reject)? >> "If sync_standbys_defined was being set from false to true and we observe it >> as >> fals

Re: SyncRepLock acquired exclusively in default configuration

2020-08-27 Thread Fujii Masao
On 2020/08/27 15:59, Asim Praveen wrote: On 26-Aug-2020, at 11:10 PM, Fujii Masao wrote: I added the following comments based on the suggestion by Sawada-san upthread. Thought? +* Since this routine gets called every commit time, it's important to +* exit quickly if sync r

Re: SyncRepLock acquired exclusively in default configuration

2020-08-26 Thread Asim Praveen
> On 26-Aug-2020, at 11:10 PM, Fujii Masao wrote: > > I added the following comments based on the suggestion by Sawada-san > upthread. Thought? > > + * Since this routine gets called every commit time, it's important to > + * exit quickly if sync replication is not requested. So we c

Re: SyncRepLock acquired exclusively in default configuration

2020-08-26 Thread Fujii Masao
On 2020/08/20 11:29, Kyotaro Horiguchi wrote: At Wed, 19 Aug 2020 13:20:46 +, Asim Praveen wrote in On 12-Aug-2020, at 12:02 PM, Masahiko Sawada wrote: On Wed, 12 Aug 2020 at 14:06, Asim Praveen wrote: On 11-Aug-2020, at 8:57 PM, Robert Haas wrote: I think this gets to the r

Re: SyncRepLock acquired exclusively in default configuration

2020-08-20 Thread Masahiko Sawada
On Wed, 19 Aug 2020 at 21:41, Fujii Masao wrote: > > > > On 2020/08/12 15:32, Masahiko Sawada wrote: > > On Wed, 12 Aug 2020 at 14:06, Asim Praveen wrote: > >> > >> > >> > >>> On 11-Aug-2020, at 8:57 PM, Robert Haas wrote: > >>> > >>> I think this gets to the root of the issue. If we check the f

Re: SyncRepLock acquired exclusively in default configuration

2020-08-19 Thread Kyotaro Horiguchi
At Wed, 19 Aug 2020 21:41:03 +0900, Fujii Masao wrote in > > > On 2020/08/12 15:32, Masahiko Sawada wrote: > > On Wed, 12 Aug 2020 at 14:06, Asim Praveen wrote: > >> > >> > >> > >>> On 11-Aug-2020, at 8:57 PM, Robert Haas wrote: > >>> > >>> I think this gets to the root of the issue. If we c

Re: SyncRepLock acquired exclusively in default configuration

2020-08-19 Thread Kyotaro Horiguchi
At Wed, 19 Aug 2020 13:20:46 +, Asim Praveen wrote in > > > > On 12-Aug-2020, at 12:02 PM, Masahiko Sawada > > wrote: > > > > On Wed, 12 Aug 2020 at 14:06, Asim Praveen wrote: > >> > >> > >> > >>> On 11-Aug-2020, at 8:57 PM, Robert Haas wrote: > >>> > >>> I think this gets to the r

Re: SyncRepLock acquired exclusively in default configuration

2020-08-19 Thread Asim Praveen
> On 12-Aug-2020, at 12:02 PM, Masahiko Sawada > wrote: > > On Wed, 12 Aug 2020 at 14:06, Asim Praveen wrote: >> >> >> >>> On 11-Aug-2020, at 8:57 PM, Robert Haas wrote: >>> >>> I think this gets to the root of the issue. If we check the flag >>> without a lock, we might see a slightly s

Re: SyncRepLock acquired exclusively in default configuration

2020-08-19 Thread Fujii Masao
On 2020/08/12 15:32, Masahiko Sawada wrote: On Wed, 12 Aug 2020 at 14:06, Asim Praveen wrote: On 11-Aug-2020, at 8:57 PM, Robert Haas wrote: I think this gets to the root of the issue. If we check the flag without a lock, we might see a slightly stale value. But, considering that there

Re: SyncRepLock acquired exclusively in default configuration

2020-08-11 Thread Masahiko Sawada
On Wed, 12 Aug 2020 at 14:06, Asim Praveen wrote: > > > > > On 11-Aug-2020, at 8:57 PM, Robert Haas wrote: > > > > I think this gets to the root of the issue. If we check the flag > > without a lock, we might see a slightly stale value. But, considering > > that there's no particular amount of ti

Re: SyncRepLock acquired exclusively in default configuration

2020-08-11 Thread Asim Praveen
> On 11-Aug-2020, at 8:57 PM, Robert Haas wrote: > > I think this gets to the root of the issue. If we check the flag > without a lock, we might see a slightly stale value. But, considering > that there's no particular amount of time within which configuration > changes are guaranteed to take e

Re: SyncRepLock acquired exclusively in default configuration

2020-08-11 Thread Robert Haas
On Tue, Aug 11, 2020 at 7:55 AM Asim Praveen wrote: > There is no out-of-order execution hazard in the scenario you are describing, > memory barriers don’t seem to fit. Using locks to synchronise checkpointer > process and a committing backend process is the right way. We have made a > consci

Re: SyncRepLock acquired exclusively in default configuration

2020-08-11 Thread Asim Praveen
The proposed fix looks good, it resolves the lock contention problem as intended. +1 from my side. > On 09-Jul-2020, at 4:22 PM, Fujii Masao wrote: > > > Regarding how to fix, don't we need memory barrier when reading > sync_standbys_defined? Without that, after SyncRepUpdateSyncStandbysDefine

Re: SyncRepLock acquired exclusively in default configuration

2020-07-09 Thread Fujii Masao
On 2020/05/19 11:41, Masahiko Sawada wrote: On Sat, 11 Apr 2020 at 09:30, Masahiko Sawada wrote: On Fri, 10 Apr 2020 at 21:52, Fujii Masao wrote: On 2020/04/10 20:56, Masahiko Sawada wrote: On Fri, 10 Apr 2020 at 18:57, Fujii Masao wrote: On 2020/04/10 14:11, Masahiko Sawada wrot

Re: SyncRepLock acquired exclusively in default configuration

2020-05-19 Thread Michael Paquier
On Tue, May 19, 2020 at 08:56:13AM -0700, Ashwin Agrawal wrote: > Sure, add it to commit fest. > Seems though it should be backpatched to relevant branches as well. It does not seem to be listed yet. Are you planning to add it under the section for bug fixes? -- Michael signature.asc Descriptio

Re: SyncRepLock acquired exclusively in default configuration

2020-05-19 Thread Ashwin Agrawal
On Mon, May 18, 2020 at 7:41 PM Masahiko Sawada < masahiko.saw...@2ndquadrant.com> wrote: > This item is for PG14, right? If so I'd like to add this item to the > next commit fest. > Sure, add it to commit fest. Seems though it should be backpatched to relevant branches as well.

Re: SyncRepLock acquired exclusively in default configuration

2020-05-18 Thread Masahiko Sawada
On Sat, 11 Apr 2020 at 09:30, Masahiko Sawada wrote: > > On Fri, 10 Apr 2020 at 21:52, Fujii Masao wrote: > > > > > > > > On 2020/04/10 20:56, Masahiko Sawada wrote: > > > On Fri, 10 Apr 2020 at 18:57, Fujii Masao > > > wrote: > > >> > > >> > > >> > > >> On 2020/04/10 14:11, Masahiko Sawada wro

Re: SyncRepLock acquired exclusively in default configuration

2020-04-10 Thread Masahiko Sawada
On Fri, 10 Apr 2020 at 21:52, Fujii Masao wrote: > > > > On 2020/04/10 20:56, Masahiko Sawada wrote: > > On Fri, 10 Apr 2020 at 18:57, Fujii Masao > > wrote: > >> > >> > >> > >> On 2020/04/10 14:11, Masahiko Sawada wrote: > >>> On Fri, 10 Apr 2020 at 13:20, Fujii Masao > >>> wrote: > > >>

Re: SyncRepLock acquired exclusively in default configuration

2020-04-10 Thread Fujii Masao
On 2020/04/10 20:56, Masahiko Sawada wrote: On Fri, 10 Apr 2020 at 18:57, Fujii Masao wrote: On 2020/04/10 14:11, Masahiko Sawada wrote: On Fri, 10 Apr 2020 at 13:20, Fujii Masao wrote: On 2020/04/08 3:01, Ashwin Agrawal wrote: On Mon, Apr 6, 2020 at 2:14 PM Andres Freund mailto:a

Re: SyncRepLock acquired exclusively in default configuration

2020-04-10 Thread Masahiko Sawada
On Fri, 10 Apr 2020 at 18:57, Fujii Masao wrote: > > > > On 2020/04/10 14:11, Masahiko Sawada wrote: > > On Fri, 10 Apr 2020 at 13:20, Fujii Masao > > wrote: > >> > >> > >> > >> On 2020/04/08 3:01, Ashwin Agrawal wrote: > >>> > >>> On Mon, Apr 6, 2020 at 2:14 PM Andres Freund >>>

Re: SyncRepLock acquired exclusively in default configuration

2020-04-10 Thread Fujii Masao
On 2020/04/10 14:11, Masahiko Sawada wrote: On Fri, 10 Apr 2020 at 13:20, Fujii Masao wrote: On 2020/04/08 3:01, Ashwin Agrawal wrote: On Mon, Apr 6, 2020 at 2:14 PM Andres Freund mailto:and...@anarazel.de>> wrote: > How about we change it to this ? Hm. Better. But I think

Re: SyncRepLock acquired exclusively in default configuration

2020-04-09 Thread Masahiko Sawada
On Fri, 10 Apr 2020 at 13:20, Fujii Masao wrote: > > > > On 2020/04/08 3:01, Ashwin Agrawal wrote: > > > > On Mon, Apr 6, 2020 at 2:14 PM Andres Freund > > wrote: > > > > > How about we change it to this ? > > > > Hm. Better. But I think it might need at least

Re: SyncRepLock acquired exclusively in default configuration

2020-04-09 Thread Fujii Masao
On 2020/04/08 3:01, Ashwin Agrawal wrote: On Mon, Apr 6, 2020 at 2:14 PM Andres Freund mailto:and...@anarazel.de>> wrote: > How about we change it to this ? Hm. Better. But I think it might need at least a compiler barrier / volatile memory load?  Unlikely here, but otherwise th

Re: SyncRepLock acquired exclusively in default configuration

2020-04-07 Thread Ashwin Agrawal
On Mon, Apr 6, 2020 at 2:14 PM Andres Freund wrote: > > How about we change it to this ? > > Hm. Better. But I think it might need at least a compiler barrier / > volatile memory load? Unlikely here, but otherwise the compiler could > theoretically just stash the variable somewhere locally (it's

Re: SyncRepLock acquired exclusively in default configuration

2020-04-06 Thread Masahiko Sawada
On Tue, 7 Apr 2020 at 06:14, Andres Freund wrote: > > Hi, > > On 2020-04-06 11:51:06 -0700, Ashwin Agrawal wrote: > > On Mon, Apr 6, 2020 at 1:52 AM Masahiko Sawada < > > masahiko.saw...@2ndquadrant.com> wrote: > > > On Mon, 6 Apr 2020 at 14:04, Andres Freund wrote: > > > > I'm really not ok with

Re: SyncRepLock acquired exclusively in default configuration

2020-04-06 Thread Andres Freund
Hi, On 2020-04-06 11:51:06 -0700, Ashwin Agrawal wrote: > On Mon, Apr 6, 2020 at 1:52 AM Masahiko Sawada < > masahiko.saw...@2ndquadrant.com> wrote: > > On Mon, 6 Apr 2020 at 14:04, Andres Freund wrote: > > > I'm really not ok with unneccessarily adding an exclusive lock > > > acquisition to such

Re: SyncRepLock acquired exclusively in default configuration

2020-04-06 Thread Ashwin Agrawal
On Mon, Apr 6, 2020 at 1:52 AM Masahiko Sawada < masahiko.saw...@2ndquadrant.com> wrote: > On Mon, 6 Apr 2020 at 14:04, Andres Freund wrote: > > > > commit 48c9f4926562278a2fd2b85e7486c6d11705f177 > > Author: Simon Riggs > > Date: 2017-12-29 14:30:33 + > > > > Fix race condition when c

Re: SyncRepLock acquired exclusively in default configuration

2020-04-06 Thread Masahiko Sawada
On Mon, 6 Apr 2020 at 14:04, Andres Freund wrote: > > Hi, > > Due to the change below, when using the default postgres configuration > of ynchronous_commit = on, max_wal_senders = 10, will now acquire a new > exclusive lwlock after writing a commit record. Indeed. > > commit 48c9f4926562278a2fd2