Re: Asynchronous and "direct" IO support for PostgreSQL.
Hi, Thank you for the amazing and great work. On 23.02.2021 15:03, Andres Freund wrote: ## Stats There are two new views: pg_stat_aios showing AIOs that are currently in-progress, pg_stat_aio_backends showing per-backend statistics about AIO. As a DBA I would like to propose a few amendments that might help with practical usage of stats when feature will be finally implemented. My suggestions aren’t related to the central idea of the proposed changes, but rather to the stats part. A quick side note, there are two terms in Prometheus (https://prometheus.io/docs/concepts/metric_types/): 1. Counter. A counter is a cumulative metric that represents a single monotonically increasing counter whose value can only increase or be reset to zero on restart. 2. Gauge. A gauge is a metric that represents a single numerical value that can arbitrarily go up and down. For the purposes of long-term stats collection, COUNTERs are preferred over GAUGEs, because COUNTERs allow us to understand how metrics are changed overtime without missing out potential spikes in activity. As a result, we have a much better historic perspective. Measuring and collecting GAUGEs is limited to the moments in time when the stats are taken (snapshots) so the changes that took place between the snapshots remain unmeasured. In systems with a high rate of transactions per second (even 1 second interval between the snapshots) GAUGEs measuring won’t provide the full picture. In addition, most of the monitoring systems like Prometheus, Zabbix, etc. use longer intervals (from 10-15 to 60 seconds). The main idea is to try to expose almost all numeric stats as COUNTERs - this increases overall observabilty of implemented feature. pg_stat_aios. In general, this stat is a set of text values, and at the same time it looks GAUGE-like (similar to pg_stat_activity or pg_locks), and is only relevant for the moment when the user is looking at it. I think it would be better to rename this view to pg_stat_progress_aios. And keep pg_stat_aios for other AIO stats with global COUNTERs (like stuff in pg_stat_user_tables or pg_stat_statements, or system-wide /proc/stat, /proc/diskstats). pg_stat_aio_backends. This stat is based on COUNTERs, which is great, but the issue here is that its lifespan is limited by the lifespan of the backend processes - once the backend exits the stat will no longer be available - which could be inappropriate in workloads with short-lived backends. I think there might be few existing examples in the current code that could be repurposed to implement the suggestions above (such as pg_stat_user_tables, pg_stat_database, etc). With this in mind, I think having these changes incorporated shouldn’t take significant effort considering the benefit it will bring to the final user. Once again huge respect to your work on this changes and good look. Regards, Alexey
Re: Asynchronous and "direct" IO support for PostgreSQL.
Hi, On 25.02.2021 02:03, Andres Freund wrote: pg_stat_aio_backends. This stat is based on COUNTERs, which is great, but the issue here is that its lifespan is limited by the lifespan of the backend processes - once the backend exits the stat will no longer be available - which could be inappropriate in workloads with short-lived backends. There's a todo somewhere to roll over the per-connection stats into a global stats piece on disconnect. In addition I was thinking of adding a view that sums up the value of "already disconnected backends" and the currently connected ones. Would that mostly address your concerns? Yes, approach with separated stats for live and disconnected backends looks good and solves problem with "stats loss". Or it can be done like a stats for shared objects in pg_stat_databases, where there is a special NULL database is used. Regards, Alexey
Re: Skipping logical replication transactions on subscriber side
Hi, Have a few notes about pg_stat_logical_replication_error from the DBA point of view (which will use this view in the future). 1. As I understand it, this view might contain many errors related to different subscriptions. It is better to name "pg_stat_logical_replication_errors" using the plural form (like this done for stat views for tables, indexes, functions). Also, I'd like to suggest thinking twice about the view name (and function used in view DDL) - "pg_stat_logical_replication_error" contains very common "logical replication" words, but the view contains errors related to subscriptions only. In the future there could be other kinds of errors related to logical replication, but not related to subscriptions - what will you do? 2. Add a field with database name or id - it helps to quickly understand to which database the subscription belongs. 3. Add a counter field with total number of errors - it helps to calculate errors rates and aggregations (sum), and don't lose information about errors between view checks. 4. Add text of last error (if it will not be too expensive). 5. Rename the "action" field to "command", as I know this is right from terminology point of view. Finally, the view might seems like this: postgres(1:25250)=# select * from pg_stat_logical_replication_errors; subname | datid | relid | command | xid | total | last_failure | last_failure_text --++---+-+-+---+---+--- sub_1 | 12345 | 16384 | INSERT | 736 | 145 | 2021-06-27 12:12:45.142675+09 | something goes wrong... sub_2 | 12346 | 16458 | UPDATE | 845 | 12 | 2021-06-27 12:16:01.458752+09 | hmm, something goes wrong Regards, Alexey On Mon, Jul 5, 2021 at 2:59 PM Masahiko Sawada wrote: > On Thu, Jun 17, 2021 at 6:20 PM Masahiko Sawada > wrote: > > > > On Thu, Jun 17, 2021 at 3:24 PM Masahiko Sawada > wrote: > > > > > > > Now, if this function is used by super > > > > users then we can probably trust that they provide the XIDs that we > > > > can trust to be skipped but OTOH making a restriction to allow these > > > > functions to be used by superusers might restrict the usage of this > > > > repair tool. > > > > > > If we specify the subscription id or name, maybe we can allow also the > > > owner of subscription to do that operation? > > > > Ah, the owner of the subscription must be superuser. > > I've attached PoC patches. > > 0001 patch introduces the ability to skip transactions on the > subscriber side. We can specify XID to the subscription by like ALTER > SUBSCRIPTION test_sub SET SKIP TRANSACTION 100. The implementation > seems straightforward except for setting origin state. After skipping > the transaction we have to update the session origin state so that we > can start streaming the transaction next to the one that we just > skipped in case of the server crash or restarting the apply worker. We > set origin state to the commit WAL record. However, since we skip all > changes we don’t write any WAL even if we call CommitTransaction() at > the end of the skipped transaction. So the patch sets the origin state > to the transaction that updates the pg_subscription system catalog to > reset the skip XID. I think we need a discussion of this part. > > With 0002 and 0003 patches, we report the error information in server > logs and the stats view, respectively. 0002 patch adds errcontext for > messages that happened during applying the changes: > > ERROR: duplicate key value violates unique constraint "hoge_pkey" > DETAIL: Key (c)=(1) already exists. > CONTEXT: during apply of "INSERT" for relation "public.hoge" in > transaction with xid 736 committs 2021-06-27 12:12:30.053887+09 > > 0003 patch adds pg_stat_logical_replication_error statistics view > discussed on another thread[1]. The apply worker sends the error > information to the stats collector if an error happens during applying > changes. We can check those errors as follow: > > postgres(1:25250)=# select * from pg_stat_logical_replication_error; > subname | relid | action | xid | last_failure > --+---++-+--- > test_sub | 16384 | INSERT | 736 | 2021-06-27 12:12:45.142675+09 > (1 row) > > I added only columns required for the skipping transaction feature to > the view for now. > > Please note that those patches are meant to evaluate the concept we've > discussed so far. Those don't have the doc update yet. > > Regards, > > [1] > https://www.postgresql.org/message-id/DB35438F-9356-4841-89A0-412709EBD3AB%40enterprisedb.com > > > -- > Masahiko Sawada > EDB: https://www.enterprisedb.com/ > -- С уважением Алексей В. Лесовский
Re: Skipping logical replication transactions on subscriber side
On Tue, Jul 6, 2021 at 10:58 AM Masahiko Sawada wrote: > > Also, I'd like to suggest thinking twice about the view name (and > function used in view DDL) - "pg_stat_logical_replication_error" contains > very common "logical replication" words, but the view contains errors > related to subscriptions only. In the future there could be other kinds of > errors related to logical replication, but not related to subscriptions - > what will you do? > > Is pg_stat_subscription_errors or > pg_stat_logical_replication_apply_errors better? > It seems to me 'pg_stat_subscription_conflicts' proposed by Amit Kapila is the most suitable, because it directly says about conflicts occurring on the subscription side. The name 'pg_stat_subscription_errors' is also good, especially in case of further extension if some kind of similar errors will be tracked. > > 3. Add a counter field with total number of errors - it helps to > calculate errors rates and aggregations (sum), and don't lose information > about errors between view checks. > > Do you mean to increment the error count if the error (command, xid, > and relid) is the same as the previous one? or to have the total > number of errors per subscription? And what can we infer from the > error rates and aggregations? > To be honest, I hurried up when I wrote the first email, and read only about stats view. Later, I read the starting email about the patch and rethought this note. As I understand, when the conflict occurs, replication stops (until conflict is resolved), an error appears in the stats view. Now, no new errors can occur in the blocked subscription. Hence, there are impossible situations when many errors (like spikes) have occurred and a user didn't see that. If I am correct in my assumption, there is no need for counters. They are necessary only when errors might occur too frequently (like pg_stat_database.deadlocks). But if this is possible, I would prefer the total number of errors per subscription, as also proposed by Amit. Under "error rates and aggregations" I also mean in the context of when a high number of errors occured in a short period of time. If a user can read the "total errors" counter and keep this metric in his monitoring system, he will be able to calculate rates over time using functions in the monitoring system. This is extremely useful. I also would like to clarify, when conflict is resolved - the error record is cleared or kept in the view? If it is cleared, the error counter is required (because we don't want to lose all history of errors). If it is kept - the flag telling about the error is resolved is needed (or set xid to NULL). I mean when the user is watching the view, he should be able to identify if the error has already been resolved or not. -- Regards, Alexey On Tue, Jul 6, 2021 at 10:58 AM Masahiko Sawada wrote: > On Mon, Jul 5, 2021 at 7:33 PM Alexey Lesovsky wrote: > > > > Hi, > > Have a few notes about pg_stat_logical_replication_error from the DBA > point of view (which will use this view in the future). > > Thank you for the comments! > > > 1. As I understand it, this view might contain many errors related to > different subscriptions. It is better to name > "pg_stat_logical_replication_errors" using the plural form (like this done > for stat views for tables, indexes, functions). > > Agreed. > > > Also, I'd like to suggest thinking twice about the view name (and > function used in view DDL) - "pg_stat_logical_replication_error" contains > very common "logical replication" words, but the view contains errors > related to subscriptions only. In the future there could be other kinds of > errors related to logical replication, but not related to subscriptions - > what will you do? > > Is pg_stat_subscription_errors or > pg_stat_logical_replication_apply_errors better? > > > 2. Add a field with database name or id - it helps to quickly understand > to which database the subscription belongs. > > Agreed. > > > 3. Add a counter field with total number of errors - it helps to > calculate errors rates and aggregations (sum), and don't lose information > about errors between view checks. > > Do you mean to increment the error count if the error (command, xid, > and relid) is the same as the previous one? or to have the total > number of errors per subscription? And what can we infer from the > error rates and aggregations? > > > 4. Add text of last error (if it will not be too expensive). > > Agreed. > > > 5. Rename the "action" field to "command", as I know this is right from > terminology point of view. > > Okay. > > Regards, > > -- > Masahiko Sawada > EDB: https://www.enterprisedb.com/ > -- С уважением Алексей В. Лесовский
Re: Skipping logical replication transactions on subscriber side
On Fri, Jul 9, 2021 at 5:43 AM Masahiko Sawada wrote: > On Tue, Jul 6, 2021 at 7:13 PM Alexey Lesovsky wrote: > > > > On Tue, Jul 6, 2021 at 10:58 AM Masahiko Sawada > wrote: > >> > >> > Also, I'd like to suggest thinking twice about the view name (and > function used in view DDL) - "pg_stat_logical_replication_error" contains > very common "logical replication" words, but the view contains errors > related to subscriptions only. In the future there could be other kinds of > errors related to logical replication, but not related to subscriptions - > what will you do? > >> > >> > >> Is pg_stat_subscription_errors or > >> pg_stat_logical_replication_apply_errors better? > > > > > > It seems to me 'pg_stat_subscription_conflicts' proposed by Amit Kapila > is the most suitable, because it directly says about conflicts occurring on > the subscription side. The name 'pg_stat_subscription_errors' is also good, > especially in case of further extension if some kind of similar errors will > be tracked. > > I personally prefer pg_stat_subscription_errors since > pg_stat_subscription_conflicts could be used for conflict resolution > features in the future. This stats view I'm proposing is meant to > focus on errors that happened during applying logical changes. So > using the term 'errors' seems to make sense to me. > Agreed > > > >> > >> > 3. Add a counter field with total number of errors - it helps to > calculate errors rates and aggregations (sum), and don't lose information > about errors between view checks. > >> > >> Do you mean to increment the error count if the error (command, xid, > >> and relid) is the same as the previous one? or to have the total > >> number of errors per subscription? And what can we infer from the > >> error rates and aggregations? > > > > > > To be honest, I hurried up when I wrote the first email, and read only > about stats view. Later, I read the starting email about the patch and > rethought this note. > > > > As I understand, when the conflict occurs, replication stops (until > conflict is resolved), an error appears in the stats view. Now, no new > errors can occur in the blocked subscription. Hence, there are impossible > situations when many errors (like spikes) have occurred and a user didn't > see that. If I am correct in my assumption, there is no need for counters. > They are necessary only when errors might occur too frequently (like > pg_stat_database.deadlocks). But if this is possible, I would prefer the > total number of errors per subscription, as also proposed by Amit. > > Yeah, the total number of errors seems better. > Agreed > > > > Under "error rates and aggregations" I also mean in the context of when > a high number of errors occured in a short period of time. If a user can > read the "total errors" counter and keep this metric in his monitoring > system, he will be able to calculate rates over time using functions in the > monitoring system. This is extremely useful. > > Thanks for your explanation. Agreed. But the rate depends on > wal_retrieve_retry_interval so is not likely to be high in practice. > Agreed > > I also would like to clarify, when conflict is resolved - the error > record is cleared or kept in the view? If it is cleared, the error counter > is required (because we don't want to lose all history of errors). If it is > kept - the flag telling about the error is resolved is needed (or set xid > to NULL). I mean when the user is watching the view, he should be able to > identify if the error has already been resolved or not. > > With the current patch, once the conflict is resolved by skipping the > transaction in question, its entry on the stats view is cleared. As > you suggested, if we have the total error counts in that view, it > would be good to keep the count and clear other fields such as xid, > last_failure, and command etc. > Ok, looks nice. But I am curious how this will work in the case when there are two (or more) errors in the same subscription, but different relations? After resolution all these records are kept or they will be merged into a single record (because subscription was the same for all errors)? > Regards, > > -- > Masahiko Sawada > EDB: https://www.enterprisedb.com/ > -- Regards, Alexey Lesovsky
Re: Skipping logical replication transactions on subscriber side
On Mon, Jul 12, 2021 at 8:36 AM Amit Kapila wrote: > > > > Ok, looks nice. But I am curious how this will work in the case when > there are two (or more) errors in the same subscription, but different > relations? > > > > We can't proceed unless the first error is resolved, so there > shouldn't be multiple unresolved errors. > Ok. I thought multiple errors are possible when many tables are initialized using parallel workers (with max_sync_workers_per_subscription > 1). -- Regards, Alexey
