Re: [HACKERS] Race conditions with WAL sender PID lookups

On Tue, Jul 4, 2017 at 1:34 PM, Noah Misch wrote: > Bundling code cleanup into commits that also do something else is strictly > worse than bundling whitespace cleanup, which is itself bad: > https://postgr.es/m/flat/20160113144826.gb3379...@tornado.leadboat.com FWIW, I agree

Re: [HACKERS] Race conditions with WAL sender PID lookups

On Mon, Jul 03, 2017 at 12:14:55PM -0400, Alvaro Herrera wrote: > Michael Paquier wrote: > > > In passing, clean up some leftover braces which were used to create > > > unconditional blocks. Once upon a time these were used for > > > volatile-izing accesses to those shmem structs, which is no

Re: [HACKERS] Race conditions with WAL sender PID lookups

Michael Paquier wrote: > Thanks Álvaro for pushing the patch. I had a second look and the > result looks good to me. > > - SpinLockAcquire(>mutex); > + } > + pid = walsnd->pid; > The WAL receiver code used a cast to (int) in > pg_stat_get_wal_receiver(). I don't recall adding

Re: [HACKERS] Race conditions with WAL sender PID lookups

On Sat, Jul 1, 2017 at 7:20 AM, Alvaro Herrera wrote: > Michael Paquier wrote: >> Considering how crazy the conditions to make the information fetched >> by users inconsistent are met, I agree with that. > > Pushed. Thanks Álvaro for pushing the patch. I had a second

Re: [HACKERS] Race conditions with WAL sender PID lookups

Michael Paquier wrote: > On Thu, Jun 29, 2017 at 7:52 AM, Alvaro Herrera > wrote: > > I think I'm done with the walsender half of this patch; I still need to > > review the walreceiver part. I will report back tomorrow 19:00 CLT. > > Thanks! > > > Currently, I'm

Re: [HACKERS] Race conditions with WAL sender PID lookups

Noah Misch wrote: > On Wed, Jun 28, 2017 at 06:52:14PM -0400, Alvaro Herrera wrote: > > I think I'm done with the walsender half of this patch; I still need to > > review the walreceiver part. I will report back tomorrow 19:00 CLT. > > This PostgreSQL 10 open item is past due for your status

Re: [HACKERS] Race conditions with WAL sender PID lookups

On Wed, Jun 28, 2017 at 06:52:14PM -0400, Alvaro Herrera wrote: > I think I'm done with the walsender half of this patch; I still need to > review the walreceiver part. I will report back tomorrow 19:00 CLT. This PostgreSQL 10 open item is past due for your status update. Kindly send a status

Re: [HACKERS] Race conditions with WAL sender PID lookups

On Thu, Jun 29, 2017 at 7:52 AM, Alvaro Herrera wrote: > Michael Paquier wrote: >> On Thu, Jun 8, 2017 at 3:31 AM, Robert Haas wrote: >> I take that you are referring to the two lookups in >> WalSndWaitForWal(), one in exec_replication_command(),

Re: [HACKERS] Race conditions with WAL sender PID lookups

Michael Paquier wrote: > On Thu, Jun 8, 2017 at 3:31 AM, Robert Haas wrote: > > I think if you're going to fix it so that we take spinlocks on > > MyWalSnd in a bunch of places that we didn't take them before, it > > would make sense to fix all the places where we're

Re: [HACKERS] Race conditions with WAL sender PID lookups

On Sun, Jun 25, 2017 at 2:11 PM, Alvaro Herrera wrote: > Noah Misch wrote: > >> IMMEDIATE ATTENTION REQUIRED. This PostgreSQL 10 open item is long past due >> for your status update. Please reacquaint yourself with the policy on open >> item ownership[1] and then reply

Re: [HACKERS] Race conditions with WAL sender PID lookups

Noah Misch wrote: > IMMEDIATE ATTENTION REQUIRED. This PostgreSQL 10 open item is long past due > for your status update. Please reacquaint yourself with the policy on open > item ownership[1] and then reply immediately. If I do not hear from you by > 2017-06-25 06:00 UTC, I will transfer this

Re: [HACKERS] Race conditions with WAL sender PID lookups

On Wed, Jun 21, 2017 at 10:52:50PM -0700, Noah Misch wrote: > On Thu, Jun 15, 2017 at 09:04:44AM +0100, Simon Riggs wrote: > > On 15 June 2017 at 02:59, Noah Misch wrote: > > > > > Formally, the causative commit is the one that removed the superuser() > > > test, > > > namely

Re: [HACKERS] Race conditions with WAL sender PID lookups

On Thu, Jun 15, 2017 at 09:04:44AM +0100, Simon Riggs wrote: > On 15 June 2017 at 02:59, Noah Misch wrote: > > > Formally, the causative commit is the one that removed the superuser() test, > > namely 25fff40. > > > > [Action required within three days. This is a generic

Re: [HACKERS] Race conditions with WAL sender PID lookups

On 15 June 2017 at 02:59, Noah Misch wrote: > Formally, the causative commit is the one that removed the superuser() test, > namely 25fff40. > > [Action required within three days. This is a generic notification.] > > The above-described topic is currently a PostgreSQL 10

Re: [HACKERS] Race conditions with WAL sender PID lookups

Hi, At Thu, 8 Jun 2017 13:15:02 +0900, Michael Paquier wrote in > On Thu, Jun 8, 2017 at 3:31 AM, Robert Haas wrote: > > I think if you're going to fix it so that we take

Re: [HACKERS] Race conditions with WAL sender PID lookups

On Tue, Jun 13, 2017 at 11:16:54AM +0900, Michael Paquier wrote: > On Thu, Jun 8, 2017 at 1:15 PM, Michael Paquier > wrote: > > 3) In walreceiver.c's pg_stat_get_wal_receiver's: > > - Launch pg_stat_get_wal_receiver and take a checkpoint on it. > > - Pass the lookups of

Re: [HACKERS] Race conditions with WAL sender PID lookups

On Thu, Jun 8, 2017 at 1:15 PM, Michael Paquier wrote: > 3) In walreceiver.c's pg_stat_get_wal_receiver's: > - Launch pg_stat_get_wal_receiver and take a checkpoint on it. > - Pass the lookups of pid and ready_to_display. > - Make the WAL receiver stop. > - The view

Re: [HACKERS] Race conditions with WAL sender PID lookups

v On Thu, Jun 8, 2017 at 3:31 AM, Robert Haas wrote: > I think if you're going to fix it so that we take spinlocks on > MyWalSnd in a bunch of places that we didn't take them before, it > would make sense to fix all the places where we're accessing those > fields without a

Re: [HACKERS] Race conditions with WAL sender PID lookups

On 2017-06-07 20:31, Robert Haas wrote: [...] [ Side note: Erik's report on this thread initially seemed to suggest that we needed this patch to make logical decoding stable. But my impression is that this is belied by subsequent developments on other threads, so my theory is that this patch

Re: [HACKERS] Race conditions with WAL sender PID lookups

On Sat, May 20, 2017 at 8:40 AM, Michael Paquier wrote: > On Fri, May 19, 2017 at 3:01 PM, Masahiko Sawada > wrote: >> Also, as Horiguchi-san pointed out earlier, walreceiver seems need the >> similar fix. > > Actually, now that I look at it,

Re: [HACKERS] Race conditions with WAL sender PID lookups

On Fri, Jun 2, 2017 at 2:21 PM, Noah Misch wrote: > On Tue, May 30, 2017 at 11:13:37AM -0700, Michael Paquier wrote: >> On Tue, May 30, 2017 at 11:09 AM, Peter Eisentraut >> wrote: >> > I don't think this is my item. Most of the behavior is

Re: [HACKERS] Race conditions with WAL sender PID lookups

On Sun, May 21, 2017 at 08:19:34AM +0200, Erik Rijkers wrote: > On 2017-05-21 06:37, Erik Rijkers wrote: > >With this patch on current master my logical replication tests > >(pgbench-over-logical-replication) run without errors for the first > >time in many days (even weeks). > > Unfortunately,

Re: [HACKERS] Race conditions with WAL sender PID lookups

On Fri, May 19, 2017 at 2:05 PM, Michael Paquier wrote: > On Thu, May 18, 2017 at 1:43 PM, Thomas Munro > wrote: >> On Thu, May 11, 2017 at 1:48 PM, Michael Paquier >> wrote: >>> I had my eyes on the WAL sender

Re: [HACKERS] Race conditions with WAL sender PID lookups

On Tue, May 30, 2017 at 11:09 AM, Peter Eisentraut wrote: > I don't think this is my item. Most of the behavior is old, and > pg_stat_get_wal_receiver() is from commit > b1a9bad9e744857291c7d5516080527da8219854. > > I would appreciate if another committer can

Re: [HACKERS] Race conditions with WAL sender PID lookups

On 5/29/17 21:38, Noah Misch wrote: >> Actually, now that I look at it, ready_to_display should as well be >> protected by the lock of the WAL receiver, so it is incorrectly placed >> in walreceiver.h. As you are pointing out, pg_stat_get_wal_receiver() >> is lazy as well, and that's new in 10, so

Re: [HACKERS] Race conditions with WAL sender PID lookups

On Sat, May 20, 2017 at 09:40:57PM +0900, Michael Paquier wrote: > On Fri, May 19, 2017 at 3:01 PM, Masahiko Sawada > wrote: > > Also, as Horiguchi-san pointed out earlier, walreceiver seems need the > > similar fix. > > Actually, now that I look at it, ready_to_display

Re: [HACKERS] Race conditions with WAL sender PID lookups

On 2017-05-21 06:37, Erik Rijkers wrote: On 2017-05-20 14:40, Michael Paquier wrote: On Fri, May 19, 2017 at 3:01 PM, Masahiko Sawada wrote: Also, as Horiguchi-san pointed out earlier, walreceiver seems need the similar fix. Actually, now that I look at it,

Re: [HACKERS] Race conditions with WAL sender PID lookups

On 2017-05-20 14:40, Michael Paquier wrote: On Fri, May 19, 2017 at 3:01 PM, Masahiko Sawada wrote: Also, as Horiguchi-san pointed out earlier, walreceiver seems need the similar fix. Actually, now that I look at it, ready_to_display should as well be protected by the

Re: [HACKERS] Race conditions with WAL sender PID lookups

On Fri, May 19, 2017 at 3:01 PM, Masahiko Sawada wrote: > Also, as Horiguchi-san pointed out earlier, walreceiver seems need the > similar fix. Actually, now that I look at it, ready_to_display should as well be protected by the lock of the WAL receiver, so it is

Re: [HACKERS] Race conditions with WAL sender PID lookups

On Fri, May 19, 2017 at 11:05 AM, Michael Paquier wrote: > On Thu, May 18, 2017 at 1:43 PM, Thomas Munro > wrote: >> On Thu, May 11, 2017 at 1:48 PM, Michael Paquier >> wrote: >>> I had my eyes on the WAL

Re: [HACKERS] Race conditions with WAL sender PID lookups

On Thu, May 18, 2017 at 1:43 PM, Thomas Munro wrote: > On Thu, May 11, 2017 at 1:48 PM, Michael Paquier > wrote: >> I had my eyes on the WAL sender code this morning, and I have noticed >> that walsender.c is not completely consistent

Re: [HACKERS] Race conditions with WAL sender PID lookups

On Thu, May 11, 2017 at 1:48 PM, Michael Paquier wrote: > I had my eyes on the WAL sender code this morning, and I have noticed > that walsender.c is not completely consistent with the PID lookups it > does in walsender.c. In two code paths, the PID value is checked >

Re: [HACKERS] Race conditions with WAL sender PID lookups

On Wed, May 17, 2017 at 12:14 AM, Robert Haas wrote: > Well, it's probably worth changing for consistency, but I'm not sure > that it rises to the level of "a very bad idea". It actually seems > almost entirely harmless. Spuriously setting the needreload flag on a >

Re: [HACKERS] Race conditions with WAL sender PID lookups

On Wed, May 10, 2017 at 9:48 PM, Michael Paquier wrote: > I had my eyes on the WAL sender code this morning, and I have noticed > that walsender.c is not completely consistent with the PID lookups it > does in walsender.c. In two code paths, the PID value is checked >

Re: [HACKERS] Race conditions with WAL sender PID lookups

At Fri, 12 May 2017 11:44:19 +0900, Masahiko Sawada wrote in

Re: [HACKERS] Race conditions with WAL sender PID lookups

On Thu, May 11, 2017 at 10:48 AM, Michael Paquier wrote: > Hi all, > > I had my eyes on the WAL sender code this morning, and I have noticed > that walsender.c is not completely consistent with the PID lookups it > does in walsender.c. In two code paths, the PID value

[HACKERS] Race conditions with WAL sender PID lookups

Hi all, I had my eyes on the WAL sender code this morning, and I have noticed that walsender.c is not completely consistent with the PID lookups it does in walsender.c. In two code paths, the PID value is checked without holding the WAL sender spin lock (WalSndRqstFileReload and