RE: [Todo item] Add entry creation timestamp column to pg_stat_replication
>I have let this stuff cool down for a couple of days, still it seems to me >that having one single column makes the most sense when it comes when >looking at a mostly idle system. I'll try to finish this one at the >beginning of next week, for now I am marking as ready for committer. Thank you for your attention! Best regards, Myungkyu, Lim
RE: [Todo item] Add entry creation timestamp column to pg_stat_replication
>> 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? Thanks! I missed it..:( >Another thing which is crossing my mind is if it would make sense to report >the timestamp of the last HS feedback message and the timestamp of the last >status update message into two separate columns. As the point of this >field is to help with the debugging of mostly idle systems it seems to me >that merging both is fine, but I'd like to hear extra opinions about that. I think purpose of this field, that react interval check and debugging on idle system. So, merging both is better. (Is 'Reply' and 'HSFeedback' worth measuring separately?)
RE: [Todo item] Add entry creation timestamp column to pg_stat_replication
>On Mon, Nov 26, 2018 at 02:48:39PM +0900, myungkyu.lim wrote: >> So I selected 'reply_time' because 'timestamp' was not used in stat >> field. > >Fine by me. Could you send a new patch please? I am switching the patch as >waiting on author for now. Ok. Changed field name 'last_msg_send_time' to 'reply_time'. Attached new patch file : 0001-Implement-following-TODO-list-item-v4.patch example> postgres=# select pid, reply_time from pg_stat_replication; -[ RECORD 1 ]- pid| 10493 reply_time | 2018-11-29 17:37:14.638428+09 check plz. Best regards, Myungkyu, Lim 0001-Implement-following-TODO-list-item-v4.patch Description: Binary data
RE: [Todo item] Add entry creation timestamp column to pg_stat_replication
On Thu, Nov 15, 2018 at 5:27 PM Michael Paquier wrote: >> Good point. 'last_reply_send_time' is better. >> How about just 'reply_time'? > >Please note that the original thread has mentioned reply_timestamp as a >consensus: So I selected 'reply_time' because 'timestamp' was not used in stat field. >On Thu, Nov 15, 2018 at 05:33:26PM +0900, Masahiko Sawada wrote: >> Yeah, I also agree with 'reply_time'. But please also note that we had >> the discussion when there is not the similar system catalogs and >> fields. Now that we have them it might be worth to consider to follow >> the existing name for consistency. > >The fields in pg_stat_wal_receiver are named after their respective fields. >Now if you look at the fields from pg_stat_replication, you have those from >the standby: >sent_lsn => Last write-ahead log location sent on this connection write_lsn >=> Last write-ahead log location written to disk by this standby server >flush_lsn => Last write-ahead log location flushed to disk by this standby >server replay_lsn => Last write-ahead log location replayed into the >database on this standby server > >So to keep the brand consistent, reply_time looks like the best choice as >Sawada-san suggests? I agree. The fields naming in pg_stat_replication are some different from other views fields. Not used 'last_' or 'latest_' prefix in fields. Best regards, Myungkyu, Lim
RE: [Todo item] Add entry creation timestamp column to pg_stat_replication
Hi. >> I changed field name from 'reply_time' to 'last_msg_send_time'. >> Because 'last_msg_send_time' is used in >> pg_stat_wal_receiver/pg_stat_subsctiption view. >> I think that field has the same meaning. > >I got confused by the field name. If we have 'last_msg_send_time' >field in pg_stat_replciation which has information of wal senders >users would think it as a time when the wal sender sent a message last >time. However values of the fields actually shows a time when the wal >receiver sent a reply message last time. So perhaps >'last_reply_send_time' would be more clear. Good point. 'last_reply_send_time' is better. How about just 'reply_time'? Best regards, Myungkyu, Lim
RE: [Todo item] Add entry creation timestamp column to pg_stat_replication
Hi. Thanks for your feedback. > Can you explain the purpose of this feature more because now we have columns > to report replication delay times like write_lag ,flush_lag and replay_lag > that can use for similar purpose . I think, time elapsed stats are very useful on DML query active system, but not present that stats on idle system - not query, or only select. sent_lsn | 0/5476C88 write_lsn | 0/5476C88 flush_lsn | 0/5476C88 replay_lsn | 0/5476C88 write_lag | 00:00:00.55 flush_lag | 00:00:00.000855 replay_lag | 00:00:00.000914 sync_priority | 0 sync_state | async last_msg_send_time | 2018-11-15 14:04:39.65889+09 state | streaming sent_lsn | 0/5476CC0 write_lsn | 0/5476CC0 flush_lsn | 0/5476CC0 replay_lsn | 0/5476CC0 write_lag | flush_lag | replay_lag | sync_priority | 0 sync_state | async last_msg_send_time | 2018-11-15 14:05:02.935457+09 state | streaming sent_lsn | 0/5476CC0 write_lsn | 0/5476CC0 flush_lsn | 0/5476CC0 replay_lsn | 0/5476CC0 write_lag | flush_lag | replay_lag | sync_priority | 0 sync_state | async last_msg_send_time | 2018-11-15 14:06:23.128947+09 This timestamp column is useful when react interval check and debugging on idle system. Best regards, Myungkyu, Lim
RE: COPY FROM WHEN condition
>> COPY table_name WHERE (some_condition)
>>
>> Users should already be familiar with the idea that WHERE performs a filter.
>>
> So, what about using FILTER here? We already use it for aggregates when
> filtering rows to process.
> That being said, I have no strong feelings either way. I'd be OK with
> both WHEN and WHERE.
I don't think it's an important point,
In gram.y,
where_clause:
WHERE a_expr
{ $$ = $2; }
| /*EMPTY*/
{ $$ = NULL; }
;
This is similar to the 'opt_when_clause' in this patch.
So, I think 'WHERE' is a better form.
BTW, 3rd patch worked very well in my tests.
However, some wrong code style still exists.
Node*whenClause= NULL;
cstate->whenClause=whenClause;
Best regards,
Myungkyu, Lim
RE: COPY FROM WHEN condition
Hello, Basically, this patch worked very well in my tests. > 3) For COPY TO, the WHEN clause is accepted but ignored, leading to confusing > cases like this: I found same issue. postgres=# copy t1 to '/home/lmk/t1.data' when c1 < 1; In the 'COPY TO' statement, 'WHEN clause' does not do any extra work. (include error or hint) > 4) There are some minor code style issues in copy.c - the variable is > misnamed as when_cluase, there are no spaces after 'if' etc. See the attached > patch fixing this. It is recommended to use 'pg tool' when you finish development. src/tools/pgindent/pgindent pgindent is used to fix the source code style to conform to pg standards. Best regards, Myungkyu, Lim
