Re: Function and view to retrieve WAL receiver status

On Wed, Jan 6, 2016 at 8:14 AM, Haribabu Kommi <kommi.haribabu@gmail.com> wrote:
> Following are my observations on the latest patch.

Thanks for your review.

> + If no WAL receiver is present on the server queried,
> +   a single tuple filled with <literal>NULL</> values is returned instead.
> +  </para>
>
> The above documentation change is not required if we change the system
> view.

Affirmative.

> +            s.received_up_to_lsn,
>
> The column name can be changed as "received_lsn" similar to "received_tli".
> up_to may not be required.
>
> + XLogRecPtr received_up_lsn;
> + TimeLineID received_up_tli;
>
> same as like above comment.

Indeed, let's make the variable names more simple and consistent by
removing this _up_ portion everywhere.

> + /* lock? */
>
> I find out that walrcv data is updated only under mutex. it is better
> to take that mutex to provide a consistent snapshot data to user.

The lock is taken, the comment is just incorrect:
+    /* lock? */
+    SpinLockAcquire(&walrcv->mutex);
[...]
+    SpinLockRelease(&walrcv->mutex);

I also found out that the description of those fields was not clear
enough actually: received_tli and received _lsn are related to what
has been received *and* flushed to disk, with an initial value being
their start equivalent. This deserves a clear description with all
those things addressed.

Attached is an updated patch.
--
Michael