Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Set database application name in workers and server #13856

Merged
merged 2 commits into from
Apr 25, 2017

Conversation

jrafanie
Copy link
Member

@jrafanie jrafanie commented Feb 9, 2017

https://bugzilla.redhat.com/show_bug.cgi?id=1445928

When we need to do database lock or connection triaging, it's really hard to tell where each connection originated from because we don't specify an application_name. It looks something like this:

vmdb_production=# select pid, application_name from pg_stat_activity;
 pid   | application_name
 ------+--------------------------------------------------
 13770 | /var/www/miq/vmdb/lib/workers/bin/evm_server.rb
 13950 | /var/www/miq/vmdb/lib/workers/bin/evm_server.rb
 13957 | /var/www/miq/vmdb/lib/workers/bin/evm_server.rb
 13969 | /var/www/miq/vmdb/lib/workers/bin/evm_server.rb
 13975 | /var/www/miq/vmdb/lib/workers/bin/evm_server.rb
 13984 | /var/www/miq/vmdb/lib/workers/bin/evm_server.rb

The much debated format of the application_name now looks like this:

MIQ <worker pid> <worker class>[<worker id>], s[<server id>], <zone name>[<zone id>]
OR:
MIQ <server pid> Server[<server id>], <zone name>[<zone id>]

Note, the pg backend pid isn't needed in application_name since the pid column in pg_stat_activity is the backend pid. Plus, each of our processes can have more than one spid.

It now looks like this:

vmdb_development=# select pid, application_name from pg_stat_activity;
 pid  | application_name
------------------+------+-------------+----------------------
 5844 | MIQ 5835 Server[1r2], default[1r1]
 5888 | MIQ 5886 Generic[1r1111], s[1r2], default[1r1]
 5891 | MIQ 5889 Generic[1r1112], s[1r2], default[1r1]
 5894 | MIQ 5892 Priority[1r1113], s[1r2], default[1r1]
 5897 | MIQ 5895 Priority[1r1114], s[1r2], default[1r1]
 5900 | MIQ 5898 Schedule[1r1115], s[1r2], default[1r1]
 5928 | MIQ 5926 EventHandler[1r1116], s[1r2], default[1r1]
 5932 | MIQ 5929 Reporting[1r1117], s[1r2], default[1r1]
 5934 | MIQ 5931 Reporting[1r1118], s[1r2], default[1r1]
 5943 | MIQ 5941 Ui[1r1120], s[1r2], default[1r1]
 5940 | MIQ 5935 Websocket[1r1119], s[1r2], default[1r1]
 5946 | MIQ 5944 WebService[1r1121], s[1r2], default[1r1]
 5964 | MIQ 5935 Websocket[1r1119], s[1r2], default[1r1]
 5965 | MIQ 5941 Ui[1r1120], s[1r2], default[1r1]
 5966 | MIQ 5944 WebService[1r1121], s[1r2], default[1r1]

def self.name=(name)
# TODO: this is postgresql specific
ENV['PGAPPNAME'] = name
ActiveRecord::Base.connection_pool.disconnect!
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We set this ENV variable and disconnect because:

  • we probably have already established at least one connection
  • any new thread/connection needs to use this value

Unfortunately, we need to query the db to determine the name so we can't module prepend establish_connection (and call super).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note, postgresql will look for an application name in this ENV variable, so just setting it before establishing a connection is enough to get this set.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of disconnecting, maybe you can set the env var but still go through each connection in the pool and explicitly set the name with the query. This way it will update preexisting connections and handle new ones.

@@ -0,0 +1,14 @@
module ArSetApplicationName
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Naming.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe just ArApplicationName?

Copy link
Member

@carbonin carbonin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm good with the form of this.

Probably should work out a better name than ar_set_application_name.rb, but looks good otherwise.

@gtanzillo
Copy link
Member

Discussing this with @jrafanie and thinking that maybe this format would be better -

13079 | 13066 Server[1], s[1], default[1]
13386 | 13340 Ui[26], s[1], default[1] 

This format lends itself to being both human and machine readable.

@Fryguy
Copy link
Member

Fryguy commented Feb 10, 2017

13079 | 13066 Server[1], s[1], default[1]

but what does it mean?

@Fryguy
Copy link
Member

Fryguy commented Feb 10, 2017

Also, If we put user configurable strings in there, we have to make sure we don't hit the limit. For example, zone name can be just about anything, so maybe we should limit to the first 10 or 15 characters or something.

@Fryguy
Copy link
Member

Fryguy commented Feb 10, 2017

Last point...might want to toss an MIQ: at the front, just so they jump out

@gtanzillo
Copy link
Member

gtanzillo commented Feb 10, 2017

@Fryguy Forgot to put in a legend -

pg pid | <worker pid> <worker class>[<worker id>], s[<server id>], <zone name>[<zone id>]

@jrafanie jrafanie force-pushed the set_database_application_name branch from 75a5ee6 to b739429 Compare April 3, 2017 21:21
@jrafanie jrafanie force-pushed the set_database_application_name branch 2 times, most recently from 682d1c1 to 2b72400 Compare April 4, 2017 20:01
@jrafanie
Copy link
Member Author

jrafanie commented Apr 4, 2017

@Fryguy @carbonin @gtanzillo updated the description and pushed the changes.

Note: I STILL need to disconnect the server process's connection so it can re-establish the connection with the right application_name. I don't need to do this for the workers themselves, SET application_name... works for them. I don't know why.

@jrafanie jrafanie changed the title [WIP] Set database application name in workers and server Set database application name in workers and server Apr 4, 2017
name = database_application_name

# disconnect! after querying the information needed to set the application name
ActiveRecord::Base.connection_pool.disconnect!
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This happens only once in the server process. I'm not quite sure why SET application_name ... doesn't work here on the existing connection.

@miq-bot miq-bot removed the wip label Apr 4, 2017
@jrafanie jrafanie force-pushed the set_database_application_name branch from 2b72400 to 3722ace Compare April 17, 2017 19:40
Previously, pg_stat_activity would look like this:

```
vmdb_production=# select pid, application_name from pg_stat_activity;
 pid   | application_name
 ------+--------------------------------------------------
 13770 | /var/www/miq/vmdb/lib/workers/bin/evm_server.rb
 13950 | /var/www/miq/vmdb/lib/workers/bin/evm_server.rb
 13957 | /var/www/miq/vmdb/lib/workers/bin/evm_server.rb
 13969 | /var/www/miq/vmdb/lib/workers/bin/evm_server.rb
 13975 | /var/www/miq/vmdb/lib/workers/bin/evm_server.rb
 13984 | /var/www/miq/vmdb/lib/workers/bin/evm_server.rb
```

It now looks like this:

```
vmdb_development=# select pid, application_name from pg_stat_activity;
 pid  | application_name
------------------+------+-------------+----------------------
 5844 | MIQ 5835 Server[1r2], default[1r1]
 5888 | MIQ 5886 Generic[1r1111], s[1r2], default[1r1]
 5891 | MIQ 5889 Generic[1r1112], s[1r2], default[1r1]
 5894 | MIQ 5892 Priority[1r1113], s[1r2], default[1r1]
 5897 | MIQ 5895 Priority[1r1114], s[1r2], default[1r1]
 5900 | MIQ 5898 Schedule[1r1115], s[1r2], default[1r1]
 5928 | MIQ 5926 EventHandler[1r1116], s[1r2], default[1r1]
 5932 | MIQ 5929 Reporting[1r1117], s[1r2], default[1r1]
 5934 | MIQ 5931 Reporting[1r1118], s[1r2], default[1r1]
 5943 | MIQ 5941 Ui[1r1120], s[1r2], default[1r1]
 5940 | MIQ 5935 Websocket[1r1119], s[1r2], default[1r1]
 5946 | MIQ 5944 WebService[1r1121], s[1r2], default[1r1]
 5964 | MIQ 5935 Websocket[1r1119], s[1r2], default[1r1]
 5965 | MIQ 5941 Ui[1r1120], s[1r2], default[1r1]
 5966 | MIQ 5944 WebService[1r1121], s[1r2], default[1r1]
```
@jrafanie jrafanie force-pushed the set_database_application_name branch from 3722ace to b0d765d Compare April 17, 2017 19:42
@miq-bot
Copy link
Member

miq-bot commented Apr 17, 2017

Checked commits jrafanie/manageiq@242af70~...b0d765d with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
8 files checked, 2 offenses detected

app/models/embedded_ansible_worker/runner.rb

@gtanzillo gtanzillo added this to the Sprint 59 Ending Apr 24, 2017 milestone Apr 25, 2017
@gtanzillo gtanzillo merged commit f241df4 into ManageIQ:master Apr 25, 2017
@jrafanie jrafanie deleted the set_database_application_name branch April 25, 2017 14:43
jrafanie added a commit to jrafanie/manageiq that referenced this pull request Apr 26, 2017
Fixes a bug in ManageIQ#13856 when you run rake evm:start on a database without
a server row for the current server: undefined method
database_application_name for nil:NilClass.
jrafanie pushed a commit to jrafanie/manageiq that referenced this pull request Apr 26, 2017
…ion_name

Set database application name in workers and server
(cherry picked from commit f241df4)
jrafanie pushed a commit to jrafanie/manageiq that referenced this pull request Apr 26, 2017
…ion_name

Set database application name in workers and server
(cherry picked from commit f241df4)
@jrafanie
Copy link
Member Author

This PR and and #14904 were manually backported to:
Darga and euwe

@simaishi
Copy link
Contributor

simaishi commented May 9, 2017

Backported to Euwe via #14909

simaishi pushed a commit that referenced this pull request Jun 2, 2017
@simaishi
Copy link
Contributor

simaishi commented Jun 2, 2017

Fine backport details:

$ git log -1
commit c8701b6f6508bfd17e22b652ec2d1ed3b7f213ad
Author: Gregg Tanzillo <[email protected]>
Date:   Mon Apr 24 21:56:40 2017 -0400

    Merge pull request #13856 from jrafanie/set_database_application_name
    
    Set database application name in workers and server
    (cherry picked from commit f241df45cc9ef588bd4c9041249ec59264f117f9)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1458339

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants