Re: Race conditions with WAL sender PID lookups

On Sat, Jul 1, 2017 at 7:20 AM, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com> 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 look and the
result looks good to me.

- SpinLockAcquire(&walsnd->mutex);
+ }
+ pid = walsnd->pid;
The WAL receiver code used a cast to (int) in
pg_stat_get_wal_receiver(). I don't recall adding it. Why not being
consistent for both by removing the one of the WAL receiver code?

> 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 longer
> required. Many other occurrences of this pattern remain.

Here are the places where a cleanup can happen:
- WalSndSetState
- ProcessStandbyReplyMessage
- XLogRead, 2 places
- XLogSendLogical
- WalRcvWaitForStartPosition
- WalRcvDie
- XLogWalRcvFlush
- ProcessWalSndrMessage
In most of the places of the WAL sender, braces could be removed to
improve the style. For the WAL receiver, declarations are not
necessary. As a matter of style, why not cleaning up just the WAL
sender stuff? Changing the WAL receiver code just to remove some
declarations would not improve readability, and would make back-patch
more difficult.
--
Michael