| From: | Michael Paquier <michael(at)paquier(dot)xyz> | 
|---|---|
| To: | "myungkyu(dot)lim" <myungkyu(dot)lim(at)samsung(dot)com> | 
| Cc: | 'Masahiko Sawada' <sawada(dot)mshk(at)gmail(dot)com>, 'PostgreSQL-development' <pgsql-hackers(at)postgresql(dot)org>, woosung(dot)sohn(at)samsung(dot)com, don(dot)hong(at)samsung(dot)com | 
| Subject: | Re: [Todo item] Add entry creation timestamp column to pg_stat_replication | 
| Date: | 2018-12-04 03:56:25 | 
| Message-ID: | 20181204035625.GQ3423@paquier.xyz | 
| Views: | Whole Thread | Raw Message | Download mbox | Resend email | 
| Thread: | |
| Lists: | pgsql-hackers | 
On Fri, Nov 30, 2018 at 05:54:15PM +0900, Michael Paquier wrote:
> Looks pretty to me at quick glance, unfortunately I have not spent much
> time on it, particularly testing it.
> 
> +    <row>
> +     <entry><structfield>reply_time</structfield></entry>
> +     <entry><type>timestamp with time zone</type></entry>
> +     <entry>Send time of last message received from WAL
> receiver</entry>
> +    </row>
> This is a bit confusing as this counts for the last *standby* message
> ('r'), and not the last feedback message ('h').
> 
> +   /*
> +    * timestamp of the latest message, reported by standby server
> +   */
> +   TimestampTz replyTime;
> 
> A small indentation problem, not a big deal.
I have been looking at this patch more in-depth, and you missed one
critical thing: hot standby feedback messages also include the timestamp
the client used when sending the message, so if we want to track the
latest time when a message has been sent we should track it as much as
the timestamp from status update messages.
Fixing that and updating a couple of comments and variables, I am
finishing with the attached.  Thoughts?
(The catversion bump is a self-reminder, don't worry about it.)
--
Michael
| Attachment | Content-Type | Size | 
|---|---|---|
| standby-message-time-v3.patch | text/x-diff | 8.8 KB | 
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Ideriha, Takeshi | 2018-12-04 04:07:34 | RE: idle-in-transaction timeout error does not give a hint | 
| Previous Message | Peter Geoghegan | 2018-12-04 03:26:31 | Re: Index Skip Scan |