Bug 172

Summary: Use application_name
Product: Slony-I Reporter: Christopher Browne <cbbrowne>
Component: slonAssignee: Christopher Browne <cbbrowne>
Status: RESOLVED FIXED    
Severity: enhancement CC: slony1-bugs
Priority: low    
Version: devel   
Hardware: All   
OS: All   

Description Christopher Browne 2010-12-03 12:56:59 UTC
http://wiki.postgresql.org/wiki/SlonyBrainstorming#Use_application_name


If on a version of Postgres that supports GUC application_name, connections should capture "this is slon" - application_name is available on PG 9.0+

The value we use should likely include various (possibly all) of the following:
- slon
- ID of the node being managed by slon
- ID of the secondary node being queried
- perhaps the cluster name?

Similar is apropos for slonik.
Comment 1 Christopher Browne 2010-12-03 12:58:11 UTC
Note that this allows monitoring processes looking at the view pg_stat_activity to have a little more information as to "what's this process for?"
Comment 2 Christopher Browne 2010-12-10 08:13:35 UTC
Taking a peek at slon source code...


-> % ack-grep slon_connectdb
local_listen.c
57:     if ((conn = slon_connectdb(rtcfg_conninfo, "local_listen")) == NULL)

cleanup_thread.c
80:     if ((conn = slon_connectdb(rtcfg_conninfo, "local_cleanup")) == NULL)

remote_worker.c
370:    if ((local_conn = slon_connectdb(rtcfg_conninfo, conn_symname)) == NULL)
2588:   if ((pro_conn = slon_connectdb(conninfo, conn_symname)) == NULL)
3745:                   provider->conn = slon_connectdb(provider->pa_conninfo,

remote_listen.c
208:                    conn = slon_connectdb(conn_conninfo, conn_symname);

sync_thread.c
59:     if ((conn = slon_connectdb(rtcfg_conninfo, "local_sync")) == NULL)

slon.h
567:extern SlonConn *slon_connectdb(char *conninfo, char *symname);

dbutils.c
47: * slon_connectdb
51:slon_connectdb(char *conninfo, char *symname)
71:                              "slon_connectdb: PQconnectdb(\"%s\") failed\n",
78:                              "slon_connectdb: PQconnectdb(\"%s\") failed - %s",
120:                             "slon_connectdb: PQconnectdb(\"%s\") PostgreSQL version not supported\n",

The value of "conn_symname" looks to be pretty suitable as being most of the value to pass in.

Values include:
 local_listen
 local_cleanup
 local_sync
And within remote_worker.c:
 remoteWorkerThread_%d
 copy_set_%d
 subscriber_%d_provider_%d
Comment 3 Christopher Browne 2010-12-10 08:26:53 UTC
Added branch for development of this feature...

Added function store_application_name(name) that checks to see if the GUC variable exists, and sets it, if so.

Added a hook to slon_connectdb() that calls the function.

https://github.com/cbbrowne/slony1-engine/commit/a2d97d94017e74b0fdad100f4be3b2327a56b29d

Sample output on Postgres HEAD:


testdb@localhost->  select datid, datname, procpid, application_name from pg_stat_activity where application_name like 'slon%';
 datid |    datname    | procpid |     application_name
-------+---------------+---------+---------------------------
 44510 | slonyregress1 |   30969 | slon.local_listen
 44510 | slonyregress1 |   30974 | slon.local_cleanup
 44510 | slonyregress1 |   30975 | slon.local_sync
 44573 | slonyregress2 |   30976 | slon.node_2_listen
 44510 | slonyregress1 |   30977 | slon.remoteWorkerThread_2
(5 rows)
Comment 4 Christopher Browne 2010-12-10 08:50:12 UTC
Do much the same for slonik...

https://github.com/cbbrowne/slony1-engine/commit/bbe9a3f0f7015768bf3fbc044d9a9bf14039859f
Comment 5 Christopher Browne 2010-12-10 11:45:20 UTC
https://github.com/cbbrowne/slony1-engine/commit/766acf3e105644878e19af8c1fa14f568f187341

Revise slonik handling of application_name...

Query to see if the GUC is there, and only try to set the GUC if it
actually exists in this database.
Comment 6 Steve Singer 2010-12-15 06:02:12 UTC
In the first patch you add a stored function store_application_name()
wouldn't it be better for your change to slonik to use this function
instead of duplicating the logic?

I realize that there is a bit of a chicken/egg situation here in that until slonik installs the slony schema the function doesn't exist so it can't be called.


I am also not sure how I feel about querying the catalog pg_settings table for application name to see what version we are on.   The other way of approaching
this is to check the server version on from the pg connection data structures and not set the application name accordingly  OR we can have a no-op implementation of store_application_name() for versions < 9.0.
Comment 7 Christopher Browne 2010-12-15 08:30:37 UTC
(In reply to comment #6)
> In the first patch you add a stored function store_application_name()
> wouldn't it be better for your change to slonik to use this function
> instead of duplicating the logic?
> 
> I realize that there is a bit of a chicken/egg situation here in that until
> slonik installs the slony schema the function doesn't exist so it can't be
> called.

There lies the problem...  slonik can't call it unless the function is already there.

The alternative, I suppose, is to change the slon logic to duplicate the logic in slonik, so we don't need to have a version-specific stored function.

> I am also not sure how I feel about querying the catalog pg_settings table for
> application name to see what version we are on.   The other way of approaching
> this is to check the server version on from the pg connection data structures
> and not set the application name accordingly  OR we can have a no-op
> implementation of store_application_name() for versions < 9.0.

I'm uninterested in what version we are on; I am interested in whether or not Postgres supports the GUC "application_name".  If we handle this feature that way, then we never have to ask what version it is tied to, because it doesn't matter.
Comment 8 Steve Singer 2010-12-15 10:07:08 UTC
(In reply to comment #7)


That argument makes sense.

It seems fine to commit except for:

- For the slonik change, even if PQntuples(res)==0 you still need to call PQclear(res) to free the structure (that contains zero tuples) or you have a small memory leak.

You do this properly in the slon change.
Comment 9 Christopher Browne 2010-12-15 12:50:36 UTC
Added freeing of res in slonik.c...

Squashed code into a commit to put into master:

http://git.postgresql.org/gitweb?p=slony1-engine.git;a=commit;h=4ae9a212ccf6fe0bd52a4c015a39a251107405a5