Ignore walsender type queries

[bheemreddy181-zz]

Can we Ignore walsender process queries ?

Logical replication processes are walsender processes that can have the appearance of a long-running query if not specially treated. These should be ignored as they will always show up at the top of the slow query list otherwise. They have backend_type "walsender" (for 10+) and can be found using pg_stat_replication for 9.6.

[bheemreddy181-zz]

@ankane Can you take a look at this whenever you are free ?

[bheemreddy181-zz]

@ankane Can I get some feedback on this one? It will be really helpful as we are planning to roll this out for a larger set audience

[ankane]

Hey @bheemreddy181, thanks for the PR. However, I think it's good to show all queries on that page.

[bheemreddy181-zz]

@ankane walsender queries are not really a set of Long-running queries in general and these don't need the user's attention - I feel the user will end up misinterpreting the General Long-Running Queries with these.

[ankane]

Makes sense (was thinking this was for the Live Queries page). Let's treat it like autovacuum queries - include them in running queries but filter in the controller and show the count in the view if there are any (see @autovacuum_queries in code). I think we can just add backend_type to running queries and have it be nil for Postgres < 10 rather than including another table in the query.

[bheemreddy181-zz]

This is actually on the Home Page , looks like below

[bheemreddy181-zz]

we can just add backend_type to running queries

Can you share some pointers on how to add backend_type to running queries - couldn’t find a reference in the master code base.

[bheemreddy181-zz]

we can just add backend_type to running queries

Can you share some pointers on how to add backend_type to running queries - couldn’t find a reference in the master code base.

I think I get it now, let me send out a PR

[bheemreddy181-zz]

nil for Postgres < 10 rather than including another table in the query.

@ankane
I am not clear on this part , Can you elaborate a bit how will a nil value will filter for Postgres < 10 , I assume we still need an extra table to make sure these queries are excluded. there does not seem to be a way to find walsender processes in 9.6 unless you join to pg_stat_replication - because there is no backend_type field in pg_stat_activity

[ankane]

For simplicity, I think we can make this a Postgres 10+ feature. pg_stat_replication isn't available on all systems and Postgres 9.6 reaches EOL in November.

[bheemreddy181-zz]

So in that case do you want me to still filter those queries in home_controller? similar to autovacuum_queries into a different variable using more if-else conditions as partition seems like only works for two results

[ankane]

You should be able to partition before autovacuum queries are partitioned, but in the Postgres < 10 case, none of the queries will have a backend type of walsender, so none will go to that partition (as expected).

[bheemreddy181-zz]

Is this how you are expecting the query to look like - where version >=10 , select backend_type column and filter it in the home controller? how is auto vacuum column added to the query here ?

SELECT
            pid,
            state,
            application_name AS source,
            age(NOW(), COALESCE(query_start, xact_start)) AS duration,
            #{server_version_num >= 90600 ? "(wait_event IS NOT NULL) AS waiting" : "waiting"},
            #{server_version_num >= 100000 ? "backend_type" : nil}           
            query,
            COALESCE(query_start, xact_start) AS started_at,
            EXTRACT(EPOCH FROM NOW() - COALESCE(query_start, xact_start)) * 1000.0 AS duration_ms,
            usename AS user
          FROM
            pg_stat_activity
          WHERE
            state <> 'idle'
            AND pid <> pg_backend_pid()
            AND datname = current_database()
            #{min_duration ? "AND NOW() - COALESCE(query_start, xact_start) > interval '#{min_duration.to_i} seconds'" : nil}
            #{all ? nil : "AND query <> '<insufficient privilege>'"}
          ORDER BY
            COALESCE(query_start, xact_start) DESC

[ankane]

You should be able to add a single line to the query:

#{server_version_num >= 100000 ? "backend_type" : "NULL AS backend_type"},

[bheemreddy181-zz]

Pushed changes, quick feedback will be appreciated

[ankane]

Hey @bheemreddy181, looking at the code, I'm not sure the partition logic is correct. Can you make sure it works on your Postgres instance? Also, let's have the UI show walsender instead of backendtype.

[bheemreddy181-zz]

@ankane Tested locally PFB screenshot

Do you think we should hide them from the queries page as well ? Or exclude the kill functionality for these queries ?

Tested with one long running query and a walsender query

[ankane]

Great, looking much better. Is there a reason it uses starts_with? instead of ==?

As mentioned earlier in the thread, I think they should be included on the Live Queries page (no changes there). Same thoughts for the "Kill all connections" feature.

[bheemreddy181-zz]

Great, looking much better. Is there a reason it uses starts_with? instead of ==?

Not really was just following the autovaccum format , do you want me to change that ?

[bheemreddy181-zz]

Great, looking much better. Is there a reason it uses starts_with? instead of ==?

Updated that